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.

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

Move attribute definition from spec #636

Conversation

joaopgrassi
Copy link
Member

@joaopgrassi joaopgrassi commented Jan 10, 2024

Changes

Some time ago, we moved the requirement levels and attribute naming docs from the spec repo to the semconv repo, as those are mostly referenced here in the semconv.

This build-tools PR https://github.com/open-telemetry/build-tools/pull/222/files improves the markdown table headers by adding links to the respect requirement levels + attribute type docs, so readers can navigate to them easily. The problem is, the tools now accept a version, which is used to pick the correct links to the respective files. Given some files are in the spec repo and others are in the semconv repo, the single version attribute won't be enough. We could duplicate it always, but it's another attribute which makes the tool more complicated than already is.
This PR moves that file to the semconv repo, which IMO is a better place for it, even more now that after the build-tools PR above is merged.

I attempted to bring the commit history, hopefully all is correct 😅

Merge requirement checklist

bogdandrutu and others added 30 commits July 22, 2020 11:30
* Centralize attributes definition

Signed-off-by: Bogdan Drutu <[email protected]>

* Update specification/common/common.md

Co-authored-by: Christian Neumüller <[email protected]>

* Update specification/common/common.md

Co-authored-by: Christian Neumüller <[email protected]>

* Update specification/common/common.md

Co-authored-by: Christian Neumüller <[email protected]>

* Update specification/common/common.md

Co-authored-by: Christian Neumüller <[email protected]>

* Update specification/common/common.md

Co-authored-by: Christian Neumüller <[email protected]>

* Update specification/common/common.md

Co-authored-by: Christian Neumüller <[email protected]>

* Update specification/overview.md

Co-authored-by: Christian Neumüller <[email protected]>

Co-authored-by: Christian Neumüller <[email protected]>
* Consistency between Span and Resource attributes

* Address feedback

* Wording
* Add conventions for attribute names

Resolves: open-telemetry/opentelemetry-specification#726

* Address PR comments

* Re-word company/application specific attribute recommendations
* Extend attribute naming rules to metric labels

We earlier defined naming rules for attributes, however we do not have
similar rules for metric labels. This commit extends the exact same
set of rules to metric labels.

This was brought up in this comment open-telemetry/opentelemetry-specification#807 (comment)

* Address PR comments
…antic convention areas (open-telemetry#832)

* Require that names and namespaces are one global space across all semantic convention areas

We have semantic conventions for Resources, Spans and Metrics (in the future also Logs are expected).
It was not clear if the attribute names across all convention areas should be globally unique.

This commit asserts that conventions are one space, they are not independent spaces with
their own namespaces each.

We prohibit using the same Span or Resource attribute name or metric label name but give them
slightly different meanings or value sets.

Resolves: open-telemetry/opentelemetry-specification#815

* Address PR comments

Co-authored-by: Bogdan Drutu <[email protected]>
* Remove absolute links where possible.

* Work around semantic conventions.

* Docfx.

* Fix YAML.

* More docfx.
* Remove ordering for attributes.

* Fill in CHANGELOG link
* Nulls SHOULD NOT be allowed in arrays.

* Fill in CHANGELOG link

Co-authored-by: Armin Ruech <[email protected]>
* Declare freeze of Trace Specification 1.0

We want to freeze Trace specification 1.0 so that we no longer accept substantial changes (unless a fundamental problem is found in the spec).

Resolves open-telemetry/opentelemetry-specification#1120
I noticed developers adding their own attributes to this namespace
without going through the specification. We need to regulate this
namespace through the specification, just like we do it for other
semantic conventions.
Resources are not susceptible to scenarios where excessive attributes
can be recorded unlike Spans. Resources are also immutable and it can be
hard for some SDKs to apply the limits at source at the time the
attributes are added to a resource. Furthermore, limits and Resources
both are generally defined and passed on to a TracerProvider which
forces a TracerProvider to either mutate the resource or generate a new
one with duplicate attributes in order to apply the limits to it.

Co-authored-by: Tigran Najaryan <[email protected]>
…tributes (plural) (#2123)

* Provide a normative definition of Attribute (singular)

- Providing a normative definition of **attribute** (singular)
- Other copyedits

/cc @austinlparker @tedsuo

* Move anchor to make markdown checker happy
* Prohibit usage of retired names in semantic conventions

This change adds a prohibition clause that requires that no old
metric or attribute name is used for a new attribute.

This is important to ensure reversibility of schema transformation
(converting from a new version to an old version of schema).

Without this restriction the following is possible:

Schema version 1. Attribute A exists.
Schema version 2. Attribute A is renamed to B. Appropriate schema file is created.
Schema version 3. Attribute A is introduced (a completely different new attribute).

Now attempting to go from Version 3 to version 1 is impossible since it requires
renaming B to A (for the change in version 2), but a different attribute A already exists.

* Fix based on comments

* Add changelog entry

Co-authored-by: Carlos Alberto Cortez <[email protected]>
Attributes keys must be unique. The key/value pair collections in the specification
was always intended to model a map.

There was a recent confusion about this. This change clarifies the spec.

Resolves open-telemetry/opentelemetry-specification#2245
…2276)

* Clarify that Trace/Meter are associated with Instrumentation Scope

This is a slightly different take on open-telemetry/opentelemetry-specification#2203

Instead of renaming instrumentation library to instrumentation scope everywhere
this PR suggests targetted editing of wording of the Trace/Meter obtaining API
to allow not just instrumentation library but other instrumentation scopes to be
used as a parameter.

This change does not force renaming of API parameters and is not a breaking change.
We consider it a clarification of usage to match the intent that we had for the "name"
field.

If this PR is accepted there will be a follow up PR that will suggest to rename
InstrumentationLibrary* messages in OTLP proto to InstrumentationScope* message.
Such a change will not be a breaking change for the OTLP wire format and is acceptable.

If this PR is accepted we will also close open-telemetry/opentelemetry-specification#1236
since it will be no longer necessary. The logger name will be recorded as the
instrumentation scope. This clarification will be a follow up PR that clarifies the
behavior in https://github.com/open-telemetry/oteps/blob/main/text/logs/0150-logging-library-sdk.md
…y support strings (#2343)

* Describe how to handle converting non-string primitives for protocols that only support strings

* update wording to make clear that only non-string values are converted to strings

* unify language

* Update specification/common/common.md

Co-authored-by: Joshua MacDonald <[email protected]>

Co-authored-by: Joshua MacDonald <[email protected]>
Co-authored-by: Bogdan Drutu <[email protected]>
OTEP 0178 [0] proposed this mapping. This PR merges the proposal into specification.
The content of this PR is mostly copy/paste of the text of the OTEP minus the
irrelevant sections such as "Alternates", etc.

0 - https://github.com/open-telemetry/oteps/blob/main/text/0178-mapping-to-otlp-anyvalue.md
* nits

* review comments

* No exceptions for Required attributes, clarifications on performance

* Conditional clarifications

* nits

* Conditional -> required conditionally and minor fixes

* Align requirement levels with RFC levels better

* Clarify Note on required attributes

* Update specification/common/attribute-requirement-level.md

Co-authored-by: Tigran Najaryan <[email protected]>

* Clarify Note on required attributes

* Remove performance from conditionally required attributes

* Clarify Conditionally Required case when condition is false

* Apply suggestions from code review

Co-authored-by: Tigran Najaryan <[email protected]>

* Apply suggestions from code review

Co-authored-by: Christian Neumüller <[email protected]>

* headings for levels and moving things around

* nits: formatting

Co-authored-by: Tigran Najaryan <[email protected]>
Co-authored-by: Christian Neumüller <[email protected]>
Co-authored-by: Reiley Yang <[email protected]>
alanwest and others added 19 commits January 26, 2023 19:22
Fixes #2860

Adds log attribute limit configuration.

These new environment variables bring more consistency between spans and logs.
…braries (#3289)

Based on discussion in semconv stability WG

Closes #3283

## Changes

Clarifies that attribute requirement levels apply to instrumentation.
And that, because users can transform their telemetry in a number of
ways (e.g. metric views, span processors, and collector
transformations), these requirement levels cannot be relied on by
telemetry consumers.

---------

Co-authored-by: Liudmila Molkova <[email protected]>
Co-authored-by: Bogdan Drutu <[email protected]>
- Contributes to
open-telemetry/opentelemetry.io#2429
- This is part of an effort to normalize links, for improved link
checking on the OTel website
…` (and `source.*`/`destination.*`) (#3402)
…k.*` and align definitions with ECS (#3426)
…l namespaces (#3507)

The @open-telemetry/technical-committee discussed and decided to keep the
existing recommendations but clarify them and explain the purpose.
- Followup changes for
open-telemetry/opentelemetry.io#2793
- There are only changes to Hugo front matter
- Adds `likeTitle`s for "Compatibility" pages
- Adds aliases for pages that have moved or were renamed
- Related:
open-telemetry/opentelemetry.io#3013 -- the
`compatibility/openmetrics` spec page is in the list because it was
renamed

/cc @svrnm @cartermp
Fixes open-telemetry#454 

Related to #3337 

As the Messaging SIG merged its last OTEP 222, we will be adding operations
that require Links after Span creation, taking a direct route with `AddLink()`,
albeit without any of the new members suggested in #3337, e.g. `timestamp` (to be
discussed in a separate issue).

```
AddLink(spanContext, attributes /* optional */)
```
…h `network.(peer|local).(address|port)`. (#3713)

Co-authored-by: Armin Ruech <[email protected]>
@joaopgrassi joaopgrassi marked this pull request as ready for review January 10, 2024 17:57
@joaopgrassi joaopgrassi requested review from a team January 10, 2024 17:57
Copy link

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

@github-actions github-actions bot added the Stale label Jan 27, 2024
Comment on lines +71 to +74
By default an SDK SHOULD apply truncation as per the list of
[configurable parameters](#configurable-parameters) below.

If an SDK provides a way to:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this document belongs into the semantic conventions repository, as it mostly contains requirements that need to be implemented by SDKs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.