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

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

Open
JamieDanielson opened this issue Jul 9, 2024 · 4 comments
Open
Assignees
Labels
area:telemetry enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage

Comments

@JamieDanielson
Copy link
Member

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.

@JamieDanielson JamieDanielson added enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage labels Jul 9, 2024
@lmolkova
Copy link
Contributor

lmolkova commented Jul 9, 2024

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
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:telemetry enhancement New feature or request experts needed This issue or pull request is outside an area where general approvers feel they can approve triage:needs-triage
Projects
None yet
Development

No branches or pull requests

4 participants