refactor#418
refactor#418alexgallotta wants to merge 2 commits intojordan.gonzalez/bottlecap/add-composite-propagatorfrom
Conversation
| fn inject(&self, context: SpanContext, carrier: &mut dyn Injector); | ||
| } | ||
|
|
||
| impl Propagator for TracePropagationStyle { |
There was a problem hiding this comment.
propagator could be directly attached to the enum, avoiding one abstraction layer
|
|
||
| pub struct DatadogCompositePropagator { | ||
| propagators: Vec<Box<dyn Propagator + 'static>>, | ||
| propagators: Vec<TracePropagationStyle>, |
There was a problem hiding this comment.
without the extra abstraction, we can avoid having dynamic dispatching here
| for propagator in propagator_style { | ||
| if let Some(context) = propagator.extract(carrier) { | ||
| contexts.push(context); | ||
| styles.push(*propagator); |
There was a problem hiding this comment.
also avoiding playing with indexes
There was a problem hiding this comment.
I agree we could make this simpler
| tracestate: tracestate.unwrap_or_default(), | ||
| attributes, | ||
| }); | ||
| } else if style == TracePropagationStyle::TraceContext { |
There was a problem hiding this comment.
this if's can be probably removed and have the enum containing the logic
| fn resolve_contexts( | ||
| contexts: Vec<SpanContext>, | ||
| styles: Vec<TracePropagationStyle>, | ||
| _carrier: &dyn Extractor, |
There was a problem hiding this comment.
no need to carry around the unused extractor
| use lazy_static::lazy_static; | ||
| use regex::Regex; | ||
| use tracing::{debug, error, warn}; | ||
|
|
There was a problem hiding this comment.
I didn't go through all the implementations (a lot of logic! Impressive work!) but maybe some stuff can be deduplicated
| origin: Option<String>, | ||
| lower_order_trace_id: Option<String>, | ||
| } | ||
| fn extract(carrier: &dyn Extractor) -> Option<SpanContext> { |
There was a problem hiding this comment.
Once there is a enum -> extractor relation, it can be also split in different files so logic is more digestible
| pub mod error; | ||
| pub mod text_map_propagator; | ||
|
|
||
| pub trait Propagator { |
There was a problem hiding this comment.
no need of trait propagator anymore
| None, | ||
| } | ||
|
|
||
| impl TracePropagationStyle { |
There was a problem hiding this comment.
Not happy with coupling this into the propagation module which is going to be a crate eventually
2 participants
Some ideas