Skip to content

Commit

Permalink
Attach client-level span fields
Browse files Browse the repository at this point in the history
This isn't quite what we want because the fields are still buried in a "self" field, despite numerous attempts to flatten them. Opened tokio-rs/tracing#3124 to track a feature request (or similar).

We have options but most are rather error prone. Perhaps the best way is our own attribute macro that would expand to `#[tracing::instrument]` with appropriate defaults to attach client-level fields like `az.namespace`.

We could also write a helper function or two to just wrap the whole thing, which would also give us better control over the error or return value as well, like attaching `error.type` per our guidelines.
  • Loading branch information
heaths committed Oct 29, 2024
1 parent bb6ece4 commit 844eaf7
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 19 deletions.
2 changes: 2 additions & 0 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[build]
rustflags = ["--cfg", "tracing_unstable"]
46 changes: 36 additions & 10 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ publish = false
[dependencies]
serde = { version = "1.0.213", features = ["derive"] }
tokio = { version = "1.41.0", features = ["sync", "time"] }
tracing = "0.1.40"
tracing = { version = "0.1.40", features = ["valuable"] }
url = "2.5.2"
valuable = { version = "0.1.0", features = ["derive"] }

[dev-dependencies]
clap = { version = "4.5.20", features = ["derive"] }
Expand Down
46 changes: 40 additions & 6 deletions src/clients.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ use std::{collections::HashMap, fmt, sync::Arc, time::Duration};
use tokio::{sync::Mutex, time::sleep};
use tracing::{info_span, Instrument};
use url::Url;
use valuable::{Fields, NamedField, NamedValues, Structable, Valuable};

pub struct ExampleClient {
endpoint: Url,
models: Arc<Mutex<HashMap<String, Model>>>,
}

impl ExampleClient {
const NAMESPACE: &str = "Microsoft.Example";

pub fn new(endpoint: impl AsRef<str>) -> Result<Self> {
Ok(Self {
endpoint: Url::parse(endpoint.as_ref())?,
Expand All @@ -30,10 +33,11 @@ impl ExampleClient {
pub async fn get_model(&self, name: &str) -> Result<Model> {
let mut span = tracing::Span::current();
if span
.field("client")
.is_none_or(|name| name.name() == "ExampleClient")
.field(AZ_CLIENT_FIELD.name())
.is_none_or(|name| name.name() == stringify!(ExampleClient))
{
span = info_span!(target: "ExampleClient::get_model", "get_model", client = "ExampleClient");
span =
info_span!(target: "ExampleClient::get_model", "get_model", self = self.as_value());
}
async move {
sleep(Duration::from_millis(100)).await;
Expand All @@ -54,10 +58,10 @@ impl ExampleClient {
pub async fn create_or_update_model(&self, model: Model) -> Result<Model> {
let mut span = tracing::Span::current();
if span
.field("client")
.is_none_or(|name| name.name() == "ExampleClient")
.field(AZ_CLIENT_FIELD.name())
.is_none_or(|name| name.name() == stringify!(ExampleClient))
{
span = info_span!(target: "ExampleClient::create_or_update_model", "create_or_update_model", client = "ExampleClient");
span = info_span!(target: "ExampleClient::create_or_update_model", "create_or_update_model", self = self.as_value());
}
async move {
sleep(Duration::from_millis(300)).await;
Expand All @@ -75,6 +79,10 @@ impl ExampleClient {
tracing::error!(name: "create_or_update_model", target: "ExampleClient::create_or_update_model", error = %err)
)
}

pub(crate) fn as_value(&self) -> valuable::Value<'_> {
tracing::field::valuable(self)
}
}

impl fmt::Debug for ExampleClient {
Expand All @@ -84,3 +92,29 @@ impl fmt::Debug for ExampleClient {
.finish()
}
}

impl Valuable for ExampleClient {
fn as_value(&self) -> valuable::Value<'_> {
valuable::Value::Structable(self)
}

fn visit(&self, visit: &mut dyn valuable::Visit) {
visit.visit_named_fields(&NamedValues::new(
FIELDS,
&[
stringify!(ExampleClient).as_value(),
Self::NAMESPACE.as_value(),
],
));
}
}

pub(crate) const AZ_CLIENT_FIELD: NamedField = NamedField::new("az.client");
pub(crate) const AZ_NAMESPACE_FIELD: NamedField = NamedField::new("az.namespace");
static FIELDS: &[NamedField<'static>] = &[AZ_CLIENT_FIELD, AZ_NAMESPACE_FIELD];

impl Structable for ExampleClient {
fn definition(&self) -> valuable::StructDef<'_> {
valuable::StructDef::new_static(stringify!(ExampleClient), Fields::Named(FIELDS))
}
}
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ pub trait ExampleClientExt: private::Sealed {

impl ExampleClientExt for ExampleClient {
#[tracing::instrument(
name = "rotate",
"rotate",
target = "ExampleClient::rotate",
skip_all,
fields(client = "ExampleClient")
fields(self = self.as_value())
)]
async fn rotate<S: Into<Secret>>(&self, name: &str, secret: S) -> Result<Model> {
let mut m = self.get_model(name).await?;
Expand Down

0 comments on commit 844eaf7

Please sign in to comment.