-
Notifications
You must be signed in to change notification settings - Fork 899
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
Allow array values for attributes #368
Allow array values for attributes #368
Conversation
Resolves open-telemetry#341 Array values are useful for representing attributes that can have multiple values (e.g. representing a Memcached `get` that can be over multiple keys and making the keys a span attribute). This change allows homogeneous array values for span attributes and specifies that array values should be JSON encoded stirng when exporting using protocols that do not natively support array values.
06568ed
to
51baa9a
Compare
value, or a numeric type. | ||
- (Required) The attribute value, which is either: | ||
- A primitive type: string, boolean or numeric. | ||
- An array of primitive type values. The array MUST be homogeneous, |
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 the requirement is not to support array, but a map so that you have {"http" : {"url":...,...}}
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.
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.
Before accepting this, I am curios to see all the requirements. Is only map/array that we need?
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.
The only requirement we have so far is in #341 and it is only for arrays, not maps.
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 remember for sure that I saw the map requirement documented as well, for the example I mentioned in the previous post.
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.
To complete the JSON structure, we'd allow a null
value in addition to the ones already discussed. There may be some connection to the discussion about unspecified values for metric labels.
(For the record, I don't think that metric labels need to support lists or maps of values--but I also don't see a problem with coercing JSON lists or maps to strings when used as metric labels. Supporting a null value distinct from the string "null" or empty string would let provide a satisfying answer to the question about unspecified values--whereas the current state was not completely satisfying.)
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.
This is only for Span and Resource attributes. It does not affect metric labels.
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 this is a solid improvement, these requirements (e.g., list homogeneity, supporting lists but not maps) can always be relaxed.
value, or a numeric type. | ||
- (Required) The attribute value, which is either: | ||
- A primitive type: string, boolean or numeric. | ||
- An array of primitive type values. The array MUST be homogeneous, |
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.
To complete the JSON structure, we'd allow a null
value in addition to the ones already discussed. There may be some connection to the discussion about unspecified values for metric labels.
(For the record, I don't think that metric labels need to support lists or maps of values--but I also don't see a problem with coercing JSON lists or maps to strings when used as metric labels. Supporting a null value distinct from the string "null" or empty string would let provide a satisfying answer to the question about unspecified values--whereas the current state was not completely satisfying.)
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.
@tigrannajaryan as a followup from the discussions in this thread I think we need to file an issue to discuss the support of the maps. @Oberon00 @arminru I think this idea of supporting a map was discuss during some discussions about typed spans.
Added a an issue to discuss: #376 |
@open-telemetry/specs-approvers can we merge this? |
Resolves open-telemetry#341 Array values are useful for representing attributes that can have multiple values (e.g. representing a Memcached `get` that can be over multiple keys and making the keys a span attribute). This change allows homogeneous array values for span attributes and specifies that array values should be JSON encoded stirng when exporting using protocols that do not natively support array values.
Resolves open-telemetry#341 Array values are useful for representing attributes that can have multiple values (e.g. representing a Memcached `get` that can be over multiple keys and making the keys a span attribute). This change allows homogeneous array values for span attributes and specifies that array values should be JSON encoded stirng when exporting using protocols that do not natively support array values.
Resolves #341
Array values are useful for representing attributes that can have
multiple values (e.g. representing a Memcached
get
that can beover multiple keys and making the keys a span attribute).
This change allows homogeneous array values for span attributes
and specifies that array values should be JSON encoded string when
exporting using protocols that do not natively support array values.