fix: make trace_id visible only if in use #359
Conversation
iroh-gateway/src/error.rs
Outdated
| HeaderValue::from_str(&self.trace_id).unwrap(), | ||
| ); | ||
| let has_trace_id = | ||
| !self.trace_id.is_empty() && self.trace_id != "00000000000000000000000000000000"; |
There was a problem hiding this comment.
So why is this a string? We don't want to expose the TraceId type from OpenTelemetry?
It seems that TraceId has a public API to convert it to and from [u8; 16]. Maybe use that for traceid? Then we don't have to do string comparisons here.
There was a problem hiding this comment.
Only reason was not to pull opentelemetry directly into iroh-gateway cause at some point we wanted to have metrics be a compile time toggle. I guess no hard reason right now. And the dep can be bound to the feature anyways.
I'll fix it up before merge.
There was a problem hiding this comment.
Also we still have the dependency anyways so duh...
Nice catch ty.
iroh-gateway/src/error.rs
Outdated
| "message": self.message, | ||
| "trace_id": self.trace_id, | ||
| })); | ||
| let body = match has_trace_id { |
There was a problem hiding this comment.
Why a match instead of a if? Does clippy allow that?
There was a problem hiding this comment.
Now that I look at it it is a bit odd for a boolean flag (was something else before though) and idk I just got used to the match magic :)
But yeah clippy is happy with it. 📎
There was a problem hiding this comment.
It's not wrong, and also not difficult to read, just unusual. So 🤷♂️
No description provided.