-
Notifications
You must be signed in to change notification settings - Fork 42
Support for attributes of type any
and template[any]
#707
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #707 +/- ##
=======================================
- Coverage 77.0% 76.5% -0.5%
=======================================
Files 65 65
Lines 4936 4978 +42
=======================================
+ Hits 3803 3812 +9
- Misses 1133 1166 +33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
any
and template[any]
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.
I think we need a way to compare compatibility of PrimitiveOrArrayTypeSpec
. In the TypeAdvisor near your change, we compare the sample type with the semconv type with a simple inequality:
if attribute_type != semconv_attribute_type {
But now if attribute_type
is string
and semconv_attribute_type
is any
then these types are compatible.
crates/weaver_resolver/data/registry-test-15-attr-any/registry/registry-messaging.yaml
Show resolved
Hide resolved
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.
Schema changes LGTM!
Let's bring it up on the semconv tooling meeting - we'll need to communicate this to maintainers - some languages need to add support for any
attributes or would generate invalid code.
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.
@lquerel - setting this to "Request Changes" so we don't merge without fixing the TypeAdvisor. (see other comment).
What's the current status of this? Is it waiting more reviews or blocked on something? |
@dyladan ETA to get this one merged is this week and release is planned for the next week. |
@jerbly I made this change |
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! Good to go!
This PR introduces support for attributes of type
any
andtemplate[any]
, enabling more flexible handling of attribute values in the semconv.Closes: #703