search

open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.55k stars 742 forks source link

Semantic Conventions Update Options #4771

Open dyladan opened 3 weeks ago

dyladan commented 3 weeks ago

This is to document each of the potential solutions we have discussed to update our semantic conventions

Option 1: package.json entry points

example: https://github.com/open-telemetry/opentelemetry-js/pull/4770

This option involves adding an entry point for each semconv version. The top level export would export stable attributes, and all experimental attributes would be hidden under a version-specific entrypoint.

import { SOME_EXPERIMENTAL_ATTR } from '@opentelemetry/semantic-conventions/1.26'
import { SOME_STABLE_ATTR } from '@opentelemetry/semantic-conventions'

Cons:

Option 2: single semconv version per release

This option involves publishing a new semconv package and version-locking it with the upstream semconv. Each version would contain only its generated attributes.

{
   "dependencies": {
      "@opentelemetry/semconv-1_26": "npm:@opentelemetry/semconv@0.126.0",
      "@opentelemetry/semconv-1_25": "npm:@opentelemetry/semconv@0.125.0"
   }
}
import { SOME_ATTR } from '@opentelemetry/semconv-1_26'
import { SOME_OTHER_ATTR } from '@opentelemetry/semconv-1_25'

Cons:

Option 2.1: Release a new package for each semconv version with version in name

This is similar to the above but we actually bake the semconv version directly into the name of the package so the user doesn't have to mess around with package.json renames. We would not keep old versions in the repo since they would never be updated. If they do need to be updated we would have to resurrect them from git history.

{
   "dependencies": {
      "@opentelemetry/semconv": "^1.26.0",
      "@opentelemetry/semconv-1_26": "^1.0.0",
      "@opentelemetry/semconv-1_25": "^1.0.0"
   }
}
import { STABLE_ATTR } from '@opentelemetry/semconv'
import { SOME_EXPERIMENTAL_ATTR } from '@opentelemetry/semconv-1_26'
import { SOME_OLD_ATTR } from '@opentelemetry/semconv-1_25'

Option 3: use stable/experimental package.json entrypoints

Example: https://github.com/open-telemetry/opentelemetry-js/pull/4690

This involves using package.json entrypoints to export experimental attributes explicitly. Top level would only include stable attributes. Deprecated attributes are included with @deprecated tags for backwards compatibility.

import { STABLE_ATTR } from '@opentelemetry/semantic-conventions'
import { SOME_EXPERIMENTAL_ATTR } from '@opentelemetry/semantic-conventions/experimental'

Other options lightning round

These options were discussed but nobody seemed to jump at them and there wasn't much discussion

dyladan commented 3 weeks ago

I think option 3 is the best. It avoids massive installation size duplication, allows users to reference deprecated attributes, and makes it clear what is experimental and what is stable.

trentm commented 3 weeks ago

I agree that option 3 is best.

Thanks very much for writing these out. It helped me sort out my understanding and preferences. I had been hesitating on option 3 because of the messaging.client.id ambiguity, and leaning towards various options with version-specific entry-points to cope. However, assuming that the ambiguous identifier issue is resolved as part of https://github.com/open-telemetry/semantic-conventions/issues/1118 then I think it works well.

pichlermarc commented 3 weeks ago

I also support Option 3.

One question: If we want to double emit with Option 3 we would tell people to install the old version via npm:@, correct? (Edit: nevermind, it's deprecated in /experimental then, so still there, neat)

dyladan commented 3 weeks ago

I also support Option 3.

~One question: If we want to double emit with Option 3 we would tell people to install the old version via npm:@, correct?~ (Edit: nevermind, it's deprecated in /experimental then, so still there, neat)

It's actually in two places:

  1. The namespace exports are in the main entrypoint in order to maintain backward compatibility (marked @deprecated)
  2. Old names are exported and marked as @deprecated in the new exports also. Technically they are both in stable and experimental, but there are no deprecated stable attributes yet.
david-luna commented 3 weeks ago

+1 to Option 3

For https://github.com/open-telemetry/semantic-conventions/issues/1118 it seems we're reaching a consensus towards option 2. IIUC this will solve the collision problem but cannot avoid the problem of old const name having the new value (as mentioned https://github.com/open-telemetry/semantic-conventions/issues/1118#issuecomment-2149190108).

Which kind of version bump is going to happen in @opentelemetry/semantic-conventions? If the answer is major. Is that enough to not break customer instrumentations?

trentm commented 3 weeks ago

If the answer is major.

If the answer is major, then let's talk about dropping the old (SEMRESATTRS_*) and old old (SemanticAttributes.*) exports. :)

legendecas commented 3 weeks ago

+1 to option 3.

Unless it is reasonable for a single package to depend on multiple semconv versions at the same time, I would find that utilize semver of the semconv package and exposing an experimental entrypoint would be more intuitive and maintainable.

dyladan commented 3 weeks ago

I thought we would have a minor version which contains both the old names and the new names. It should not be necessary to depend on multiple versions. When an attribute is renamed/deprecated, it is added to the deprecated yaml and still appears in newer versions of the semconv, so should be in the latest version. Any code that compiles with semver 1.25 should also compile just fine with semver 1.26

We could consider going to 2.0 to remove the namespace versions and old SEMATTRS names, but I think we should have at least some period of overlap before doing that.