Skip to content
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

[pkg/otlp] Fix cumulative histogram handling in distributions mode #9623

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

KSerrania
Copy link
Contributor

@KSerrania KSerrania commented Oct 22, 2021

What does this PR do?

Backport of open-telemetry/opentelemetry-collector-contrib#5867 to the pkg/otlp package.

Fixes an issue in mapHistogramMetrics where the delta boolean wouldn't be passed to getSketchBuckets when the histograms mode is set to distributions.
Fixes an issue in getSketchBuckets where buckets were not tagged by bounds when getting and setting values in the points cache (in the cumulative histogram case). This led to all buckets of an histogram sharing the same cache key, making the resulting sketch wrong.
Fixes the TestNaNMetrics histogram case, which was missing its ExplicitBounds definition.
Adds missing unit tests for the distributions mode.

Checklist

  • A release note has been added or the changelog/no-changelog label has been applied.
  • The need-change/operator and need-change/helm labels has been applied if applicable.
  • The appropriate team/.. label has been applied, if known.
  • If known, an appropriate milestone has been selected; otherwise the Triage milestone is set.
  • The config template has been updated if applicable.

@KSerrania KSerrania marked this pull request as ready for review October 22, 2021 09:06
@KSerrania KSerrania requested a review from a team as a code owner October 22, 2021 09:06
@KSerrania KSerrania merged commit b1b898a into main Oct 22, 2021
@KSerrania KSerrania deleted the kserrania/backport-otel-cumulative-histograms branch October 22, 2021 09:27
KSerrania added a commit that referenced this pull request Oct 22, 2021
…9623)

Backport of open-telemetry/opentelemetry-collector-contrib#5867 to the `pkg/otlp` package.

Fixes an issue in `mapHistogramMetrics` where the `delta` boolean wouldn't be passed to `getSketchBuckets` when the histograms mode is set to distributions.
Fixes an issue in `getSketchBuckets` where buckets were not tagged by bounds when getting and setting values in the points cache (in the cumulative histogram case). This led to all buckets of an histogram sharing the same cache key, making the resulting sketch wrong.
Fixes the `TestNaNMetrics` histogram case, which was missing its `ExplicitBounds` definition.
Adds missing unit tests for the distributions mode.
KSerrania added a commit that referenced this pull request Oct 25, 2021
…9623) (#9624)

Backport of #9623 to the `7.32.x` branch.

Fixes an issue in `mapHistogramMetrics` where the `delta` boolean wouldn't be passed to `getSketchBuckets` when the histograms mode is set to distributions.
Fixes an issue in `getSketchBuckets` where buckets were not tagged by bounds when getting and setting values in the points cache (in the cumulative histogram case). This led to all buckets of an histogram sharing the same cache key, making the resulting sketch wrong.
Fixes the `TestNaNMetrics` histogram case, which was missing its `ExplicitBounds` definition.
Adds missing unit tests for the distributions mode.
@mx-psi mx-psi mentioned this pull request Oct 26, 2021
5 tasks
@mx-psi mx-psi added the component/otlp PRs and issues related to OTLP ingest label Feb 17, 2022
zandrewitte pushed a commit to StackVista/stackstate-agent that referenced this pull request Nov 17, 2022
…ataDog#9623)

Backport of open-telemetry/opentelemetry-collector-contrib#5867 to the `pkg/otlp` package.

Fixes an issue in `mapHistogramMetrics` where the `delta` boolean wouldn't be passed to `getSketchBuckets` when the histograms mode is set to distributions.
Fixes an issue in `getSketchBuckets` where buckets were not tagged by bounds when getting and setting values in the points cache (in the cumulative histogram case). This led to all buckets of an histogram sharing the same cache key, making the resulting sketch wrong.
Fixes the `TestNaNMetrics` histogram case, which was missing its `ExplicitBounds` definition.
Adds missing unit tests for the distributions mode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants