Fix issue with historgram statistic set being converted by duration #6
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
azagniotov
reviewed
Aug 14, 2017
| @@ -242,7 +242,7 @@ private void processHistogram(final String metricName, final Histogram histogram | |||
| stageMetricDatum(true, metricName, value, StandardUnit.None, percentile.getDesc(), metricData); | |||
| } | |||
|
|
|||
| stageMetricDatum(builder.withStatisticSet, metricName, snapshot, StandardUnit.None, "snapshot-summary", metricData); | |||
| stageHistorgramMtricStatisticSet(builder.withStatisticSet, metricName, snapshot, StandardUnit.None, "snapshot-summary", metricData); | |||
There was a problem hiding this comment.
Typo in: stageHistorgramMtricStatisticSet
azagniotov
reviewed
Aug 14, 2017
| .filter(metricDatum -> metricDatum.getDimensions() | ||
| .contains(new Dimension().withName("Type").withValue(dimensionValue))) | ||
| .filter(metricDatum -> { | ||
| System.out.println(metricDatum.getDimensions()); |
There was a problem hiding this comment.
Forgot the System.out.println due to debugging?
|
Thank you! Left some comments |
…ogram to show subsequent reports are correct
|
Addressed the comments and added another test for a sliding window histogram to sanity check that subsequent report methods correctly update the min/max etc |
azagniotov
reviewed
Aug 15, 2017
| slidingWindowHistogram.update(100); | ||
| slidingWindowHistogram.update(5); | ||
| slidingWindowHistogram.update(6); | ||
| reporterBuilder.withStatisticSet().build().report(); |
There was a problem hiding this comment.
I am curious: did you intentionally re-build the CloudWatchReporter before reporting the 2nd time? Why not just reporter.report(); without rebuilding - I guess it can be another test case?
|
Thank you for your input with this!
…On Tue, Aug 15, 2017 at 3:07 AM, williedoran ***@***.***> wrote:
Addressed the comments and added another test for a sliding window
histogram to sanity check that subsequent report methods correctly update
the min/max etc
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#6 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA8azKjDPo0S3JAIfKrIOI2dH3GK-eU5ks5sYW3ggaJpZM4O2taK>
.
|
azagniotov
added a commit
that referenced
this pull request
Aug 20, 2017
- Issue #4: Not reporting zero values. - PR #6: Histogram snapshot values being converted using a duration factor, instead of reporting raw values (https://github.com/williedoran) - Checking 'isDebugEnabled' when logging debug information
azagniotov
added a commit
that referenced
this pull request
Aug 20, 2017
- Issue #4: Not reporting zero values. - PR #6: Histogram snapshot values being converted using a duration factor, instead of reporting raw values (https://github.com/williedoran) - Checking 'isDebugEnabled' when logging debug information
azagniotov
added a commit
that referenced
this pull request
Aug 20, 2017
- Issue #4: Not reporting zero values. - PR #6: Histogram snapshot values being converted using a duration factor, instead of reporting raw values (https://github.com/williedoran) - Checking 'isDebugEnabled' when logging debug information
azagniotov
added a commit
that referenced
this pull request
Aug 20, 2017
- Issue #4: Not reporting zero values. - PR #6: Histogram snapshot values being converted using a duration factor, instead of reporting raw values (https://github.com/williedoran) - Checking 'isDebugEnabled' when logging debug information
azagniotov
added a commit
that referenced
this pull request
Aug 20, 2017
- Issue #4: Not reporting zero values. - PR #6: Histogram snapshot values being converted using a duration factor, instead of reporting raw values (https://github.com/williedoran) - Checking 'isDebugEnabled' when logging debug information
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.
removed covert by duration and added test for historgram min,max and sum reporting in statistic set