Make tracing dependencies optional #374
Conversation
Codecov ReportBase: 52.47% // Head: 52.49% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #374 +/- ##
==========================================
+ Coverage 52.47% 52.49% +0.02%
==========================================
Files 53 53
Lines 5795 5781 -14
==========================================
- Hits 3041 3035 -6
+ Misses 2754 2746 -8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| macro_rules! trace { | ||
| ($($t:tt)+) => {{ | ||
| #[cfg(feature = "tracing")] | ||
| ::tracing::trace!($($t)*); |
There was a problem hiding this comment.
Since tracing has the tracing_log to convert log entry to tracing event. How about always using log::debug in our code?
This change will also makes other libs / apps to integrate with isahc (because nearly all logging libs supports/compatible to log). And of course, makes our code more understandable.
There was a problem hiding this comment.
So the problem with that is that tracing_log can only expose the limited data that the log crate provides for each event into a trace event. If we exclusively use the log crate, then we cannot provide the following information to tracing users:
- Structured log fields
- Spans that group log events by the request they are associated with
Both are pretty valuable, and the reason why Isahc switched to tracing in the first place. But the Promised Land of tracing integrating with log in a seamless, backward-compatible way seems to have failed somewhat. If we use log then tracing users won't be happy, and if we use tracing then log users won't be happy.
Honestly if we can't use the structured features of tracing, then we might as well not bother supporting tracing at all.
So the idea here is to throw together an abstraction layer between the two that allows us to continue exposing structured data as well as spans, which fall back to no-ops when using log.
| @@ -419,15 +432,18 @@ impl AgentContext { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| #[tracing::instrument(level = "trace", skip(self))] | |||
There was a problem hiding this comment.
I guess we can use #[cfg_attr(feature="tracing", tracing::instrument)] instead.
Fixes #293.