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 db|messaging|rpc.system be an instrumentation scope attribute? #803

Open
lmolkova opened this issue Mar 8, 2024 · 5 comments
Open
Assignees

Comments

@lmolkova
Copy link
Contributor

lmolkova commented Mar 8, 2024

*.system is a compile-time constant which is repetitive on spans and is a good candidate for instrumentation scope attribute.

AFAIK, not every OTel api impl supports them (dotnet/runtime#93795, open-telemetry/opentelemetry-java#5503)

@github-project-automation github-project-automation bot moved this to V1 - Stable Semantics in Spec: Messaging Semantics Mar 8, 2024
@lmolkova lmolkova moved this from V1 - Stable Semantics to Post-stability in Spec: Messaging Semantics Mar 8, 2024
@lmolkova lmolkova moved this from Post-stability to In Triage in Spec: Messaging Semantics Mar 8, 2024
@pyohannes
Copy link
Contributor

Should semantic conventions specify which attributes originate from instrumentation scopes and which not?

Or asked differently, is an instrumentation which currently adds db.system via an instrumentation scope currently conforming to semantic conventions (which defines it as a span attribute), or not?

@lmolkova
Copy link
Contributor Author

lmolkova commented Mar 12, 2024

@pyohannes great questions, I don't think any of it is defined - https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/glossary.md#instrumentation-scope.

Given that Java does not support scope attributes, I assume nobody uses them.

I think the future reasonable solution would be that attributes can appear on span attributes or instrumentation scope attributes or even resource attributes (on the broker side) and ideally should be query-able in the same way regardless of how they were added. If that's the case, this is not blocking current db/messaging efforts.

I'd like to come up with a desirable outcome from semconv group that we can bring to the general spec.

@joaopgrassi
Copy link
Member

Today we usually have documents such as http-spans.md, http-metrics.md, but nothing for scope, true. I think we could "merge" scope attributes into these existing documents in case we want to add any.

@gregkalapos
Copy link
Member

This was briefly discussed in one of the Database Client Semconv Stability meetings, so raising it here as well: this could be problematic for higher lever frameworks, like ORMs - e.g. Hibernate or Entity Framework (Core).

Those can e.g. talk to multiple databases systems - of course it depends which layer is instrumented (the ORM or the underlying database library), but e.g. EF Core has an issue to directly support OTel, and there is an external instrumentation library doing so, which sets db.system at runtime. It's also possible that the library talks to multiple database systems at the same time and emits spans with different db.system values.

There are similar libraries for messaging and rpc as well.

I think the future reasonable solution would be that attributes can appear on span attributes or instrumentation scope attributes or even resource attributes (on the broker side) and ideally should be query-able in the same way regardless of how they were added.

Could then be in the spec something like, in general db|messaging|rpc.system are instrumentation scope attributes, but libraries still have the option to set it on the signal level?

Also, sorry for the noob question, but for my understanding:

*.system is a compile-time constant

Where is that defined? Sorry for my missing knowledge here, but I wasn't aware there is guideline on this. I understand that in case of most of the libraries (e.g. let's take MongoDb) it makes sense to have a compile-time constant (db.system: mongodb), but is it defined that it has to be a compile-time somewhere?

@lmolkova
Copy link
Contributor Author

@gregkalapos thanks for the clarification!
It's not required for *.system to be a compile-time constant and it's not documented anywhere. It's more like what you explained - sometimes it is, but not all the time.

I believe we also discussed that we're not targeting solving it for db semconv stability and we're going to assume it can be put on span attributes.

If in the future we can optimize it with instrumentation scope attributes, it'd be great, but not essential for DB semconv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Post Stability
Status: Post-stability
Development

No branches or pull requests

5 participants