WIP: Make Trace compatible with OpenTelemetry

[davidpdrsn]

OpenTelemetry defines a bunch of conventions for HTTP
spans. This makes adopting those conventions easy when
using Trace:

.layer(TraceLayer::new_for_http().opentelemetry_server())

This changes the span for the request to:

• Name: HTTP request
• Fields:
  • http.flavor: "1.1"
  • http.host: "localhost:3000"
  • http.method: "GET"
  • http.route: "/users/123". Can be configured by user to instead be
    the path pattern for the route, ie /users/:id
  • http.scheme: "http". Must be set by user as middleware don't know
    this
  • http.target: "/users/123"
  • http.user_agent: "curl/7.64.1"
  • http.client_ip: Must be extracted by user in a "make service"
  • http.status_code: 200
  • request_id: Requires Add SetRequestId and PropagateRequestId middleware #150.
  • otel.kind: "server"
  • otel.status_code: Unset or "ERROR"
  • exception.message: Set by user based on response classification.
  • exception.details: Set by user based on response classification.

The fields are a bit different for HTTP clients hence the name
opentelemetry_server. I don't think supporting clients initially is
required. We can always add it later.

TODO

• Add Trace::opentelemetry_server method. Currently it only exists
  on the layer
• Docs, including getting request ids
• Example using axum and opentelemetry_jaeger
• Changelog
• Request id integration
• Implement callbacks for functions

[davidpdrsn]

When #181 is merged I'm gonna update it to use an opentelemetry config.

[davidpdrsn]

I've removed this from the 0.2 milestone. Been thinking that it might be overly complicated and that a simpler solution should be possible. So think it needs more time to bake. Since its not a breaking change it doesn't have be included in 0.2

[Oliboy50]

when this will ship, I'll definitely replace the use of warp's trace filter with tower + tower-http in my projects 🙏

the biggest fail for "warp's trace filter" when working with OpenTelemetry, is that it does not set http.status_code in spans (and it is really difficult to hook in it to set it ourselves => I didn't succeed)

[Oliboy50]

http.route: "/users/123". Can be configured by user to instead be the path pattern for the route, ie /users/:id

@davidpdrsn It would be an amazing feature 😍
This is so much useful to group incoming requests or spans by their name or "static path" instead of their http.target in dashboards... but I don't really get how we could do that with the current PR?

Could you give an example of usage you have in mind to go from /users/123 to /users/:id?
Is it something crate users will have to do manually for each path or will it be possible to let tower handles it automatically?

Besides, I don't think having /users/123 as default value for http.route would be useful, we already can use http.target for this purpose.

The OTEL doc says:

http.route | string | The matched route (path template). | /users/:userID? | No

So it doesn't feel like a good default value to set the http.target value here... but I understand that it will be hard to have it "automatically" set by tower-http (even harder if our routing is hidden behind a warp service).

[davidpdrsn]

I don't think this will ship any time soon, definitely not in its current form. I think the implementation in this PR is too complicated. It shouldn't require implementing four traits...

The /users/1 vs /users/:id issue is part of it. I'm not sure whether frameworks allow middleware to get the matched path rather than the literal request path. With axum its possible with MatchedPath but I don't know about warp.

I imagine that someday I (or someone else) will make a crate that contains an axum specific middleware. At Embark Studios we've been working on an implementation here but its not published to crates.io yet. Don't have a timeline on that I'm afraid.

In fact I think I'll just close this PR. I don't like having stale PR lingering.