-
Notifications
You must be signed in to change notification settings - Fork 482
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
Initial attempt at adding metric semconvs. #1918
Initial attempt at adding metric semconvs. #1918
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1918 +/- ##
=====================================
Coverage 75.0% 75.0%
=====================================
Files 122 122
Lines 20290 20290
=====================================
Hits 15221 15221
Misses 5069 5069 ☔ View full report in Codecov by Sentry. |
The crate is pre-release (with no planned stable release date as of today), so don't worry too much about back compatibility. Feel free to do the correct thing, without feeling burdened by the need of back-compatbility for this crate. |
Oh awesome, I wasn't really aware of the current release status, thanks for the heads up! Changes made to remove those attributes from |
Current CI failures are unrelated: error: package `cc v1.1.0` cannot be built because it requires rustc 1.67 or newer, while the currently active rustc version is 1.65.0
Either upgrade to rustc 1.67 or newer, or use
cargo update -p [email protected] --precise ver
where `ver` is the latest version of `cc` supporting rustc 1.65.0
Error: Process completed with exit code 1. Looks like #1924 might fix this, will update when that's merged. |
Yep, my PR adds an entry to the |
@tbrockman Can you also update changelog for the support of metrics semconvs, along with the breaking change for consolidation of attributes. |
Can do. |
… semconv and zipkin. Fix deprecated attribute checking in template.
@lalitb believe all related |
@tbrockman Just few nit comments, should be good to go otherwise. Thanks for the contribution. |
@lalitb makes sense, I wasn't entirely sure what the release process entailed. Happy to contribute, thanks for making the process quick and painless 😬 |
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.
Thanks @tbrockman . One nit comment.
Fixes #
TBD
Design discussion issue (if applicable) #
Slack context # https://cloud-native.slack.com/archives/C03GDP0H023/p1720453950905069
Changes
semantic_metrics.rs.j2
Jinja template for producingmetric.rs
, which contains Metric semantic conventions.const &str
inattributes.rs
fileresource.rs
,trace.rs
) re-export attributes fromattribute.rs
to preserve backwards compatibility (though it might be desirable to break compatibility here as it seemsresource.rs
andtrace.rs
both contain duplicated attributes fromattribute_group
)Generated docstring VSCode examples:
![image](https://private-user-images.githubusercontent.com/8674362/347117308-b44607a5-2251-4582-a0b0-1414266b498a.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5OTA1MTIsIm5iZiI6MTczODk5MDIxMiwicGF0aCI6Ii84Njc0MzYyLzM0NzExNzMwOC1iNDQ2MDdhNS0yMjUxLTQ1ODItYTBiMC0xNDE0MjY2YjQ5OGEucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMDQ1MDEyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9ZGJiYzNmZWY3OTIxM2RjM2M5NzI2MTExM2NlMTk1OTE5YjU4ODZiNjkyNTg1ZmE1ZDY4NTg3NjQxYmI2YjVhNCZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.1vzZHFM4kObYohSzP0r0TjmvPiIB4wIXglH6NKw3KbM)
![image](https://private-user-images.githubusercontent.com/8674362/347117343-3a4117fe-4527-4ce3-ba4a-cf6fc0360047.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzg5OTA1MTIsIm5iZiI6MTczODk5MDIxMiwicGF0aCI6Ii84Njc0MzYyLzM0NzExNzM0My0zYTQxMTdmZS00NTI3LTRjZTMtYmE0YS1jZjZmYzAzNjAwNDcucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI1MDIwOCUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNTAyMDhUMDQ1MDEyWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9YzgzYmZmNDNhNWUzMWI3OWQ1YjQyYjIwNWQ0YzYzYzNkZDEzZjEzOTY5MWNkOTAxOGJkMmVmZTc3ZTRkNTZhNSZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.Seb3rWI9yuFkoK4_tLV-2O94m2KOjNLNoNFO8fY7z4M)
As mentioned in Slack, it seems like the code generation will be migrated to
weaver
, but I'd already wrote this and perhaps this could be useful in the interim.Merge requirement checklist
CHANGELOG.md
files updated for non-trivial, user-facing changes