OkHttp3 client instrumentation doesn't respect NetPeerAttributes config

[michaelbannister]

Describe the bug
With javaagent instrumentation, and peer service mapping provided by an environment variable OTEL_INSTRUMENTATION_COMMON_PEER_SERVICE_MAPPING, an OkHttp3 client does not assign the peer.service span attribute.

Steps to reproduce
Described above

What did you expect to see?
Attribute peer.service set on the client span

What did you see instead?
No peer.service attribute set on the client span

What version are you using?
v1.2.0

Environment
Not relevant, AFAICT.

Additional context
The class io.opentelemetry.instrumentation.okhttp.v3_0.OkHttpClientTracer calls super(openTelemetry, new NetPeerAttributes()).

Changing this to super(openTelemetry, NetPeerAttributes.INSTANCE) gives the expected behaviour (tested using a local build with this change made).

The equivalent class for OkHttp 2.2: io.opentelemetry.javaagent.instrumentation.okhttp.v2_2.OkHttpClientTracer does use NetPeerAttributes.INSTANCE.

However, there's the comment below on NetPeerAttributes.INSTANCE which I don't really understand so unsure whether this fix is in the right direction or whether this should be done some other way?

TODO: this should only be used by the javaagent; move to javaagent-api after removing all library usages

If the change I suggest is right, I'm happy to raise a PR (if I can work out how to add a test for this!)

[mateuszrzeszutek]

Hi @michaelbannister,

It's okay to use NetPeerAttributes.INSTANCE, as long as it makes it work it's fine.

We're treating the tracer API as deprecated and trying to migrate to Instrumenters (#2713), so those TODOs in the tracer code can be ignored - we'll be getting rid of it all sooner or later.

[michaelbannister]

Thanks @mateuszrzeszutek, in that case I'll raise a PR with the quick fix in case that makes it into a release any time soon, and then I'll have a look at whether I can figure out how to migrate the OkHttp instrumentation to the Instrumenter API 😄