Enable optional tracing via OpenTelemetry

[neoeinstein]

In order to support distributed tracing scenarios, it would be nice to optionally integrate creation and sending of spans according to the OpenTelemetry specification for HTTP clients and servers.

As hyper forms a base layer to many HTTP libraries, enabling tracing via a crate feature would allow an application to be able to automatically trace incoming and outgoing requests as a cross-cutting concern, even if provided by some other library, or through some higher-level library such as reqwest or awc, among others.

I'm planning on doing a proof of concept implementation for this where the functionality will be behind an otel crate feature. Costs for the tracing functionality must only be incurred when this crate feature is enabled.

[olix0r]

Hyper uses tracing internally:

hyper/Cargo.toml

Line 36 in 8c89a8c

tracing = { version = "0.1", default-features = false, features = ["std"] }

You should be able to instrument what you want with tracing-opentelemetry.

[neoeinstein]

Not quite. Part of opentelemetry includes some semantic attributes that should be added to certain types of spans. It's possible that these could just be straight up added to hyper, and then tracing-opentelemetry would benefit from them transparently, but will require adding some spans around the actual request/response, up to consumption of the response body, and decorating them with attributes for the URL being accessed, the method being used, and the HTTP version/flavor. In addition, having a way for a user to provide a low-cardinality name for the span is valuable too, but having a static span name could be sufficient.

The way that tracing is currently used in hyper completely misses this use case.

[davidpdrsn]

tower-http has a tracing middleware that should be configurable to match opentelemetry https://docs.rs/tower-http/0.1.0/tower_http/trace/index.html

It's something I have considered adding built in support for in tower-http but since I don't use opentelemetry myself, I haven't done it. But would be cool to have. Contributions are very welcome.

I'm personally not sure if it makes sense for something like this to be built in to hyper. Feels too high level 🤔

[neoeinstein]

The issue is looking at doing this in a way that works as a cross-cutting concern. If there were a way to inject some behavior before and after all hyper requests, that might be more ideal and extensible. Perhaps even something like a way to inject a global service middleware on requests.

Otherwise, it becomes something where this needs to be reimplemented on top of pretty much every other library that might make outgoing requests. For some libraries (like, say, one for using the AWS API), it becomes impossible to then inject this at the right level.

I'd be happy to make a contribution to tower for a service that could do this logging in an opentelemetry-compatible way, but having a way to get these events directly out of hyper would be preferred and handle 90%+ of the ecosystem.

[davidbarsky]

(important disclosure: I worked on the AWS SDK and contribute to tokio-rs/tracing. I also apologize for not editing this post as well as I should have.)

The issue is looking at doing this in a way that works as a cross-cutting concern. If there were a way to inject some behavior before and after all hyper requests, that might be more ideal and extensible. Perhaps even something like a way to inject a global service middleware on requests.

I believe that a defining your own connector (tower::Service<http::Uri>) and passing it to a client builder should provide you the mechanism to add the instrumentation you need. If it doesn't, then the lower-level conn client should provide you the ability to do so, at the cost of you needing to do DNS resolution and connection pooling yourself.

Otherwise, it becomes something where this needs to be reimplemented on top of pretty much every other library that might make outgoing requests.

If the libraries are using Hyper but not exposing a way to configure the Hyper client, I'd argue that warrants opening a issue on the library.

For some libraries (like, say, one for using the AWS API), it becomes impossible to then inject this at the right level.

As of https://github.com/awslabs/aws-sdk-rust/releases/tag/v0.0.8-alpha, I'm decently confident you can add additional Tower layers to emit data in an OpenTelemetry-compliant fashion. This obviously doesn't extend to every library, but I think it's reasonable to push libraries to expose configuration knobs for Tower middleware (if they're using Tower!) or sending a PR to change/improve their logging.

I'd be happy to make a contribution to tower for a service that could do this logging in an opentelemetry-compatible way, but having a way to get these events directly out of hyper would be preferred and handle 90%+ of the ecosystem.

At the risk of speaking for Sean, I don't think that OpenTelemetry should be baked into a library like Hyper and would push for richer extensions to tokio-rs/tracing and Tower instead. Despite OpenTelemetry's widespread adoption, tokio-rs/tracing can be used for more than just OpenTelemetry-specific exposition.

[taqtiqa-mark]

Continuing this discussion in issue #2498, and trying to gather input for an effort to improve tracing. @neoeinstein would the current proposal made there be sufficient from your PoV?

[seanmonstar]

At the risk of speaking for Sean, I don't think that OpenTelemetry should be baked into a library like Hyper and would push for richer extensions to tokio-rs/tracing and Tower instead.

I agree. hyper will have tracing support (see #2678), and from that, it should be possible to integrate with HTTP request tracing services with the help of another library. Therefore, I'm going to close this specific request.