@opentelemetry/semantic-conventions deprecated warnings point to missing exports

[tmcw]

trafficstars

What happened?

Steps to Reproduce

Playground: https://www.typescriptlang.org/play/?ssl=4&ssc=1&pln=4&pc=21#code/JYWwDg9gTgLgBAKjgQwM4pjK6BmUIhwBEAAhGAKYB2MFANhSBVgJ4D0qjyNwAxgLS8IVAG7UYwYaiIBuAFBzkmbADoAygFEAsgEEAKnoBKagPoAJAwAUTAVUMAZeUqyoV+o+au2HMoA

Expected Result

If something has a deprecation message with a recommended updated strategy, that strategy should exist.

Actual Result

It doesn't.

Additional Details

OpenTelemetry Setup Code

import * as attrs from "@opentelemetry/semantic-conventions";

attrs.SEMATTRS_HTTP_URL;
attrs.ATTR_HTTP_URL;

package.json

No response

Relevant log output

No response

[pichlermarc]

Hi @tmcw, thanks for reaching out.

Looks like we're missing a mention that the @opentelemetry/semantic-conventions/incubating entrypoint is where these attributes are located. We'll have to add it to the Jinja template.

[tmcw]

That seems incorrect as well:

https://www.typescriptlang.org/play/?#code/JYWwDg9gTgLgBAKjgQwM4pjK6BmUIhwBEAAhGAKYB2MFANhSBVgJ4D0qjyNwAxgLS8IVAG7UYwYaiIBuAFChIsRCnTJM2AJJw8BYmUo16jZlHacQ3CQKGjxkqqjbAqvAK4AjdS4DmsuXLqWKgAdADKAKIAsgCCACpxAEphAPoAEgkACikAqokAMvJB2CHxSelZuQVFGqiapQmJFXHZeYVAA

SEMATTRS_HTTP_URL says to use ATTR_HTTP_URL, but then if I import from /incubating, and use ATTR_HTTP_URL, it says that it's deprecated in favor of url.full.

[Annosha]

@pichlermarc can I work on this issue as a beginner? I'm aiming to get familiar and contribute to opentelemetry-js code base.

[pichlermarc]

@pichlermarc can I work on this issue as a beginner? I'm aiming to get familiar and contribute to opentelemetry-js code base.

@Annosha thanks for reaching out. I'd say this issue is not very beginner friendly as all of that code is auto-generated - the generation of this is rather involved with custom tooling built around the semantic-conventions repo.

[JamieDanielson]

It looks in this deprecated warning we could update our deprecated message to use the actual attribute variable (ATTR_URL_FULL) vs just the attribute url.full. Same for others as well. I will take a look at this.

[trentm]

SEMATTRS_HTTP_URL says to use ATTR_HTTP_URL, but then if I import from /incubating, and use ATTR_HTTP_URL, it says that it's deprecated in favor of url.full.

There are two stages of deprecation here in 1.x versions of @opentelemetry/semantic-conventions, unfortunately, because of changes to the package.

0.x

Before 1.x there was 0.x, which exported a handful of big namespace-type objects:

exports.SemanticAttributes = {
    AWS_LAMBDA_INVOKED_ARN: 'aws.lambda.invoked_arn',
    DB_SYSTEM: 'db.system',
...

I only point this out for history.

early 1.x

Starting with 1.0.0 these large namespace-type objects were turned into flat consts -- to facilitate tree-shaking for smaller bundles for web usage, mainly. Effectively:

exports.SEMATTRS_HTTP_URL = 'http.url';
...
exports.SEMRESATTRS_SERVICE_NAME = 'service.name';

later 1.x

Later 1.x did a few things (https://github.com/open-telemetry/opentelemetry-js/pull/4690):

• update to recent version of the OTel Semantic Conventions (the JS package had been years behind);
• Change SEMRESATTRS_ and SEMATTRS_ to just ATTR_ for attributes, and deprecated the old names
• Generate enum values as <attribute name>_VALUE_<enum value name>
• Generate constants for metric names with METRIC_ prefix
• Added separate entry-points for the package: import ... from '@opentelemetry/semantic-conventions' now gets you stable semconv values (per https://github.com/open-telemetry/semantic-conventions) and import ... from '@opentelemetry/semantic-conventions/incubating' includes both the stable semconv values and experimental values (that might change, be removed, whatever)

So:

1. SEMATTRS_HTTP_URL really was deprecated in favour of ATTR_HTTP_URL. With the subtlety that to use ATTR_HTTP_URL you now need to import it from @opentelemetry/semantic-conventions/incubating.
2. Separately, the OTel Semantic Conventions team stabilized HTTP-related semconv. Part of that stabilization involved deprecated http.url (which is exported as ATTR_HTTP_URL from the 'incubating' entry point) in favour of url.full (which is exported as ATTR_URL_FULL from both the top-level and 'incubating' entry points).

They are two different kind of deprecations: 1. a JS package export-deprecation and 2. a semconv-deprecation.

options

First, I think we'd want to update the templates to have the ATTR_HTTP_URL deprecation message point to ATTR_URL_FULL rather than url.full. And perhaps clarify which entry point to use. I think every semconv deprecation is for a new var that is stable, so perhaps it is always the top entry point.

Second, to update the deprecation messages for SEMRESATTRS_ and SEMATTRS_ vars means we'll need to manually update the relevant files. They are no longer part of the generation -- because they are intentionally frozen. We could either:

1. Update those messages to point to the ATTR_... equivalent, even if it is semconv-deprecated. E.g. point SEMATTRS_HTTP_URL to ATTR_HTTP_URL. Perhaps the wording of the deprecation could make it clear that it is an "export-deprecation" (as I've called it) unrelated to semconv stabilization.
2. Or, update those messages to point to ultimate stabilized attribute, if there is one (e.g. point SEMATTRS_HTTP_URL to ATTR_URL_FULL).

I think I'm in favour of option 1. There are two things going on here. Directing a user directly from SEMATTRS_HTTP_URL to ATTR_URL_FULL is possibly skipping some detail they should know about (e.g. the OTEL_SEMCONV_STABILITY_OPT_IN handling).

[trentm]

I was playing a bit. Take this excerpt from "semantic-conventions/src/experimental_attributes.ts":

/**
 * Deprecated, use `url.full` instead.
 *
 * @example https://www.foo.bar/search?q=OpenTelemetry#SemConv
 *
 * @experimental This attribute is experimental and is subject to breaking changes in minor releases of `@opentelemetry/semantic-conventions`.
 *
 * @deprecated Replaced by `url.full`. See {@link ATTR_URL_FULL}, {@link ATTR_HTTP_TARGET}.
 */
export const ATTR_HTTP_URL = 'http.url' as const;

I added the See {@link...content. The second link, {@link ATTR_HTTP_TARGET}, to a variable in the same file works somewhat nicely in VS Code hovertips:

However the {@link ATTR_URL_FULL} -- the one we actually want to use -- does not actually link up. At least not in the cobbled up example I have here.

Still, it might be useful for us to do this. Note that doing so will be a heuristic. It will be having the jinja template process the Replaced by `url.full`. string (or perhaps the Deprecated, use `url.full` instead. "brief" string), pull out the backtick-quoted field, assume it is a semconv string, convert it to the ATTR_... or METRIC_... name and then emit See {@link ...}.

[trentm]

However the {@link ATTR_URL_FULL} -- the one we actually want to use -- does not actually link up. At least not in the cobbled up example I have here.

FWIW, the limitation in linking to some export in another file is discussed here: https://stackoverflow.com/questions/68611099/vscode-only-renders-jsdoc-link-to-another-file-when-symbol-is-imported

[trentm]

https://github.com/open-telemetry/opentelemetry-js/pull/5160 is a first change for this. It updates refs in @deprecated ... messages to point to constants that now actually exist. I'll have other changes after this one is merged.