Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement tracing for client methods, transports #1881

Open
heaths opened this issue Oct 29, 2024 · 6 comments
Open

Implement tracing for client methods, transports #1881

heaths opened this issue Oct 29, 2024 · 6 comments
Assignees
Labels
Azure.Core The azure_core crate Client This issue points to a problem in the data-plane of the library.
Milestone

Comments

@heaths
Copy link
Member

heaths commented Oct 29, 2024

We need to implement tracing for all client methods and transports. PII needs to be sanitized and the safest option it so remove incidental printing of values e.g., derive or implement Debug and/or Display sparingly.

For now we are using the tracing crate and friends because it supports OpenTelemetry et. al. We could consider other options but these crates are fairly ubiquitous. The opentelemetry crate and friends are geared almost entirely to OpenTelemetry and would be good to understand how that is an advantage over something more generic.

@heaths heaths added Azure.Core The azure_core crate Client This issue points to a problem in the data-plane of the library. labels Oct 29, 2024
@heaths heaths self-assigned this Oct 29, 2024
@heaths
Copy link
Member Author

heaths commented Oct 29, 2024

We need to be able to define client-level fields. I opened a tracking issue for some options using the #[tracing::instrument] procedural macro. We could accomplish "client fields" with some boilerplate and/or helper functions in every client method but that gets tedious and error-prone for convenience methods.

@heaths
Copy link
Member Author

heaths commented Feb 27, 2025

For trace levels, I propose the following after talking with @LarryOsterman:

  1. Level::Error for errors.
  2. 'Level::Warn for warnings i.e., non-fatal errors (note: @lmolkova recommends not warning for retrying HTTP requests since we do it implicitly)
  3. Level::Info for client methods and similar. This is the default for #[tracing::instrument]
  4. Level::Debug for information we think developers using our crate would find useful (@lmolkova perhaps retrying a request automatically?)
  5. Level::Trace for information Azure crate developers would find useful.

I'm basing 3 through 5 partly on the fact that the tracing crate defaults to Info for their #[instrument] attribute. Assuming that most crates don't change this, it means we'd be pretty consistent with most other crates that trace through tracing...which is most of them, if they trace at all. It's basically akin to .NET's System.Diagnostics (at least for Activity and friends).

@catalinaperalta

This comment has been minimized.

@heaths

This comment has been minimized.

@RickWinter
Copy link
Member

As far as I know changing a trace level for a message isn't breaking. Thus we could start with Error, Warn, and Info. Then if needed we could add Debug, Trace later and move messages around.

@heaths
Copy link
Member Author

heaths commented Mar 1, 2025

This would significantly affect dev productivity as we debug our own tests. I'm not talking about devs using our crates, but us writing them. Stepping through async code when you're not sure exactly where the problem is exactly is incredibly unproductive.

@heaths heaths added this to the 2025-04 milestone Mar 20, 2025
@heaths heaths modified the milestones: 2025-04, 2025-05 Mar 21, 2025
@RickWinter RickWinter moved this from Untriaged to Not Started in Azure SDK Rust Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core The azure_core crate Client This issue points to a problem in the data-plane of the library.
Projects
Status: Not Started
Development

No branches or pull requests

3 participants