StatsDEmitter rounds rates/percentages to 0 #3936
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michaelschiff
force-pushed
the
issue3350
branch
from
0273bdb to
f6c4919
Compare
gianm
reviewed
Feb 15, 2017
There was a problem hiding this comment.
@michaelschiff, thanks for the fix. The code looks good, but one of the tests is failing -- could you please fix it?
java.lang.AssertionError: correct StatsDMetric.Type expected:<timer> but was:<io.druid.emitter.statsd.StatsDMetric@67c33749>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:743)
at org.junit.Assert.assertEquals(Assert.java:118)
at DimensionConverterTest.testConvert(DimensionConverterTest.java:59)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:483)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:50)
at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:367)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:274)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:238)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:161)
at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:290)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:242)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:121)
|
|
||
| /** | ||
| * Created by michael.schiff on 2/14/17. | ||
| */ |
There was a problem hiding this comment.
Please don't include author tags.
| @JsonCreator | ||
| public StatsDMetric( | ||
| @JsonProperty("dimensions") SortedSet<String> dimensions, | ||
| @JsonProperty("type") Type type) | ||
| @JsonProperty("type") Type type, | ||
| @JsonProperty("convertRange") boolean convertRange |
There was a problem hiding this comment.
Please update the docs in "statsd.md" for this new parameter.
…metric values from range 0-1 to 0-100. This prevents rates and percentages expressed as Doubles (0.xx) from being rounded down to 0.
michaelschiff
force-pushed
the
issue3350
branch
from
f6c4919 to
602a35c
Compare
gianm
approved these changes
Feb 16, 2017
There was a problem hiding this comment.
👍 LGTM
In the future please do updates by pushing new commits rather than amending or rebasing, since that makes it easier for reviewers to see what's new since they last looked. (GitHub can show a diff for just the latest commits)
We'll squash the multiple commits before merging them.
|
👍 |
This was referenced Sep 5, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
StatsD expects that metric values be integers. Some Druid metrics are emitted as doubles with values between 0 and 1. Currently these values are floored to 0 when converted to longs. This PR implements the solution proposed in #3350 which optionally converts metric values into the proper range by multiplying by 100 and rounding to the nearest integer.