feat: Implement scope_info_enabled#3503
Conversation
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3503 +/- ##
=======================================
- Coverage 82.8% 82.8% -0.1%
=======================================
Files 130 130
Lines 27276 27284 +8
=======================================
- Hits 22609 22608 -1
- Misses 4667 4676 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Thank you for your contribution! This PR has been automatically marked as stale because it has not had activity in the last 14 days. This may be due to a delay in review on our side or awaiting a response from you; either is fine, and we appreciate your patience. It will be closed in 14 days if no further activity occurs. Pushing a new commit or leaving a comment will remove the stale label and keep the PR open. |
|
not stale, just waiting for reviews |
|
hey @cijothomas, checking in here, anything I could do to move this forward? |
|
@ArthurSens Left few inline comments, nothing major. Please address them, and I'll do a final approaal and merge. |
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
ArthurSens
changed the title
ArthurSens
changed the title
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
cijothomas
left a comment
cijothomas
left a comment
There was a problem hiding this comment.
Looks good, just want another nit fix on changelog and good to go.
Signed-off-by: Arthur Silva Sens <arthursens2005@gmail.com>
| let mut attr_labels = BTreeMap::<String, Vec<String>>::new(); | ||
| for kv in scope.attributes() { | ||
| let label_name = utils::sanitize_prom_kv(&format!("{SCOPE_ATTRIBUTE_PREFIX}{}", kv.key)); | ||
| if RESERVED_SCOPE_LABELS.contains(&label_name.as_str()) { |
There was a problem hiding this comment.
good for followup - we can check if the attribute is one of name/version/schema_url before allocating the label_name by invoking format.
| res | ||
| } | ||
|
|
||
| fn get_scope_labels(scope: &InstrumentationScope) -> Vec<LabelPair> { |
There was a problem hiding this comment.
existing bug - I don't think we are handling the collision when scope_name (etc.) and metric attributes are same. Just noting , not blocker for this PR but a good follow up.
There was a problem hiding this comment.
Interesting, I have never seen a metric attribute with the otel_scope_* prefix, but I guess indeed that's possible
There was a problem hiding this comment.
the spec calls out this possibility and tells us what to do. We need to follow that. (of course in a future PR)
8a2067c
2 participants
Related to #3474, but entirely focused on the implementation of Instrumentation Scope.
Here I'm doing quite a few things:
otel_scope_infometric, which was removed from the spec long ago.without_scope_infotoscope_info_enabled, following the latest discussions we had at Prometheus exporter: Stabilize config optionscope_info_enabledopentelemetry-specification#5056Disclosure: Although I've done Rust before, I'm not great at it. I do rely on LLMs to write most of the code.
Changes
Please provide a brief description of the changes here.
Merge requirement checklist
CHANGELOG.mdfiles updated for non-trivial, user-facing changes