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

[receiver/statsd] Add an exponential histogram option #12951

Merged
merged 52 commits into from Oct 14, 2022

Conversation

jmacd
Copy link
Contributor

@jmacd jmacd commented Aug 4, 2022

Description:
Adds a "histogram" option to enable the OTLP v0.11 auto-scaling exponential histogram aggregator for Timing and Histogram measurements.

This uses the Histogram mapping functions and data structure implemented in github.com/lightstep/go-expohisto.

The same mapping functions and data structure are pending as for OTel-Go (e.g., open-telemetry/opentelemetry-go#3022); if OTel-Go agrees to expose these APIs, this module can be updated to use it directly, see:

open-telemetry/opentelemetry-go#3169
open-telemetry/opentelemetry-go#3170

Link to tracking Issue:
Fixes #5742

Testing: New tests added.

Documentation: README.md updated with examples including histogram configuration.

mx-psi
mx-psi previously requested changes Aug 4, 2022
receiver/statsdreceiver/go.mod Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser_test.go Outdated Show resolved Hide resolved
@mx-psi mx-psi changed the title Add an exponential histogram option to statsdreceiver [receiver/statsd] Add an exponential histogram option Aug 4, 2022
@mx-psi mx-psi dismissed their stale review August 5, 2022 15:33

We now can use Go 1.18 features!

@mx-psi
Copy link
Member

mx-psi commented Aug 5, 2022

This probably needs a rebase

CHANGELOG.md Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
receiver/statsdreceiver/README.md Outdated Show resolved Hide resolved
receiver/statsdreceiver/config_test.go Outdated Show resolved Hide resolved
receiver/statsdreceiver/go.mod Outdated Show resolved Hide resolved
receiver/statsdreceiver/testdata/config.yaml Outdated Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser_test.go Outdated Show resolved Hide resolved
@jmacd jmacd marked this pull request as draft August 16, 2022 07:42
@jmacd
Copy link
Contributor Author

jmacd commented Aug 16, 2022

Will leave this in draft until the testbed refactoring is complete.

@jmacd
Copy link
Contributor Author

jmacd commented Oct 5, 2022

This PR has been updated to use a new, separate copy of Lightstep's OTel exponential histogram mapping functions and data structure. Please take another look!

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.

Hopefully my last round of comments, generally looks great :)

receiver/statsdreceiver/factory_test.go Show resolved Hide resolved
receiver/statsdreceiver/protocol/metric_translator.go Outdated Show resolved Hide resolved
receiver/statsdreceiver/protocol/metric_translator.go Outdated Show resolved Hide resolved
receiver/statsdreceiver/config.go Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser_test.go Outdated Show resolved Hide resolved
receiver/statsdreceiver/protocol/statsd_parser_test.go Outdated Show resolved Hide resolved
Copy link
Contributor Author

@jmacd jmacd left a comment

Choose a reason for hiding this comment

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

This hasn't been updated for the v0.62 changes yet, otherwise mostly addressed your feedback @mx-psi/

receiver/statsdreceiver/protocol/metric_translator.go Outdated Show resolved Hide resolved
receiver/statsdreceiver/config.go Show resolved Hide resolved
receiver/statsdreceiver/factory_test.go Show resolved Hide resolved
receiver/statsdreceiver/protocol/metric_translator.go Outdated Show resolved Hide resolved
@jmacd
Copy link
Contributor Author

jmacd commented Oct 13, 2022

Updated for v0.62.0.

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.

LGTM, my only pending question is #12951 (comment) (also one small thing missing from the v0.62.0 bump).

I will merge after you reply @jmacd if @codeboten has no further comments

receiver/statsdreceiver/go.mod Outdated Show resolved Hide resolved
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
@codeboten codeboten merged commit a2f9e12 into open-telemetry:main Oct 14, 2022
@jmacd jmacd deleted the jmacd/statsd_receiver branch October 14, 2022 15:56
shalper2 pushed a commit to shalper2/opentelemetry-collector-contrib that referenced this pull request Dec 6, 2022
…#12951)

Adds a "histogram" option to enable the OTLP v0.11 auto-scaling exponential histogram aggregator for Timing and Histogram measurements.

Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
@plantfansam plantfansam mentioned this pull request Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace off-spec statsdreceiver Summary usage with new exponential histogram
5 participants