feat: add attributes support to instrumentation scope for tracers#2094
feat: add attributes support to instrumentation scope for tracers#2094robertlaurin wants to merge 2 commits intoopen-telemetry:mainfrom
Conversation
17bac28 to
6aceef6
Compare
89a3d55 to
b042e7b
Compare
The OpenTelemetry spec (stable as of v1.24.0) defines an `attributes`
parameter on `TracerProvider#tracer` as part of the instrumentation
scope. This enables instrumentations to attach metadata to the scope
that produced their spans.
The `tracer` method now accepts both legacy positional arguments and a
new keyword-based interface. When both are provided for the same
parameter, keyword arguments take precedence.
# Legacy positional (unchanged, fully supported)
tracer('name', '1.0')
# New keyword interface
tracer(name: 'name', version: '1.0', attributes: { 'key' => 'value' })
Backwards compatibility across independent gem releases is preserved:
- New API + Old SDK: ProxyTracerProvider introspects the delegate's
method signature at assignment time and omits the attributes keyword
when delegating to providers that do not support it.
- Old API + New SDK: The SDK's tracer method accepts positional
arguments via splat, so old callers continue to work unchanged.
API changes:
- TracerProvider#tracer signature: (*args, name:, version:, attributes:)
- ProxyTracerProvider stores attributes in its registry key and
delegates them when the underlying provider supports it
SDK changes:
- TracerProvider#tracer passes attributes through to Tracer
- Tracer#initialize accepts attributes and attaches them to
InstrumentationScope
- InstrumentationScope struct gains an attributes field
- Attributes are normalized (nil -> frozen empty hash) and frozen
to prevent mutation of registry keys
b042e7b to
7dd5cce
Compare
|
This is on my list to review today/this week. I had just started planning a proposal to implement Is "scope-level attributes" and "scope schema_url" separate enough concerns to implement in separate PRs or should we combine forces? |
|
|
||
| it 'prefers keyword arguments over positional arguments' do | ||
| tracer1 = tracer_provider.tracer('positional', '1.0') | ||
| tracer2 = tracer_provider.tracer('keyword', '2.0', name: 'keyword', version: '2.0') |
There was a problem hiding this comment.
🤔
| tracer2 = tracer_provider.tracer('keyword', '2.0', name: 'keyword', version: '2.0') | |
| tracer2 = tracer_provider.tracer('positional', '1.0', name: 'keyword', version: '2.0') |
I think these suggested tracer1 positional params given to tracer2 will properly exhibit the kw arg precedence. When all arguments are entirely different, tracer2 will always be different than tracer1.
| def tracer(deprecated_name = nil, deprecated_version = nil, name: nil, version: nil, attributes: nil) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity | ||
| name ||= deprecated_name || '' | ||
| version ||= deprecated_version || '' | ||
| attributes = attributes&.dup&.freeze || EMPTY_ATTRIBUTES |
There was a problem hiding this comment.
Hmm. We should probably do attribute validation here like we do on spans.
|
I'm a fan of all of this. Scope attributes! Are these intentionally out-of-scope for this PR?
|
| @@ -37,21 +37,50 @@ def delegate=(provider) | |||
|
|
|||
| @mutex.synchronize do | |||
| @delegate = provider | |||
There was a problem hiding this comment.
Rather than have conditional code executed on every provider.tracer(...) call, could we conditionally wrap the provider before the mutex with a legacy provider that just drops the attributes: arg? I.e. (roughly)
class LegacyProviderWrapper
def initialize(legacy_provider)
@legacy_provider = legacy_provider
end
def tracer(name, version, attributes:)
@legacy_provider.tracer(name, version)
end
end
# then above
provider = LegacyProviderWrapper.new(provider) unless provider.respond_to?(:tracer) && provider.method(:tracer).parameters.any? { |_, n| n == :attributes }This would avoid penalizing providers implementing the new signature (which should be the common case, assuming most people use the SDK provider) and avoids the conditional dispatch in the legacy case.
Conditionally wrap the delegate in a LegacyProviderWrapper that drops the attributes: kwarg, rather than branching on every tracer(...) call. This avoids penalizing providers implementing the new signature.
The OpenTelemetry spec (stable as of v1.24.0) defines an
attributesparameter onTracerProvider#traceras part of the instrumentation scope. This enables instrumentations to attach metadata to the scope that produced their spans.The
tracermethod now accepts both legacy positional arguments and a new keyword-based interface. When both are provided for the same parameter, keyword arguments take precedence.Backwards compatibility across independent gem releases is preserved:
API changes:
SDK changes: