Skip to content

Conversation

@dashpole
Copy link
Contributor

Description

Update the opentelemetry instrumentation sample to use exponential histograms and update the collector image version. This is the same as GoogleCloudPlatform/opentelemetry-operations-python#364 and GoogleCloudPlatform/opentelemetry-operations-java#387.

Tested by running the quickstart myself and verifying the metrics, traces and logs.

Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.

Checklist

  • I have followed Contributing Guidelines from CONTRIBUTING.MD
  • Tests pass: go test -v ./.. (see Testing)
  • Code formatted: gofmt (see Formatting)
  • Vetting pass: go vet (see Formatting)
  • These samples need a new API enabled in testing projects to pass (let us know which ones)
  • These samples need a new/updated env vars in testing projects set to pass (let us know which ones)
  • This sample adds a new sample directory, and I updated the CODEOWNERS file with the codeowners for this sample
  • This sample adds a new Product API, and I updated the Blunderbuss issue/PR auto-assigner with the codeowners for this sample
  • Please merge this PR for me once it is approved

@dashpole dashpole requested review from a team as code owners January 14, 2025 18:18
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will provide my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly understand the changes:

This pull request updates the OpenTelemetry instrumentation sample to use exponential histograms and upgrades the collector image version. The changes are based on similar updates made in the Python and Java OpenTelemetry repositories.

Intent: The primary goal is to improve the accuracy and granularity of metrics collected by the OpenTelemetry instrumentation sample by switching to exponential histograms. Additionally, updating the collector image ensures compatibility and access to the latest features and bug fixes.

Changes:

  • opentelemetry/instrumentation/docker-compose.yaml: Updated the OpenTelemetry collector image version from 0.105.0 to 0.115.1 (lines 15-16). Added environment variable OTEL_EXPORTER_OTLP_METRICS_DEFAULT_HISTOGRAM_AGGREGATION=BASE2_EXPONENTIAL_BUCKET_HISTOGRAM to enable exponential histograms (lines 9-10).
  • opentelemetry/instrumentation/otel-collector-config.yaml: Added a Prometheus exporter configuration to expose metrics (lines 93-100). This allows for easier monitoring and visualization of the collected metrics.

Testing: The author reports that they tested the changes by running the quickstart and verifying metrics, traces, and logs. However, further review and testing are recommended.

I'll provide a more detailed review shortly. Thanks!


A little haiku to lighten the mood:

Histograms grow,
Data points, a clearer view,
Insights now unfold.

@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Jan 14, 2025
@dashpole dashpole force-pushed the otel_exponential_histograms branch from 67e4ec3 to 58c463f Compare January 14, 2025 18:21
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This pull request updates the OpenTelemetry sample to use exponential histograms and a newer collector image. The changes are straightforward and well-organized. The addition of the Prometheus exporter enhances metrics visualization and monitoring capabilities. The provided documentation and testing efforts ensure the changes are robust and reliable.

@iennae iennae changed the title Update opentelemetry sample to use exponential histograms fix: Update opentelemetry sample to use exponential histograms Jan 14, 2025
@dashpole dashpole requested a review from psx95 January 14, 2025 19:19
@iennae iennae merged commit 73b45f3 into GoogleCloudPlatform:main Jan 14, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

samples Issues that are directly related to samples.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants