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: update otel versions for prometheus to 0.27 #2309

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

kudlatyamroth
Copy link

@kudlatyamroth kudlatyamroth commented Nov 20, 2024

Changes

  • Update opentelemetry dependency version to 0.27
  • Update opentelemetry_sdk dependency version to 0.27
  • Update opentelemetry-semantic-conventions dependency version to 0.27

Merge requirement checklist

  • CONTRIBUTING guidelines followed
  • Unit tests added/updated (if applicable)
  • Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
  • Changes in public API reviewed (if applicable)

@kudlatyamroth kudlatyamroth requested a review from a team as a code owner November 20, 2024 14:45
@kudlatyamroth kudlatyamroth force-pushed the feature/update-prometheus-to-otel-27 branch from 4800dd6 to eda6bca Compare November 20, 2024 14:48
Copy link

codecov bot commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 8 lines in your changes missing coverage. Please review.

Project coverage is 79.5%. Comparing base (506a4f9) to head (ce41608).

Files with missing lines Patch % Lines
opentelemetry-prometheus/src/lib.rs 63.6% 8 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #2309     +/-   ##
=======================================
- Coverage   79.5%   79.5%   -0.1%     
=======================================
  Files        123     123             
  Lines      21479   21483      +4     
=======================================
  Hits       17084   17084             
- Misses      4395    4399      +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kudlatyamroth kudlatyamroth force-pushed the feature/update-prometheus-to-otel-27 branch from eda6bca to 13ca8df Compare November 20, 2024 15:11
@kudlatyamroth kudlatyamroth force-pushed the feature/update-prometheus-to-otel-27 branch from 13ca8df to 984b0d8 Compare November 20, 2024 17:40
Copy link
Member

@cijothomas cijothomas left a comment

Choose a reason for hiding this comment

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

Changes look good. I am requesting changes to fix the internal logs. These are now emitted via tracing and most users likely have a tracing subscriber already enabled.. So we need to be very careful about emitting logs, especially the ones with high severity.
Also, try and make the descriptions more self-explanatory.(Totally understand these are existing issues, but just want to make it easier for end users to navigate the internal logs)

kudlatyamroth and others added 5 commits November 21, 2024 12:04
Enhanced error messages with specific names and differentiated warnings. Updated temporality method for Prometheus to always return cumulative temporality, aligning with Prometheus behavior.
Updated several error log names in `opentelemetry-prometheus/src/lib.rs` for better clarity and consistency. This includes renaming `AcquireLockError`, `ReaderError`, and `MetricValidation` to `MetricScrapeFailed` and `MetricValidationFailed`.
@cijothomas cijothomas requested a review from hdost December 2, 2024 20:02
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.

5 participants