open-telemetry / semantic-conventions

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

Should `db|messaging|rpc.system` be an instrumentation scope attribute? #803

Open lmolkova opened 3 months ago

lmolkova commented 3 months ago

*.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 (https://github.com/dotnet/runtime/issues/93795, https://github.com/open-telemetry/opentelemetry-java/issues/5503)

pyohannes commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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.