Azure / azure-sdk-for-rust

This repository is for the active development of the Azure SDK for Rust. For consumers of the SDK we recommend visiting Docs.rs and looking up the docs for any of libraries in the SDK.
MIT License
717 stars 249 forks source link

tracing sdk operations #1615

Open rbtcollins opened 9 months ago

rbtcollins commented 9 months ago

I'm happy to put up a patch for this, but thought it should be discussed first.

The problem to be solved is getting visibility into HTTP requests made by the azure SDK.

The obvious thing to do is add a dependency on tracing, and call #[tracing::instrument] on execute_request. Perhaps with skip_all as a parameter, to avoid logging the request.

However this isn't particularly great for being able to categorise things, and it won't use standard otel metadata nor the http status etc, unless more work is done.

We use https://crates.io/crates/reqwest-tracing to do 2 things: 1) add a span with otel standard metadata to each request 2) trigger otel trace propogation to the next service.

2) is probably less relevant to azure (unless azure has a service like AWS X-Ray, where customer traces can integrate deeply) ? However the DisableOtelPropagation extension can be used to disable that.

But 1) is really quite nice - and it means less to maintain.

An alternative approach would be to just implement HTTPClient on a newtype wrapper of ReqwestMiddlewareClient myself - which I haven't tested yet - but that would need a way of injecting that into the various factories like DefaultAzureCredentialBuilder to keep ergnomics nice, and is going to require keeping the implementation in sync with the reqwest one in the sdk

rbtcollins commented 9 months ago

One friction point in an external implementation of HttpClient: try_from_method is not exposed, along with to_headers and try_from_status.

If those were From / TryFrom implementations, that would expose them quite cleanly, I think.

rbtcollins commented 9 months ago

... and PinnedStream is private

rbtcollins commented 9 months ago

As a comparison point, the google rust sdk explicitly uses request-middleware : https://github.com/yoshidan/google-cloud-rust/tree/main/storage#passing-a-custom-reqwest-middleware-cliemt