feat: Harmonize HTTP client instrumentation

[tt]

This attempts to harmonize the HTTP client instrumentation by adopting the latest semantic conventions, version 1.26.0.

Each gem currently sets a slightly different set of attributes which makes it harder to create good dashboards in an environment where more than one HTTP client is used.

My change adopts the latest semantic conventions but doesn't retire what's already there. That seems like a better fit for the next major release which will allow a seamless migration.

Notably, this pull request makes the following HTTP client span attributes available:

• http.request.method
• http.response.status_code
• server.address
• server.port
• url.full
• url.scheme

There are no constants available for these yet as the opentelemetry-semantic_conventions gem was last updated in open-telemetry/opentelemetry-ruby@159044d74ce006cdfda8bb344b1f9c2267d7f040 to version 1.10.0.

See also this summary of changes and the list of deprecated attributes.

[linux-foundation-easycla[bot]]

• :white_check_mark:login: tt / (39221d1b816d30b7261705519424cb88e605f8ba)
• :white_check_mark:login: tt / (39221d1b816d30b7261705519424cb88e605f8ba, 2ac3f835bde4ac3c353ce08ad7b2ba637099e3be)
• :white_check_mark:login: tt / (39221d1b816d30b7261705519424cb88e605f8ba, 2ac3f835bde4ac3c353ce08ad7b2ba637099e3be, a42aa744ccebb913fa250f126a7f61e51457ab51)
• :white_check_mark:login: tt / (39221d1b816d30b7261705519424cb88e605f8ba, 2ac3f835bde4ac3c353ce08ad7b2ba637099e3be, a42aa744ccebb913fa250f126a7f61e51457ab51, 49d0d63e40ca0b0ce483242a02869eb7799c113a)
• :white_check_mark:login: tt / (39221d1b816d30b7261705519424cb88e605f8ba, 2ac3f835bde4ac3c353ce08ad7b2ba637099e3be, a42aa744ccebb913fa250f126a7f61e51457ab51, 49d0d63e40ca0b0ce483242a02869eb7799c113a, ea385d72ddb097e1c70e6b6ea340e158b39274cf)
• :white_check_mark:login: tt / (39221d1b816d30b7261705519424cb88e605f8ba, 2ac3f835bde4ac3c353ce08ad7b2ba637099e3be, a42aa744ccebb913fa250f126a7f61e51457ab51, 49d0d63e40ca0b0ce483242a02869eb7799c113a, ea385d72ddb097e1c70e6b6ea340e158b39274cf, 6a6d46d6f686c7e8ed87acdbb6a2a733ead5abc0)

The committers listed above are authorized under a signed CLA.

[tt]

I saw to use conventional commit messages but also that this will be squash merged. I used the style for the pull request title and not for individual commits but happy to change it if necessary.

[tt]

@arielvalentin:

Though I recognize this is extremely challenging, many of our users are relying on pre 1.0 schema name, so having the ability to support http/dup would be important for us to provide during a transition.

I agree this is important. I also want overlap for our current traces. That said, isn't this essentially the same as what I've done where both sets of attributes are being included?

I think the downside of not supporting these new conventions is that people starting on their observability journey today will have to go through a migration that could be avoided.

Anyway, it seems you could achieve this entirely through releases:

1. Version 1.0 contains old set ("default behavior")
2. Version 1.1 contains old and new set (http/dup equivalent)
3. Version 2.0 contains new set (http equivalent)

I can certainly add OTEL_SEMCONV_STABILITY_OPT_IN support if that's the preferred direction but I'm probably not going to do it for all HTTP clients. We fortunately don't use all of them. 😅

[tt]

@arielvalentin, any appetite for converging the existing instrumentation towards the now deprecated attributes?

For instance, look at the instrumentation for Excon and Faraday:

https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/aedc42cbb4bf2f6aba5d4e4a69c9fa45aacd172a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb#L29-L36

https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/da831f0b622830364197d48fb03c9615f8231249/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb#L49-L55

These are mostly different sets. I don't strictly need the newest conventions but having a common set of attributes would be really valuable.

[arielvalentin]

any appetite for converging the existing instrumentation towards the now deprecated attributes?

Yes. I love symmetry and very much dislike surprises.

These instrumentations where likely written by different authors and we don't exactly have a "checklist" of what an instrumentation should emit but only that it should do the absolute minimum or order to add instrumentation.

In some of those cases there is a bit of divergence in the data provided by the libraries themselves.

In the example you referenced above, Faraday and Excon, one provides the full URL (http.url) while the other only provides the path (http.target)^1.

I assert the authors were likely trying to avoid any additional object allocation and only provided the minimal data made available to the instrumentation by the library itself; so, it's a tradeoff here but depending on system load it may not be so bad.

Faraday is another one of the odd cases, where those attributes really matter most to the underlying adapter client spans. Unlike the underlying adapters, Faraday provides a much nicer and symmetric abstraction since it has a URL object, where you may be able to extract all of the attributes and share them with the adapter spans by amending them to the OpenTelemetry::Common::HTTP::ClientContext shared attributes helper:

https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/2230403871a9ee5ed9bac4fc6a7205b7d0876cd8/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb#L58

What should we do next?

I am in favor of providing symmetric attributes in HTTP instrumentations, but if we could do so by avoiding allocating any additional objects in drivers that would be best.

If Faraday were to include all of the attributes as part of OpenTelemetry::Common::HTTP::ClientContext, would that address your use case?

If it does, then how would you feel about approaching it that way instead of updating individual drivers?

[tt]

@arielvalentin:

If Faraday were to include all of the attributes as part of OpenTelemetry::Common::HTTP::ClientContext, would that address your use case?

~I need a bit of help understanding this. Isn't OpenTelemetry::Common::HTTP::ClientContext for passing additional context in?~

I think I understand it now: You would make it the responsibility of the adapter to then add the attributes to the span. I guess that might work based on which adapter is used.

My most immediate problem was probably the lack of http.url in the Excon instrumentation so I opened a pull request for that change.

[github-actions[bot]]

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot