Allow attributes on a Tracer

[JnRouvignac]

Is your feature request related to a problem? Please describe.
I am working on an LDAP server and I want to add attributes to my traces.
There are several level of attributes:

• server-related attributes, for which I am using resource attributes
• connection-related attributes, for which I am using span attributes
• request-related attributes, for which I am using span attributes

In LDAP, connections are persistent, and several requests can be multiplexed on the same connection.
Therefore repeating connections attributes in every request spans is a waste of bandwidth for the span exporter.

Describe the solution you'd like
Similar to #4473, I was thinking it would be interesting to add attributes to "instrumentation scope", i.e. at the tracer level. This would require creating one tracer per connection.

Problem is we have not been able to find an API to do that opentelemetry-sdk-trace :)

Describe alternatives you've considered

For the moment, connection attributes are duplicated into each span's attributes, but this is not ideal for bandwidth of the span exporter.

Using reflection we have been able to hack the internal fields, and we verified the connection attributes are no longer duplicated in each span:

        final var scopeInfoField = tracer.getClass().getDeclaredField("instrumentationScopeInfo");
        scopeInfoField.setAccessible(true);
        final var scopeInfo = (InstrumentationScopeInfo) scopeInfoField.get(tracer);
        final var attributesField = scopeInfo.getClass().getDeclaredField("attributes");
        attributesField.setAccessible(true);
        attributesField.set(scopeInfo, scopeInfo.getAttributes()
                                                .toBuilder()
                                                .put(SERVER_ADDRESS, connection.getServerAddress())
                                                .put(SERVER_PORT, connection.getServerPort())
                                                .put(CLIENT_ADDRESS, connection.getClientAddress())
                                                .put(CLIENT_PORT, connection.getClientPort())
                                                .put(NETWORK_PROTOCOL_NAME, connection.getProtocol()
                                                                                      .toLowerCase(ROOT))
                                                .put(NETWORK_PROTOCOL_VERSION, Integer.toString(ldapVersion))
                                                .build());

The current internal API has all the plumbing needed, except it's missing a public API to set the attributes:

        var openTelemetrySdk = OpenTelemetrySdk.builder().build();
        var sdkTracer = openTelemetrySdk.getTracerProvider()
                .tracerBuilder("instrumentationScopeName")
                // .setAttributes(Attributes.of(stringKey("key"), "value")) // Such an API does not exist 
                .build();

Additional context

Given that some LDAP connections are long lived, while some others are not (some clients open a connection, send a request, then close the connection), I have become worried that ComponentRegistry will become a memory leak over time because it has no flushing mechanism in place to remove metadata for old unused tracers.

[breedx-splk]

I suspect that this is bound to be tricky for a few reasons, not the least of which is a specification api (unless that already exists, which the above linked issue alludes to, I haven't checked).

All "components" in the sdk are cached in the ComponentRegistry to facilitate reuse instead of recreating things. These components are stored by name -- so it isn't really possible (practical?) to have more than one instance of the same kind of tracer with the same name.

In your case, it sounds like you are (or want to) create multiple tracers of the same type but with different attributes. Unfortunately, I think the design doesn't really afford this. In the hack above, I think (please maintainers correct me if I'm wrong) that you are mutating the single instrumentation scope instance for a single tracer instance. I suspect that same tracer is used across multiple LDAP requests, and when you're changing those attributes you're changing it in a scope larger than you expect. That's my quick $0.02.

I do like this idea though, just not sure how to get there. I don't think a setter is enough.

[trask]

hi @JnRouvignac, is this a duplicate of #4695?

[JnRouvignac]

Hi @trask,

Yes! #4695 looks like a more general issue which covers what I am asking for here indeed.
Thanks for pointing it out! I could not find it with my search criteria (this may be because there is no mention of "tracer" in the issue description).
In particular, PR #4711 seemed to be doing exactly what I wanted, but by reading through the comments, it's not totally clear to me why it was not merged into the main branch.

Anyway, also note that the current issue is more use case oriented, and it also highlights a potential memory leaks issues with the ComponentRegistry.

Based on this observation, I don't know how you would like to deal with the current issue?
I am fine if you prefer to close it and keep #4695 instead.

[JnRouvignac]

There is something very important that I'd like to point out: this use case equally applies to HTTP/2 connections.
HTTP/2 connections are like LDAP connections: they are persistent and requests can be multiplexed on them.

Therefore, they can hit the exact same problem of repeating the same attributes in request spans. I think this raises the priority of the use case reported here.

[JnRouvignac]

@breedx-splk Thanks for taking the time to analyze the use case, and write an answer!
This is great feedback.

All "components" in the sdk are cached in the ComponentRegistry to facilitate reuse instead of recreating things. These components are stored by name -- so it isn't really possible (practical?) to have more than one instance of the same kind of tracer with the same name.

All the connections are identified by aconnection ID, so I was thinking that maybe we could use it as the version?
Then different tracers would end up being effectively separated in the ComponentRegistry#componentByNameAndVersion field.

I am not sure if this is a correct use because I have not fully understood the design around the instrumentationScopeName, instrumentationScopeVersion and schemaUrl, i.e. how it is supposed to be used.

In your case, it sounds like you are (or want to) create multiple tracers of the same type but with different attributes. Unfortunately, I think the design doesn't really afford this. [...] I suspect that same tracer is used across multiple LDAP requests, and when you're changing those attributes you're changing it in a scope larger than you expect.

Yes, I fully agree, this does not work for the use case that I presented. Well spotted.

Alas in that case, the ComponentRegistry seems to create more problems than it solves.
I would be better off without it, but removing it is a big change in behaviour for the library, which is probably unacceptable.
The only path forward I see would be to make the ComponentRegistry pluggable, and replacing it with a no-op registry.
But I do not know if that's an acceptable solution for maintainers, and maybe they have other better ideas too more inline with future plans. We shall see.

[JnRouvignac]

It looks like the ComponentRegistry solves the problem of continuously recreating tracers (compared to keeping them around and reusing them). I wonder why this use case has been considered important compared to saying "cache your own tracers please". 🤔