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

[exporter/datadog] Add support for delta exponential histograms #8350

Merged
merged 25 commits into from
May 6, 2022

Conversation

KSerrania
Copy link
Contributor

@KSerrania KSerrania commented Mar 9, 2022

Description:

Adds support for exporting delta exponential histograms as sketches to Datadog. The conversion is done as follows:
ExponentialHistogram -> DDSketch (done here by copying the buckets, with the same scale)
DDSketch -> Agent Sketch (done by github.com/DataDog/datadog-agent/pkg/quantile, in this PR)

Optionally, the count and sum of the exponential histogram can also be sent as metrics. This behavior is managed by the histograms.send_count_sum_metrics (the same that is used for histograms - should it be a different one?).

Cumulative histograms are left out for now (they'll require more involved changes, notably to compute the diff between two exponential histograms) and will be done in a later PR.

Note: this PR currently used unreleased versions of github.com/DataDog/sketches-go and github.com/DataDog/datadog-agent/pkg/quantile that will be released at a later date. Once this happens, the go.mod should point to released versions.

Link to tracking Issue: n/a

Testing: Added unit tests, as well as benchmarks for gauges, delta sums and delta exponential histograms, for comparison purposes.

Documentation: n/a

@KSerrania KSerrania force-pushed the kserrania/exponentialhistogram branch 2 times, most recently from 887006a to de2deb1 Compare March 10, 2022 09:37
@KSerrania KSerrania force-pushed the kserrania/exponentialhistogram branch from de2deb1 to f654ed7 Compare March 10, 2022 09:56
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

Looks good (pending merging the other PR on datadog-agent and adding a changelog entry)

@mx-psi
Copy link
Member

mx-psi commented Mar 16, 2022

There are a bunch of conflicts that need to be solved here

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Mar 31, 2022
@mx-psi mx-psi removed the Stale label Mar 31, 2022
@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Apr 19, 2022
@mx-psi mx-psi removed the Stale label Apr 19, 2022
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

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

:shipit:

@mx-psi
Copy link
Member

mx-psi commented May 3, 2022

Failing checks should be solved by either #9695 or #9694

@mx-psi
Copy link
Member

mx-psi commented May 5, 2022

If you mark it as ready for review, I think we can mark this as ready to merge 🎉

@KSerrania KSerrania marked this pull request as ready for review May 5, 2022 09:52
@KSerrania KSerrania requested review from a team and djaglowski May 5, 2022 09:52
@mx-psi mx-psi added the ready to merge Code review completed; ready to merge by maintainers label May 5, 2022
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Please resolve conflicts and we'll get this reviewed/merged!

@codeboten codeboten merged commit 4990177 into open-telemetry:main May 6, 2022
djaglowski pushed a commit to djaglowski/opentelemetry-collector-contrib that referenced this pull request May 10, 2022
…-telemetry#8350)

* [WIP] Add ExponentialHistogram support

* Do not support cumulative expHists for now

* Add unit tests

* Update go.mod / go.sum

* Fix rebase conflicts

* Add MapMetrics tests

* Fix go mod tidy errors

* Update github.com/DataDog/datadog-agent/pkg/quantile

* Refactor store creation in toStore method

* Refactor tests to use public MapMetrics function

* Add benchmarks

* Update DataDog/datadog-agent/pkg/quantile version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants