Skip to content

Add datasource and taskId to metrics emitted by peons#1967

Merged
himanshug merged 1 commit intoapache:masterfrom
metamx:realtime-metrics-improvements
Dec 3, 2015
Merged

Add datasource and taskId to metrics emitted by peons#1967
himanshug merged 1 commit intoapache:masterfrom
metamx:realtime-metrics-improvements

Conversation

@nishantmonu51
Copy link
Copy Markdown
Member

@nishantmonu51 nishantmonu51 commented Nov 13, 2015

This PR adds the datasource and taskId to the jvm and sys metrics
emitted by the peons.

gianm
gianm reviewed Nov 13, 2015
View reviewed changes
Comment thread server/src/main/java/io/druid/server/metrics/MonitorsConfig.java Outdated
Copy link
Copy Markdown
Contributor

@gianm gianm Nov 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

emitter (spelling)

@nishantmonu51 nishantmonu51 force-pushed the realtime-metrics-improvements branch from 4fc027b to 9fdc048 Compare November 13, 2015 21:55
@nishantmonu51
Copy link
Copy Markdown
Member Author

nishantmonu51 commented Nov 14, 2015

travis failed with
io.druid.query.extraction.namespace.TestKafkaExtractionCluster
testSimpleRename(io.druid.query.extraction.namespace.TestKafkaExtractionCluster) Time elapsed: 0.182 sec <<< FAILURE!
java.lang.AssertionError: expected: but was:
at org.junit.Assert.fail(Assert.java:88)

bouncing the PR.

@nishantmonu51 nishantmonu51 reopened this Nov 14, 2015
drcrallen
drcrallen reviewed Nov 16, 2015
View reviewed changes
Comment thread server/src/main/java/io/druid/server/metrics/MetricsModule.java
Copy link
Copy Markdown
Contributor

@drcrallen drcrallen Nov 16, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tested this with the LoggingEmitter? and null property value?

Copy link
Copy Markdown
Member Author

@nishantmonu51 nishantmonu51 Nov 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified it with loggingEmitter,
the key we got is from an existing property, afaik in this case the value cannot be null here, am i missing anything ?

Copy link
Copy Markdown
Contributor

@drcrallen drcrallen Nov 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing it.

Where guarantees that the property exists?

Copy link
Copy Markdown
Contributor

@drcrallen drcrallen Nov 18, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nevermind

@nishantmonu51 nishantmonu51 force-pushed the realtime-metrics-improvements branch from 9fdc048 to 7b080c8 Compare November 18, 2015 18:48
@nishantmonu51 nishantmonu51 reopened this Nov 20, 2015
@nishantmonu51
Copy link
Copy Markdown
Member Author

nishantmonu51 commented Nov 24, 2015

@himanshug @drcrallen any more comments ?

@himanshug
Copy link
Copy Markdown
Contributor

himanshug commented Nov 25, 2015

👍 (please update the documentation appropriately if needed.)

@nishantmonu51 nishantmonu51 added this to the 0.8.3 milestone Dec 1, 2015
drcrallen
drcrallen reviewed Dec 1, 2015
View reviewed changes
Comment thread processing/src/main/java/io/druid/query/DruidMetrics.java Outdated
Copy link
Copy Markdown
Contributor

@drcrallen drcrallen Dec 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Camel case is usually used for taskId I believe

@nishantmonu51
Add datasource and taskId to metrics emitted by peons
1eb8211 
This PR adds the datasource and taskId to the jvm and sys metrics
emitted by the peons.

fix spelling

review comment

review comment
@nishantmonu51 nishantmonu51 force-pushed the realtime-metrics-improvements branch from 7b080c8 to 1eb8211 Compare December 1, 2015 17:51
@drcrallen
Copy link
Copy Markdown
Contributor

drcrallen commented Dec 1, 2015

👍 after travis

@nishantmonu51
Copy link
Copy Markdown
Member Author

nishantmonu51 commented Dec 1, 2015

travis failed with -
DruidCoordinatorTest.testCoordinatorRun:349 expected:<1> but was:<0>

@nishantmonu51 nishantmonu51 reopened this Dec 1, 2015
@nishantmonu51 nishantmonu51 mentioned this pull request Dec 1, 2015
Merged
@drcrallen drcrallen removed this from the 0.8.3 milestone Dec 1, 2015
himanshug added a commit that referenced this pull request Dec 3, 2015
@himanshug
Merge pull request #1967 from metamx/realtime-metrics-improvements
d02be61 
Add datasource and taskId to metrics emitted by peons
@himanshug himanshug merged commit d02be61 into apache:master Dec 3, 2015
nishantmonu51 added a commit that referenced this pull request Dec 17, 2015
@nishantmonu51
Merge pull request #2105 from metamx/backport-1967
e640146 
Backport #1967 - Add datasource and taskId to metrics emitted by peons
@gianm gianm mentioned this pull request Jan 8, 2016
Closed
@fjy fjy modified the milestone: 0.9.0 Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.9.0

Development

Successfully merging this pull request may close these issues.

6 participants

@nishantmonu51 @himanshug @drcrallen @xvrl @gianm @fjy