-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix(metrics): calculate ddsketch averages from histogram sum & count fields #24379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
tony-resendiz
wants to merge
16
commits into
vectordotdev:master
from
tony-resendiz:ddsketch-averages
Closed
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
31d3db0
update some ddsketch fields from histogram
tony-resendiz be24867
add changelog fragment
tony-resendiz 3911f2e
formatting
tony-resendiz 70399f7
Merge branch 'vectordotdev:master' into ddsketch-averages
tony-resendiz 479e10c
don't modify the sketch if count is bigger than u32
tony-resendiz 38c8605
Update lib/vector-core/src/metrics/ddsketch.rs
tony-resendiz cffc82c
pr feedback
tony-resendiz 2068dc0
pr checks
tony-resendiz d93e259
Merge branch 'master' into ddsketch-averages
pront 98a1bda
update changelog.d
tony-resendiz 371280a
feedback
tony-resendiz 90450b2
feedback
tony-resendiz 647916b
feedback
tony-resendiz 52d0ee4
feedback
tony-resendiz 5cfc57f
feedback
tony-resendiz 7a84055
feedback
tony-resendiz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -522,6 +522,7 @@ SIGHUPs | |
| sinkbuilder | ||
| sinknetworkbytessent | ||
| slideover | ||
| SLOs | ||
| smartphone | ||
| Smol | ||
| smtracking | ||
|
|
||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| Modified behavior of histograms (ddsketch) submitted by the Datadog metrics sink. Previously the ddsketch sum, count, and average fields were interpolated from bucket boundaries, which could cause slight inaccuracies. Now those fields will be calculated directly from the corresponding fields on the incoming histogram. Min and max fields remain interpolated. | ||
|
|
||
| Users with alerts, dashboards, or SLOs based on histogram averages should review and adjust thresholds after upgrading, as values will settle to their correct levels. | ||
|
|
||
| authors: tony-resendiz |
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When histogram
countdiffers from the interpolated bucket total, this computesavgassum / sketch.count()rather thansum / histogram.count. In that case (for example when bucket totals are incomplete), the resulting sketch average is still biased by interpolation loss and does not reflect the upstream histogram mean, which defeats the purpose of this fix and can misreportavgdownstream.Useful? React with 👍 / 👎.