Skip to content

fix(datadog metrics sink): fix metric type comparison using wrong operand in sort#25621

Open
gwenaskell wants to merge 1 commit into
masterfrom
yoenn.burban/OPA-5665-datadog-metrics-sorting-error
Open

fix(datadog metrics sink): fix metric type comparison using wrong operand in sort#25621
gwenaskell wants to merge 1 commit into
masterfrom
yoenn.burban/OPA-5665-datadog-metrics-sorting-error

Conversation

@gwenaskell

Copy link
Copy Markdown
Contributor

Summary

In sort_and_collapse_counters_by_series_and_timestamp, the second tuple of the .cmp() call was incorrectly using a.value().as_name() instead of b.value().as_name(). This caused the metric type name (counter/gauge/histogram) to always compare equal to itself, making it a no-op as a sort key. The sort was effectively only ordering by series() and timestamp(), not by metric type name as intended.

Vector configuration

No configuration change — this is an internal sink implementation fix.

How did you test this PR?

Existing unit tests in sort_and_collapse_counters_by_series_and_timestamp cover this code path.

Change Type

  • Bug fix
  • New feature
  • Dependencies
  • 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

Made with Cursor

@gwenaskell gwenaskell requested a review from a team as a code owner June 15, 2026 08:30
@github-actions github-actions Bot added the domain: sinks Anything related to the Vector's sinks label Jun 15, 2026
@gwenaskell gwenaskell requested a review from bruceg June 15, 2026 13:52

@pront pront left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch! Please add a changelog entry.

@bruceg

bruceg commented Jun 15, 2026

Copy link
Copy Markdown
Member

Existing unit tests in sort_and_collapse_counters_by_series_and_timestamp cover this code path.

@gwenaskell Please clarify. If the unit tests did cover this typo, it should have been failing to start with, so it would seem this code path is not actually covered.

@bruceg bruceg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The fix looks correct but it needs a coverage test and changelog entry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: sinks Anything related to the Vector's sinks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants