[Common] Clarify Attribute support based on stable wire protocol definition of Attributes #2581 #2620

MSNev · 2022-06-16T21:42:57Z

Fixes #
#2581 [Common] Spec inconsistency with proto definition of Attributes

Changes

Clarifies the Attribute support as defined by Proto AnyValue and clarifies Attribute Collections.
Clarifies and identifies how nested attributes support (as defined by Proto) should be supported.

~~Related issues #~~
~~#376 Support ~~map values and~~ nested values for attributes #376~~

github-actions · 2022-06-24T03:45:34Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

MSNev · 2022-06-28T15:03:31Z

Not stale

spec-compliance-matrix.md

Aneurysm9 · 2022-06-28T15:46:43Z

Clarifies the Attribute support as defined by Proto AnyValue and clarifies Attribute Collections.
Clarifies and identifies how nested attributes support (as defined by Proto and some SDK's) should be supported.

To the extent that this would apply to attributes for traces and metrics I think this goes beyond clarification and changes the specification. To my knowledge there are no trace/metric SDK implementations that allow nested attributes.

MSNev · 2022-07-01T23:55:34Z

Clarifies the Attribute support as defined by Proto AnyValue and clarifies Attribute Collections.
Clarifies and identifies how nested attributes support (as defined by Proto and some SDK's) should be supported.

To the extent that this would apply to attributes for traces and metrics I think this goes beyond clarification and changes the specification. To my knowledge there are no trace/metric SDK implementations that allow nested attributes.

If you read the spec changes, I believe that I have not specified that trace/metric implementations have to implement or allow for nested attributes. And even the PR comment from above I am stating how it should be supported not that it must or is.

I've now removed the "and some SDK's" based on re-reviewing the JavaScript implementation and removing it from the support matrix.

What specific or additional clarification would you suggest?

specification/common/README.md

spec-compliance-matrix.md

CHANGELOG.md

specification/common/README.md

github-actions · 2022-07-20T03:46:13Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

specification/common/README.md

Oberon00 · 2022-08-03T15:39:34Z

Please give this PR a more descriptive title. I think the "inconsistency" issue has been explained. It seems to me this is not solving an inconsistency but proposing a new feature.

MSNev · 2022-08-03T22:28:10Z

Please give this PR a more descriptive title. I think the "inconsistency" issue has been explained. It seems to me this is not solving an inconsistency but proposing a new feature.

Renamed.

I don't believe that this PR is identifying any new functionality, it is attempting to properly document what Attribute support is today.

The protocol SUPPORTS nesting for Attributes (and has for around 2 years)
Logs SUPPORTS and requires nested attributes

While Spans and Metrics also use "Attributes" their definition of Attributes does not support nesting (from at the spec level), which if not clarified is going to cause confusion not just at the consumer level (using the API vs Protocol) but also at the implementation level when SDK's start to support the proper definition of the Logs Attributes

Oberon00 · 2022-08-04T07:14:45Z

I don't believe that this PR is identifying any new functionality, it is attempting to properly document what Attribute support is today.

In practice, you are proposing a new feature. This is not supported today.

Do note that OTLP is merely a protocol that can be used for transporting (a superset of) OpenTelemetry-generated data. It is not the authoritative definition of the data model.

MSNev · 2022-08-08T16:05:56Z

In practice, you are proposing a new feature. This is not supported today.

That is because none of the SDK's (that I'm aware of) are currently SPEC compliant for LOGS, once they start to correctly implement Attributes for Logs everyone will hit this issue.

Oberon00 · 2022-10-04T09:45:50Z

specification/common/README.md

@@ -31,8 +32,15 @@ An `Attribute` is a key-value pair, which MUST have the following properties:
  - A primitive type: string, boolean, double precision floating point (IEEE 754-1985) or signed 64 bit integer.
  - An array of primitive type values. The array MUST be homogeneous,
    i.e., it MUST NOT contain values of different types.
+  - When specified via semantic conventions it MAY (see [Nested Attributes](#nested-attributes))


This is not a clarification (as written in the PR title) but allows a new feature. Also what is allowed on the level of the basic attribute definition must not depend on semantic conventions.

This line explicitly calls out to see the Nested Attribute support which clarifies that this is NOT automatically supported by this line but must be explicitly defined by the semantic conventions (and is supported by Logs).

But it allows semantic conventions to add them, as written now, doesn't it?

github-actions · 2022-10-13T04:02:08Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2022-10-26T03:48:18Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

MSNev · 2022-10-26T15:32:46Z

Not stale as this is part of a larger issue like #2888

MSNev · 2022-11-07T17:21:43Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Don't have the ability (permissions) to re-open...

…telemetry#2581 and Support map values and nested values for attributes open-telemetry#376

Use a Permalink Change Support table to yes/no rather than same as Compliance Matrix

github-actions · 2022-11-22T03:28:23Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2022-11-30T03:18:58Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions · 2022-12-21T03:17:20Z

Closed as inactive. Feel free to reopen if this PR is still being worked on.

MSNev requested review from a team as code owners June 16, 2022 21:42

github-actions bot assigned SergeyKanzhelev Jun 16, 2022

github-actions bot added the Stale label Jun 24, 2022

MSNev commented Jun 28, 2022

View reviewed changes

spec-compliance-matrix.md Outdated Show resolved Hide resolved

arminru added area:api Cross language API specification issue area:data-model For issues related to data model labels Jun 28, 2022

github-actions bot removed the Stale label Jun 29, 2022

MSNev force-pushed the MSNev/AttributeSupport branch from 8bb0e81 to 08e3fed Compare July 1, 2022 23:43

arminru reviewed Jul 5, 2022

View reviewed changes

arminru requested review from a team July 5, 2022 11:29

MSNev force-pushed the MSNev/AttributeSupport branch from 52fe194 to 2f9fc47 Compare July 7, 2022 20:23

arminru added the spec:logs Related to the specification/logs directory label Jul 12, 2022

arminru reviewed Jul 12, 2022

View reviewed changes

specification/common/README.md Outdated Show resolved Hide resolved

specification/common/README.md Outdated Show resolved Hide resolved

specification/common/README.md Outdated Show resolved Hide resolved

specification/common/README.md Outdated Show resolved Hide resolved

github-actions bot added the Stale label Jul 20, 2022

djaglowski removed the Stale label Jul 20, 2022

reinasalondra mentioned this pull request Jul 23, 2022

Add process.parent_pid to semantic convention constants #2689

Closed

mateuszrzeszutek reviewed Jul 25, 2022

View reviewed changes

specification/common/README.md Outdated Show resolved Hide resolved

specification/common/README.md Outdated Show resolved Hide resolved

MSNev force-pushed the MSNev/AttributeSupport branch from 2fa721d to 2cbe005 Compare August 2, 2022 15:26

MSNev changed the title ~~[Common] Spec inconsistency with proto definition of Attributes #2581~~ [Common] Clarify Attribute support based on stable wire protocol definition of Attributes #2581 Aug 3, 2022

Oberon00 requested changes Oct 4, 2022

View reviewed changes

github-actions bot added the Stale label Oct 13, 2022

alanwest mentioned this pull request Oct 17, 2022

Add log attribute limit configuration #2861

Merged

MSNev force-pushed the MSNev/AttributeSupport branch from 8b0ba0b to fd3e680 Compare October 18, 2022 15:03

github-actions bot closed this Oct 26, 2022

djaglowski reopened this Nov 7, 2022

github-actions bot removed the Stale label Nov 8, 2022

MSNev added 8 commits November 14, 2022 09:13

[Common] Spec inconsistency with proto definition of Attributes open-…

ae94042

…telemetry#2581 and Support map values and nested values for attributes open-telemetry#376

Remove JavaScript from the nested support matrix

37b6114

Address issues and roll in changes from @arminru

9447b63

Run markdown-toc to fixup text on the TOC

7b76810

Move "See" to better highlight

7ffffcd

Use a Permalink Change Support table to yes/no rather than same as Compliance Matrix

Renaming heading to Supported

f4cb81b

Rework and organize based on additional comments

8a43439

Fix lint issue

f5b8f6e

MSNev force-pushed the MSNev/AttributeSupport branch from dc809f8 to f5b8f6e Compare November 14, 2022 17:17

github-actions bot added the Stale label Nov 22, 2022

djaglowski removed the Stale label Nov 22, 2022

github-actions bot added the Stale label Nov 30, 2022

Merge branch 'open-telemetry:main' into MSNev/AttributeSupport

98de935

djaglowski mentioned this pull request Dec 6, 2022

Propose new StructuredBody field for logs #3014

Closed

Merge branch 'open-telemetry:main' into MSNev/AttributeSupport

b490b8c

github-actions bot closed this Dec 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Common] Clarify Attribute support based on stable wire protocol definition of Attributes #2581 #2620

[Common] Clarify Attribute support based on stable wire protocol definition of Attributes #2581 #2620

MSNev commented Jun 16, 2022 •

edited

github-actions bot commented Jun 24, 2022

MSNev commented Jun 28, 2022

Aneurysm9 commented Jun 28, 2022

MSNev commented Jul 1, 2022 •

edited

github-actions bot commented Jul 20, 2022

Oberon00 commented Aug 3, 2022

MSNev commented Aug 3, 2022

Oberon00 commented Aug 4, 2022 •

edited

MSNev commented Aug 8, 2022

Oberon00 Oct 4, 2022 •

edited

MSNev Oct 4, 2022

Oberon00 Oct 5, 2022

github-actions bot commented Oct 13, 2022

github-actions bot commented Oct 26, 2022

MSNev commented Oct 26, 2022

MSNev commented Nov 7, 2022

github-actions bot commented Nov 22, 2022

github-actions bot commented Nov 30, 2022

github-actions bot commented Dec 21, 2022

[Common] Clarify Attribute support based on stable wire protocol definition of Attributes #2581 #2620

[Common] Clarify Attribute support based on stable wire protocol definition of Attributes #2581 #2620

Conversation

MSNev commented Jun 16, 2022 • edited

Changes

github-actions bot commented Jun 24, 2022

MSNev commented Jun 28, 2022

Aneurysm9 commented Jun 28, 2022

MSNev commented Jul 1, 2022 • edited

github-actions bot commented Jul 20, 2022

Oberon00 commented Aug 3, 2022

MSNev commented Aug 3, 2022

Oberon00 commented Aug 4, 2022 • edited

MSNev commented Aug 8, 2022

Oberon00 Oct 4, 2022 • edited

Choose a reason for hiding this comment

MSNev Oct 4, 2022

Choose a reason for hiding this comment

Oberon00 Oct 5, 2022

Choose a reason for hiding this comment

github-actions bot commented Oct 13, 2022

github-actions bot commented Oct 26, 2022

MSNev commented Oct 26, 2022

MSNev commented Nov 7, 2022

github-actions bot commented Nov 22, 2022

github-actions bot commented Nov 30, 2022

github-actions bot commented Dec 21, 2022

MSNev commented Jun 16, 2022 •

edited

MSNev commented Jul 1, 2022 •

edited

Oberon00 commented Aug 4, 2022 •

edited

Oberon00 Oct 4, 2022 •

edited