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

feat(exporter-metrics-otlp-*): configure histogram aggregation #5061

Conversation

olabodeIdowu
Copy link

@olabodeIdowu olabodeIdowu commented Oct 10, 2024

Which problem is this PR solving?

Fixes 3920

Short description of the changes

To implement default histogram aggregation for OTLP Metrics exporters, you can use the OpenTelemetry library. The sample code snippet demonstrates how to allow for either Explicit Bucket histogram aggregation or Exponential histogram aggregation based on an environment variable.

Explanation:
  1. Environment Variable: The OTLP_HISTOGRAM_AGGREGATION environment variable is set to either 'explicit' or 'exponential'.
  2. Histogram Aggregation Configuration:
    • For Explicit Bucket, boundaries are defined.
    • For Exponential, a scale factor and bounds are defined.
  3. Metric Reader: A PeriodicExportingMetricReader is set up to export metrics at a specified interval.
  4. Meter Provider: The global meter provider is set to start collecting and exporting metrics.
  5. Histogram Metric Creation: A histogram metric is created based on the chosen aggregation type, and sample values are recorded.
Usage:
  • Change the value of process.env.OTLP_HISTOGRAM_AGGREGATION before running the code to select the desired histogram aggregation method.
  • Adjust the bucket boundaries and export interval according to your needs.

Type of change

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Recorded some values in the histogram
Checklist:

  • Followed the style guidelines of this project
  • Unit tests have been added
  • Documentation has been updated

@olabodeIdowu olabodeIdowu requested a review from a team as a code owner October 10, 2024 12:35
Copy link

linux-foundation-easycla bot commented Oct 10, 2024

CLA Not Signed

@@ -71,6 +71,7 @@
},
"dependencies": {
"@opentelemetry/api-logs": "0.53.0",
"@opentelemetry/sdk-logs": "^0.53.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this change intentional? I don't see any usage of the sdk-logs package in the example above.

@pichlermarc
Copy link
Member

@olabodeIdowu thanks for taking the time to work on this, but I think like this PR missed the original intent of what the author of #3920 wanted to be implemented (see the linked specification on #3920). This is adding an example not the actual feature. Many of our issues are not yet well defined, so if anything is unclear, please ask questions on the issues first, and then ask to be assigned to issues once you're ready to start working on it.

While this is not exciting work that involves coding, asking well thought out questions is still very important as this will save everyone a lot of time in the long run:

  • requirements will get clearer, regardless of you picking up coding work on the issue or not
  • if you comment on the issue and wait to get assigned until you start working, this signals others that someone else is doing this work already, preventing duplicate work on the same issue
  • approvers/maintainers will be quicker reviewing the PR when when we don't have to figure out what needs to be done when a PR is already open.

Please also note that all issues labelled feature-request on them are just that, requests. A feature request does not indicate anything other than that someone asked for it to be implemented. So: if in doubt, please ask first. 🙂

@olabodeIdowu
Copy link
Author

@olabodeIdowu thanks for taking the time to work on this, but I think like this PR missed the original intent of what the author of #3920 wanted to be implemented (see the linked specification on #3920). This is adding an example not the actual feature. Many of our issues are not yet well defined, so if anything is unclear, please ask questions on the issues first, and then ask to be assigned to issues once you're ready to start working on it.

While this is not exciting work that involves coding, asking well thought out questions is still very important as this will save everyone a lot of time in the long run:

  • requirements will get clearer, regardless of you picking up coding work on the issue or not
  • if you comment on the issue and wait to get assigned until you start working, this signals others that someone else is doing this work already, preventing duplicate work on the same issue
  • approvers/maintainers will be quicker reviewing the PR when when we don't have to figure out what needs to be done when a PR is already open.

Please also note that all issues labelled feature-request on them are just that, requests. A feature request does not indicate anything other than that someone asked for it to be implemented. So: if in doubt, please ask first. 🙂

alright. thank you @pichlermarc

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.

Implement OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION envvar
3 participants