-
Notifications
You must be signed in to change notification settings - Fork 1.9k
cmd/mdatagen: introduce additional metadata for deprecated metrics #14408
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: main
Are you sure you want to change the base?
cmd/mdatagen: introduce additional metadata for deprecated metrics #14408
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (70.27%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #14408 +/- ##
==========================================
+ Coverage 91.80% 91.81% +0.01%
==========================================
Files 677 677
Lines 42608 42700 +92
==========================================
+ Hits 39116 39207 +91
- Misses 2429 2434 +5
+ Partials 1063 1059 -4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CodSpeed Performance ReportMerging this PR will not alter performanceComparing
|
|
Note: generate-contrib failures are expected. This PR enforces the metadata schema by disallowing A follow-up issue will be opened in opentelemetry-collector-contrib. |
d43516d to
1aef5c5
Compare
64fdd62 to
93e470f
Compare
93e470f to
64fdd62
Compare
Co-authored-by: Damien Mathieu <42@dmathieu.com>
dmathieu
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 is starting to look nice 😄
Co-authored-by: Damien Mathieu <42@dmathieu.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>
Thanks! Please let me know if any further changes are needed. |
|
@Syedowais312 there is one open comment left 😺 |
@dmathieu thanks for the clarification. I’ve resolved the comment. |
Removed note about logging limitations in metric configuration.
ChrsMark
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.
LGTM
| # Optional: the stability of the metric. Set to alpha by default. | ||
| stability: | ||
| # Optional: the stability level of the metric. Set to alpha by default. | ||
| level: [alpha|stable|deprecated] |
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.
Hmm, interesting that it was flattened already 🤔 .
We will handle the flattening in follow-ups in any case, but the levels here should be <development|alpha|beta|stable|deprecated> based on https://opentelemetry.io/docs/collector/internal-telemetry/#metrics.
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.
IMHO, adding that should be its own PR though.
|
Ping @dmitryax for review, since you created the original issue. |
This PR fixes how
mdatagenhandles deprecated metric fields inmetadata.yaml.The metadata loader now aligns correctly with the schema and test expectations,
ensuring deprecated fields are only set when explicitly defined.
What was changed?
mdatagentests pass after this changeTesting
go test ./cmd/mdatagen/...mdatagentests pass.Related: #14113
N/A (internal correctness and test alignment fix)