-
Notifications
You must be signed in to change notification settings - Fork 2.7k
[pkg/ottl] Fix the context inference order to prioritize scope
over resource
#39307
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
[pkg/ottl] Fix the context inference order to prioritize scope
over resource
#39307
Conversation
scope
over resource
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.
Can we add a test that mimics the reported context inference, using the default inferrer?
It seems all the tests use a test-provided priority rather than the default inferrer priority.
Hi @dehaansa, thanks for reviewing! the current testing strategy was ensuring the context priority list works (by mocking the priority values and checking the inferred value), and then testing if the default priority list is the one we expect so OTTL contexts would be properly inferred (see). Those tests cases should be enough to guarantee correctness, but I agree that it wouldn't hurt adding more explicit tests for the existing OTTL contexts. I've pushed the changes already. |
… `resource` (open-telemetry#39307) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Changed the context inferrer so the `scope` context has priority over the `resource` context, fixing the error described on the linked issue. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#39155 <!--Please delete paragraphs that you did not use before submitting.-->
… `resource` (open-telemetry#39307) <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Changed the context inferrer so the `scope` context has priority over the `resource` context, fixing the error described on the linked issue. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#39155 <!--Please delete paragraphs that you did not use before submitting.-->
Description
Changed the context inferrer so the
scope
context has priority over theresource
context, fixing the error described on the linked issue.Link to tracking issue
Fixes #39155