fix(metrics): calculate ddsketch averages from histogram sum & count fields by tony-resendiz · Pull Request #24379 · vectordotdev/vector

tony-resendiz · 2025-12-13T00:21:05Z

Summary

when histograms are processed by the datadog metrics sink a ddsketch is built by interpolating values from the bucket boundaries. this causes some slight inaccuracy in the sketch average. this change sets the sketch average to the histogram's sum / count.

Vector configuration

[log_schema]
level_key = "status"
level = "info"

[api]
enabled = true
address = "0.0.0.0:8787"
playground = false

[sources.opentelemetry]
type = "opentelemetry"
grpc.address = "0.0.0.0:4317"
http.address = "0.0.0.0:4318"

[sinks.datadog_metrics_sink]
type = "datadog_metrics"
inputs = ["opentelemetry.metrics"]
default_api_key = "${DD_API_KEY}"
endpoint = "https://metrics.agent.datadoghq.com"

How did you test this PR?

built new vector binary and tested against the above config.

Change Type

Bug fix
New feature
Non-functional (chore, refactoring, docs)
Performance

Is this a breaking change?

Yes
No

Does this PR include user facing changes?

Yes. Please add a changelog fragment based on our guidelines.
No. A maintainer will apply the no-changelog label to this PR.

References

Closes: ddsketch from histogram seems inaccurate #24365

Notes

Please read our Vector contributor resources.
Do not hesitate to use @vectordotdev/vector to reach out to us regarding this PR.
Some CI checks run only after we manually approve them.
- We recommend adding a pre-push hook, please see this template.
- Alternatively, we recommend running the following locally before pushing to the remote branch:
  - make fmt
  - make check-clippy (if there are failures it's possible some of them can be fixed with make clippy-fix)
  - make test
After a review is requested, please avoid force pushes to help us review incrementally.
- Feel free to push as many commits as you want. They will be squashed into one before merging.
- For example, you can run git merge origin master and git push.
If this PR introduces changes Vector dependencies (modifies Cargo.lock), please
run make build-licenses to regenerate the license inventory and commit the changes (if any). More details here.

github-actions · 2025-12-13T00:21:15Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

tony-resendiz · 2025-12-13T00:27:45Z

I have read the CLA Document and I hereby sign the CLA

tony-resendiz · 2026-01-14T19:50:17Z

hi @fuchsnj sry for the ping. would you mind taking a look at this pr? 🙇

bruceg · 2026-02-05T16:55:31Z

+                buckets,
+                sum,
+                count,
+                ..


I don't see AggregatedHistogram as having any other fields, so the .. seems to be superfluous here.

thanks for catching, removed it

bruceg · 2026-02-05T16:58:24Z

+                let orig_count = *count;
+                if orig_count > 0 {
+                    sketch.sum = orig_sum;
+                    sketch.count = orig_count as u32;


If *count is ever larger than a u32 can hold, I think we would want to leave the sketch.count as it is, and possibly not set sum or avg too. Does that make sense? If so, we'd want to do something along the lines of:

if let Some(count) = *count.try_into() { sketch.count = count; }

yes makes total sense, just pushed an update. it seems like it doesn't require the clippy notes anymore when we do it that way as make check-clippy passes without them now. 👍

bruceg

LGTM

Co-authored-by: Bruce Guenter <bruce@untroubled.org>

pront · 2026-02-17T17:04:50Z

It's also interesting that this didn't affect any existing tests. I wonder if this indicates a testing gap.

tony-resendiz · 2026-02-23T22:03:52Z

It's also interesting that this didn't affect any existing tests. I wonder if this indicates a testing gap.

@pront good call i added a test to check the resulting sum, count, average of the sketch.

pront · 2026-04-17T16:56:27Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d93e2596dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-17T17:01:26Z

+                            sketch.sum = *sum;
+                            sketch.avg = *sum / f64::from(c);


Skip overriding avg when histogram sum may be unknown

Overwriting sketch.sum/sketch.avg from sum / count here can zero out valid histograms when upstream sum is absent. OpenTelemetry histogram sum is optional, and our OTLP conversion currently does sum: self.point.sum.unwrap_or(0.0) (see lib/opentelemetry-proto/src/metrics.rs), so a non-empty histogram with missing sum will now be emitted with avg=0 after this change. Before this commit, the sketch average came from bucket interpolation and remained non-zero for those inputs. Please gate this override on a known sum value (or preserve interpolated stats as fallback).

Useful? React with 👍 / 👎.

@codex address that feedback

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 98a1bda6de

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-04-23T23:19:45Z

+                match u32::try_from(*count) {
+                    Ok(c) => {
+                        if c > 0 {
+                            sketch.count = c;


Preserve sketch/bin count consistency when overriding histogram stats

Setting sketch.count directly from histogram count here can break DDSketch invariants for valid inputs where bucket totals differ from count (for example, Prometheus parsing drops the +Inf bucket in src/sources/prometheus/parser.rs lines 133-153 but preserves total count). In that case cnt becomes larger than the total mass represented by n, so later operations that rely on count-vs-bin consistency (e.g. AgentDDSketch::quantile) can return incorrect values/NaN, and encoded sketch payloads carry internally inconsistent statistics.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 371280abaf

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-02T17:02:35Z

+                        if c > 0 && *sum != 0.0 {
+                            sketch.count = c;
+                            sketch.sum = *sum;
+                            sketch.avg = *sum / f64::from(c);


Skip sum/count override when sketch has no interpolated bins

This block overwrites count/sum/avg whenever count > 0 && sum != 0, even if insert_interpolate_buckets inserted nothing. That can happen for valid OTLP histograms that carry only count/sum and no bucket counts (lib/opentelemetry-proto/src/metrics.rs builds buckets: Vec::new() when bucket_counts.len() == 0). In that path, min/max remain at their initialization sentinels and bins stay empty, but the sketch is now treated as non-empty and later encoded, producing corrupted sketch statistics instead of dropping or safely handling the metric.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90450b23ff

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-02T17:15:31Z

+                        if c > 0 && *sum != 0.0 && has_buckets {
+                            sketch.count = c;
+                            sketch.sum = *sum;
+                            sketch.avg = *sum / f64::from(c);


Require non-empty sketch before overriding summary stats

This branch updates count/sum/avg based only on has_buckets, but has_buckets only checks vector length, not whether any bucket actually contributed samples. If a histogram arrives with bucket entries whose counts are all zero (or otherwise fail to add samples) while count is positive, this code makes the sketch non-empty with count > 0 but leaves min/max at their defaults (f64::MAX/f64::MIN). The Datadog encoder then emits these invalid extrema for a non-empty sketch. Gate this override on actual inserted samples (for example sketch.count() > 0) rather than bucket vector non-emptiness.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 647916b816

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-02T17:23:27Z

+                match u32::try_from(*count) {
+                    Ok(c) => {
+                        if c > 0 && *sum != 0.0 && sketch.count() > 0{
+                            sketch.count = c;


Preserve bin-derived count when overriding histogram average

Do not overwrite sketch.count from the histogram count field here. The interpolated bins already define the sketch's sample mass, and AgentDDSketch::quantile uses self.count to compute rank; if upstream sends a histogram where count and bucket totals differ (this type is accepted without validation), the rank can exceed cumulative bin weights and produce NaN/skewed quantiles. Overriding sum/avg is sufficient for the accuracy fix while keeping count consistent with the bins.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 52d0ee4e15

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-02T17:38:57Z

+                            sketch.sum = *sum;
+                            sketch.avg = *sum / f64::from(c);


Guard sum/count override on bucket-count mismatch

This branch overwrites sketch.sum/sketch.avg using histogram-wide sum and count, but sketch.count is still derived only from interpolated buckets. That creates inconsistent sketches when input histograms keep total count/sum larger than represented buckets (for example, Prometheus parsing drops the +Inf bucket in src/sources/prometheus/parser.rs lines 133-138 while preserving count/sum at lines 152-153). In that case we emit cnt from sketch.count but avg/sum from full totals, so downstream receives contradictory stats for the same sketch.

Useful? React with 👍 / 👎.

@codex Good point — this can introduce inconsistencies between sum/avg and the bin-derived count when upstream histogram totals differ from bucket mass (e.g. Prometheus +Inf bucket handling).

In this change, the intent is to improve summary statistic accuracy (avg) while preserving DDSketch invariants for quantile calculations by keeping count aligned with bins.

We accept the potential inconsistency (avg != sum / count) as a tradeoff, since sum/avg are not used in quantile computation and this improves accuracy for common cases.

To use Codex here, create a Codex account and connect to github.

@codex review

thanks @codex you're awesome

To use Codex here, create a Codex account and connect to github.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5cfc57f701

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-02T17:56:19Z

+                        if c > 0 && *sum != 0.0 && sketch.count() > 0{
+                            sketch.sum = *sum;
+                            sketch.avg = *sum / f64::from(c);


Keep sketch avg aligned with retained sketch count

This branch sets avg from the histogram-level count while deliberately not updating sketch.count, which creates an inconsistent sketch whenever those counts differ. That mismatch is real for Prometheus histograms because src/sources/prometheus/parser.rs drops the +Inf bucket (133-138) but still preserves count (152), so avg can be computed against a larger denominator than the sketch actually stores. Once that happens, later aggregation via MetricValue::add -> AgentDDSketch::merge weights averages by self.count, producing incorrect merged averages that are sent in the Datadog sketch payload.

Useful? React with 👍 / 👎.

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7a840552fa

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-02T18:09:28Z

+                        let sc = sketch.count();
+
+                        if c > 0 && *sum != 0.0 && sc > 0 {
+                            sketch.sum = *sum;
+                            sketch.avg = *sum / sc as f64;


Divide histogram sum by histogram count when overriding avg

When histogram count differs from the interpolated bucket total, this computes avg as sum / sketch.count() rather than sum / 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 misreport avg downstream.

Useful? React with 👍 / 👎.

tony-resendiz · 2026-05-02T18:33:07Z

ok, based on the feedback from chatgpt and codex, it seems like this change would be pretty bad. the ddsketch algorithm is merging histograms/sketches. if we are not preserving the interpolated values (count, avg) in the ddsketch then it can corrupt merging and make matters worse. so the accuracy is a tradeoff for merge-ability. 😮 🤯

gwenaskell · 2026-07-09T13:38:57Z

related #25752

tony-resendiz requested a review from a team as a code owner December 13, 2025 00:21

github-actions Bot added the domain: core Anything related to core crates i.e. vector-core, core-common, etc label Dec 13, 2025

tony-resendiz changed the title ~~enhancement(ddsketch): averages are calculated from histogram sum & count fields~~ enhancement(metrics): ddsketch averages are calculated from histogram sum & count fields Dec 13, 2025

tony-resendiz added 2 commits December 15, 2025 11:38

update some ddsketch fields from histogram

31d3db0

add changelog fragment

be24867

tony-resendiz force-pushed the ddsketch-averages branch from 0fbff1c to be24867 Compare December 15, 2025 19:40

tony-resendiz mentioned this pull request Dec 15, 2025

ddsketch from histogram seems inaccurate #24365

Closed

formatting

3911f2e

Merge branch 'vectordotdev:master' into ddsketch-averages

70399f7

bruceg changed the title ~~enhancement(metrics): ddsketch averages are calculated from histogram sum & count fields~~ enhancement(metrics): calculate ddsketch averages from histogram sum & count fields Feb 5, 2026

bruceg added type: bug domain: metrics Anything related to Vector's metrics events labels Feb 5, 2026

bruceg changed the title ~~enhancement(metrics): calculate ddsketch averages from histogram sum & count fields~~ fix(metrics): calculate ddsketch averages from histogram sum & count fields Feb 5, 2026

bruceg reviewed Feb 5, 2026

View reviewed changes

don't modify the sketch if count is bigger than u32

479e10c

bruceg previously approved these changes Feb 6, 2026

View reviewed changes

Comment thread lib/vector-core/src/metrics/ddsketch.rs Outdated

Update lib/vector-core/src/metrics/ddsketch.rs

38c8605

Co-authored-by: Bruce Guenter <bruce@untroubled.org>

pront reviewed Feb 17, 2026

View reviewed changes

Comment thread lib/vector-core/src/metrics/ddsketch.rs Outdated

pront reviewed Feb 17, 2026

View reviewed changes

Comment thread lib/vector-core/src/metrics/ddsketch.rs Outdated

pront reviewed Feb 17, 2026

View reviewed changes

Comment thread changelog.d/dd_sketch_averages_from_histograms.fix.md Outdated

pront removed the type: bug label Feb 21, 2026

pr feedback

cffc82c

pr checks

2068dc0

github-actions Bot added the domain: ci Anything related to Vector's CI environment label Feb 23, 2026

tony-resendiz dismissed bruceg’s stale review via 2068dc0 March 16, 2026 14:24

Merge branch 'master' into ddsketch-averages

d93e259

chatgpt-codex-connector Bot reviewed Apr 17, 2026

View reviewed changes

pront reviewed Apr 17, 2026

View reviewed changes

Comment thread changelog.d/dd_sketch_averages_from_histograms.fix.md Outdated

update changelog.d

98a1bda

chatgpt-codex-connector Bot reviewed Apr 23, 2026

View reviewed changes

feedback

371280a

chatgpt-codex-connector Bot reviewed May 2, 2026

View reviewed changes

feedback

90450b2

chatgpt-codex-connector Bot reviewed May 2, 2026

View reviewed changes

feedback

647916b

chatgpt-codex-connector Bot reviewed May 2, 2026

View reviewed changes

feedback

52d0ee4

chatgpt-codex-connector Bot reviewed May 2, 2026

View reviewed changes

feedback

5cfc57f

chatgpt-codex-connector Bot reviewed May 2, 2026

View reviewed changes

feedback

7a84055

chatgpt-codex-connector Bot reviewed May 2, 2026

View reviewed changes

tony-resendiz closed this May 2, 2026

github-actions Bot locked and limited conversation to collaborators May 2, 2026

Uh oh!

Conversation

tony-resendiz commented Dec 13, 2025

Summary

Vector configuration

How did you test this PR?

Change Type

Is this a breaking change?

Does this PR include user facing changes?

References

Notes

Uh oh!

github-actions Bot commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tony-resendiz commented Dec 13, 2025

Uh oh!

tony-resendiz commented Jan 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bruceg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pront commented Feb 17, 2026

Uh oh!

Uh oh!

tony-resendiz commented Feb 23, 2026

Uh oh!

pront commented Apr 17, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 17, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Dec 13, 2025 •

edited

Loading