-
Notifications
You must be signed in to change notification settings - Fork 927
feat(metrics): add advisory attributes parameter to metric instruments #5783
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?
feat(metrics): add advisory attributes parameter to metric instruments #5783
Conversation
Add optional attributes parameter to MetricAdvice interface to support advisory attribute filtering for metric instruments. This parameter allows users to specify which attribute keys should be kept when recording metrics, acting as an allow-list filter. The parameter is marked as experimental and follows the OpenTelemetry specification for advisory parameters.
Implement advisory attributes filtering in ViewRegistry by creating a default view with AllowListProcessor when no matching views are found and the instrument has advisory attributes specified. This ensures that only the specified attribute keys are retained when recording metrics, reducing cardinality and improving performance.
Add comprehensive unit and integration tests for the advisory attributes parameter functionality: - Unit tests for ViewRegistry advisory attributes handling - Integration tests covering various scenarios including edge cases - Tests for attribute filtering behavior with and without views - Verification of experimental feature marking
Add documentation and example for the new advisory attributes parameter: - Update README with usage instructions and examples - Create standalone example demonstrating the feature - Include notes about experimental status and performance benefits - Provide clear guidance on when and how to use advisory attributes
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.
Thank you for working on this. 🙂
Looks like the tests are failing - I added some assumption as to why to my comments below.
Please also add an entry in ./CHANGELOG.md
and ./api/CHANGELOG.md
for this feature.
} | ||
|
||
// Create a view that matches this specific instrument | ||
return new View(viewOptions); |
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.
My guess is that creating a view always is what's eventually failing the tests. You might have to look into which part of the code calls findViews()
to investigate what's wrong.
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.
Sure , I will look into all the issues and get back to you
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.
Hi @pichlermarc
I've addressed the aforementioned issues
Could you take a look
Thank you
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.
looks like it's still failing due to the issue I mentioned above, did you force-push over the change that addressed this? 🤔
Added the suggested change Co-authored-by: Marc Pichler <[email protected]>
- Add experimental `attributes` parameter to MetricAdvice interface - Implement attribute filtering via ViewRegistry default views - Only applies when no Views are configured for the instrument - Views take precedence over advisory attributes when present - Add comprehensive unit and integration tests - Update documentation and changelogs Closes open-telemetry#4365
3e84236
to
b5eca96
Compare
Which problem is this PR solving?
This PR implements the "attributes" instrument advisory parameter for OpenTelemetry metrics as specified in the OpenTelemetry specification. The advisory parameter allows users to specify an allow-list of attribute keys that should be retained when recording metrics, helping to reduce cardinality and improve performance.
Fixes #4365
Short description of the changes
attributes
parameter toMetricAdvice
interface in the APIViewRegistry
usingAllowListProcessor
The implementation ensures that:
Type of change
How Has This Been Tested?
ViewRegistry
advisory attributes handlingThe tests cover:
Checklist: