-
Notifications
You must be signed in to change notification settings - Fork 398
chore(otel): fix telemetry collection for otel metrics #5138
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
base: master
Are you sure you want to change the base?
Conversation
|
Thank you for updating Change log entry section 👏 Visited at: 2025-12-17 19:59:20 UTC |
BenchmarksBenchmark execution time: 2025-12-18 19:16:29 Comparing candidate commit 0db7126 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 44 metrics, 2 unstable metrics. |
6e1aac5 to
77fbfb0
Compare
|
✅ Tests 🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage 🔗 Commit SHA: 0db7126 | Docs | Datadog PR Page | Was this helpful? Give us feedback! |
y9v
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR fixes a bug, but there is not a single test here, which verifies that the bug is fixed now. Would it be possible to add tests?
I don't want to block the PR since I am planning to take some days off
I linked the the system tests for this change in the PR description. I can duplicate these tests in dd-trace-rb but I think that reduces the value of having shared tests. I prefer not writing the same test cases multiple times but I can make this change if it is required to merge this PR |
Sorry, I missed the link to system-tests PR. It's up to you, but I would suggest to add at least one test here - simply because it makes it easier to spot a mistake early on in the future. If I am working on the library code, I run the tests we have, but not necessarily system tests (which are more like a contract). |
383242e to
2806353
Compare
Yup, I can do that. This change only impacts instrumentation telemetry. I'll add a test cases that captures all supported configs and the success metrics |
|
@mabdinur Can you please adjust the changelog section of the PR to be compliant with the requirements stated in the first comment above? |
Typing analysisNote: Ignored files are excluded from the next sections. Untyped methodsThis PR introduces 1 partially typed method, and clears 1 partially typed method. It increases the percentage of typed methods from 55.78% to 55.8% (+0.02%). Partially typed methods (+1-1)❌ Introduced:If you believe a method or an attribute is rightfully untyped or partially typed, you can add |
What does this PR do?
Ensures configurations used in OpenTelemetry Metrics SDK are reported to the instrumentation telemetry platform. Updates configuration fallback logic to properly detect when metrics-specific configs use default values and fall back to general OTLP exporter configs per OpenTelemetry spec.
Motivation:
Emit diagnostic data about the state of the OpenTelemetry Components.
Change log entry
None.
Additional Notes:
How to test the change?
Regression was caught by system tests. This update to system tests will be merged after this PR: DataDog/system-tests#5811