open-telemetry / semantic-conventions

Defines standards for generating consistent, accessible telemetry across a variety of domains
Apache License 2.0
288 stars 176 forks source link

Should library-specific attributes be added to semantic conventions? #1218

Open JamieDanielson opened 4 months ago

JamieDanielson commented 4 months ago

Area(s)

area:telemetry

Is your change request related to a problem? Please describe.

Note: I am primarily working on JS so those are the examples I'll be using, but this is likely to be relevant across languages.

As we look toward stabilizing instrumentations, we are revisiting attributes generated by instrumentation libraries. Most attributes are defined in semantic conventions, like http attributes and db attributes. Others, however, are library-specific and provide necessary value (e.g. for ExpressJS express.type: middleware or express.name: jsonParser).

The specification states "Stable instrumentations authored by OpenTelemetry SHOULD NOT produce telemetry that is not described by OpenTelemetry semantic conventions." This was discussed in comments on the spec PR that added this statement, and at the time it was determined that the term SHOULD (vs. MUST) allowed for deviation.

At this point we have generally kept (or sought) consistency in directory structure and naming for those other (src/enums/AttributeNames), but it's unclear whether this is still a sufficient approach long-term.

Describe the solution you'd like

I don't think there is an "ideal" solution. There are likely to be a few options to consider with different tradeoffs. It is unfeasible to have all library-specific attributes listed in the attribute registry in semantic conventions. But there is a desire for consistency and stability.

Describe alternatives you've considered

There may be some commonalities that can be extracted out (an attribute with a value of middleware is not uncommon). For others, we may consider a prefix to avoid possible naming collisions (e.g. oteljs.layer.name:jsonParser?). It may be that each language has a separate semantic conventions package for library-specific attributes, and that is used as a guide for other instrumentation authors in the same language as well?

Additional context

Other issues and PRs suggest this may have just been a lower priority as we've all been focusing on the general attributes (valid), and it's been kicked down the road until it was necessary to address. I think that time is coming upon us. This (now closed) otep PR seems like it was intended to take steps toward addressing this, but ultimately it had too far-reaching a scope. A similar issue exists for library-specific metric semantic conventions, and it was included in the working group project plan and proposal.

lmolkova commented 4 months ago

There are a lot of precedents to have library-specific attributes in semantic conventions.

E.g.

I don't think it makes sense to unify things for different web frameworks - we already have HTTP server spans and such. Having some unique set of attributes for express sounds reasonable.

I believe Java guards all undocumented attributes behind experimental feature flag which could be a good approach if there are things that we're not ready to document/stabilize.

joaopgrassi commented 4 months ago

The only concern of having these framework specific attributes to the semconv repo, is who will maintain them. For the .NET/ASP.NET we are in a good place as there's a specific team for it, but I'd be surprised if we are in a place where that's always the case. This may cause the semconv repo to become bloated and with lots of unmaintained conventions (as it happens in may contrib sigs).

JamieDanielson commented 4 months ago

I agree that including all of the attributes for all the instrumentation frameworks in the main semconv repo is not feasible. I am mainly wondering if we want to provide guidance for non-semconv attributes, to try to help with relative consistency across packages and ecosystems, especially as more libraries consider native telemetry. Even, for example, "As a general guideline, attributes should be prefixed with the short version of the package name and namespaced with a dot". This guidance may be helpful in preventing naming collisions as well.

If the stance right now is still "let each framework in each language decide what seems best for that use case", that is still a valid stance.

joaopgrassi commented 3 months ago

I think we can definitely consider adding a guidance section for library authors. We already have guidance on like metric names, which instrument type to use, attribute names and so on.

But I think their definition is best if kept close to the library, otherwise it will become bloated here in semconv.