Skip to content
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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Dec 6, 2019
Merged

Allow array values for attributes #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
51baa9a
Allow array values for attributes
Dec 2, 2019
bdf9aa1
Merge branch 'master' into feature/tigran/multivalue
bogdandrutu Dec 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions specification/api-tracing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,10 @@ A `Span` MUST have the ability to set attributes associated with it.
An `Attribute` is defined by the following properties:

- (Required) The attribute key, which must be a string.
- (Required) The attribute value, which must be either a string, a boolean
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,
Copy link
Member

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":...,...}}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a different requirement. We would need that for #76

This one is only adding support for arrays which is needed by #341

Copy link
Member

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?

Copy link
Member Author

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.

Copy link
Member

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.

Copy link
Contributor

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.)

Copy link
Member Author

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.

i.e. it MUST NOT contain values of different types.

The Span interface MUST provide:

Expand Down
5 changes: 5 additions & 0 deletions specification/data-semantic-conventions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ Span attributes.
* [Resource Conventions](data-resource-semantic-conventions.md)
* [Span Conventions](#span-conventions)

The type of the attribute SHOULD be specified in the semantic convention
for that attribute. Array values are allowed for attributes. For
protocols that do not natively support array values such values MUST be
represented as JSON strings.

## Span Conventions

In OpenTelemetry spans can be created freely and it’s up to the implementor to
Expand Down