open-telemetry / opentelemetry-specification

Specifications for OpenTelemetry
https://opentelemetry.io
Apache License 2.0
3.69k stars 886 forks source link

Define how providers handle instrumentation scope attributes and identical creation calls #4160

Open pellared opened 1 month ago

pellared commented 1 month ago

Towards https://github.com/open-telemetry/opentelemetry-go/issues/3368

Most of the SDKs ignore instrumentation scope attributes (C++ and PHP are exceptions). This is a functional blocker for following issues:

The specification does not explicitly stay how the SDK should handle tracer/meter/logger creation when the user calls it with the same identifying parameters (name, version, scheme_url), but when other parameters are different (currently only attributes).

Because of this the Go SDK ignores instrument scope attributes (see: https://github.com/open-telemetry/opentelemetry-go/issues/3368). I think the same is true for Java SDK, .NET SDK, JS SDK.

C++ SDK creates only a single tracer for many "identical" creation calls (the instrumentation scope attributes value comes from the first "identical" tracer creation call). However, it is only for OPENTELEMETRY_ABI_VERSION_NO=2 which is experimental.

Python SDK, PHP SDK create always a new instance of a tracer for many "identical" creation calls (with potentially different instrumentation scope attributes).

Right now, the specification does not require the SDK to handle scope attributes. Currently, having them ignored is specification compliant.

It is also unclear why scope attributes are not taking part of being one of the "identifying fields" and why all fields cannot be used to identify an instrument. From my database experience adding a column to a primary key it is an acceptable change/migration. However, in order to include instrumentation scope attributes to the identifying fields we would need to define the equality of Attribute Collections. In my opinion, the order or attributes should not affect the equality of attribute ([k1: v1, k2: v2] should be equal to [k2: v2, k1: v1]), especially that some languages model the attributes as map/hashset.

"Identical" and "identifying fields" terms are (currently) relevant and used a lot in metrics' duplicate instrument registration section.

Prior discussions: https://github.com/open-telemetry/opentelemetry-specification/pull/4146

pellared commented 1 month ago

I made a little draft/POC PR https://github.com/open-telemetry/opentelemetry-specification/pull/4161.

I can look into it further (e.g. make a prototype in Go SDK) no sooner than on September. However, I may be more effective if someone who has been working more on the metrics specs/SDKs would take this issue after me (or with me).

carlosalberto commented 1 month ago

From the TC call last week: we think the current behavior described in the Specification can be considered a bug or an error, and we would like to recommend that getTracer/getMeter/getLogger calls return instances reflecting the actual passed attributes in every call.

However, we want to verify with SIGs that this is fine, so users do not suffer because of the behavior change.

lzchen commented 1 month ago

The specification does not explicitly stay how the SDK should handle tracer/meter/logger creation when the user calls it with the same identifying parameters (name, version, scheme_url), but when other parameters are different (currently only attributes).

I think the spec does not explicitly state how SDK should handle tracer/meter/logger creation even with the same identifying parameters (name, verison, schema_url) even WITHOUT differing attributes correct? (from the spec -> When more than one Tracer of the same name, version, and schema_url is created, it is unspecified whether or under which conditions the same or different Tracer instances are returned.).

Because of this the Go SDK ignores instrument scope attributes

Just a small correction to the above, Python SDK does support scope attributes for tracer/meter/logger creation with similar implementation details to PHP SDK in which we creating multiple instances for the same "identifying call".

pellared commented 1 month ago

Just a small correction to the above, Python SDK does support scope attributes for tracer/meter/logger creation with similar implementation details to PHP SDK in which we creating multiple instances for the same "identifying call"

Thanks. Description updated.

I also discovered that the C++ behavior and API is experimental which is available when OPENTELEMETRY_ABI_VERSION_NO=2 is defined.

pellared commented 1 week ago

@open-telemetry/technical-committee, I can be a sponsor as I am already working on this. See https://github.com/open-telemetry/opentelemetry-specification/pull/4161