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

Use valuable crate and tracing_unstable to allow population of TransactionContext and SpanContext (guarded behind feature = valuable) #23

Merged
merged 1 commit into from
May 23, 2024

Conversation

merc1031
Copy link

This proposed feature gated code (feature = valuable)has allowed me to enrich the APM requests sent to elastic search
I am using the experimental api from tracing
tokio-rs/tracing#1906
https://tokio.rs/blog/2021-05-valuable
https://docs.rs/tracing/latest/tracing/field/index.html#using-valuable
https://docs.rs/valuable/latest/valuable/

i am using it like so

library (Some code elided)

struct MatchValueVisitor<T> {
    value: Option<T>,
}

pub const SPAN_CONTEXT_DB: &str = "span_context.db";

impl Visit for MatchValueVisitor<model::Db> {
    fn record_debug(&mut self, _field: &Field, _value: &dyn Debug) {}
    fn record_value(&mut self, field: &Field, value: valuable::Value<'_>) {
        if field.name() == SPAN_CONTEXT_DB {
            let mut db_visit = model::Db::default();
            value.visit(&mut db_visit);
            self.value = Some(db_visit);
        }
    }
}

pub const TRANSACTION_CONTEXT_REQUEST: &str = "transaction_context.request";

impl Visit for MatchValueVisitor<model::Request> {
    fn record_debug(&mut self, _field: &Field, _value: &dyn Debug) {}
    fn record_value(&mut self, field: &Field, value: valuable::Value<'_>) {
        if field.name() == TRANSACTION_CONTEXT_REQUEST {
            let mut request_visit = model::Request::default();
            value.visit(&mut request_visit);
            self.value = Some(request_visit);
        }
    }
}

pub struct ApmAugmentLayer {
    inner_layer: ApmLayer,
}

impl ApmAugmentLayer {
    pub fn new(layer: ApmLayer) -> Self {
        ApmAugmentLayer { inner_layer: layer }
    }
}

impl<S> Layer<S> for ApmAugmentLayer
where
    S: Subscriber + for<'lookup> LookupSpan<'lookup>,
{

    fn on_record(&self, span: &Id, values: &Record<'_>, ctx: Context<'_, S>) {
        {
            let spanref = ctx.span(span).expect("Span not found!");
            let mut extensions = spanref.extensions_mut();
            if let Some(transaction_context) = extensions.get_mut::<TransactionContext>() {
		        let mut visitor = MatchValueVisitor::<model::Request> { value: None };
		        can_record.record(&mut visitor);
		        if let Some(request) = visitor.value {
		            transaction_context.request = Some(request);
		        };
		
		        let mut visitor = MatchValueVisitor::<model::Response> { value: None };
		        can_record.record(&mut visitor);
		        if let Some(response) = visitor.value {
		            transaction_context.response = Some(response);
		        };
		  }
	     if let Some(span_context) = extensions.get_mut::<SpanContext>() {
		        let mut visitor = MatchValueVisitor::<model::Destination> { value: None };
		        can_record.record(&mut visitor);
		        if let Some(destination) = visitor.value {
		            span_context.destination = Some(destination);
		        };
		
		        let mut visitor = MatchValueVisitor::<model::SpanService> { value: None };
		        can_record.record(&mut visitor);
		        if let Some(span_service) = visitor.value {
		            span_context.service = span_service;
		        };
          };
        };
        self.inner_layer.on_record(span, values, ctx);
    }

    fn on_close(&self, id: Id, ctx: Context<'_, S>) {
        {
            let span = ctx.span(&id).expect("Span not found!");
            let mut extensions = span.extensions_mut();

            if let Some(mut transaction) = extensions.remove::<Transaction>() {
             
                if let Some(transaction_context) = extensions.remove::<TransactionContext>() {
                    transaction.context = Some(transaction_context)
                }

                extensions.insert(transaction);
            }
            if let Some(mut span) = extensions.remove::<Span>() {
                
                if let Some(span_context) = extensions.remove::<SpanContext>() {
                    span.context = Some(span_context)
                }

                extensions.insert(span);
            }
        };
        self.inner_layer.on_close(id, ctx);
    }

middleware

    let current_span = info_span!(
                "request",
                { TRANSACTION_CONTEXT_REQUEST } = field::Empty,
            );

       let tracing_req = tracing_elastic_apm::model::Request {
            method: req.method().to_string(),
            url: tracing_uri,
            headers: Some(tracing_req_headers),
            ..Default::default()
        };
        current_span.record(TRANSACTION_CONTEXT_REQUEST, tracing_req.as_value());

@krojew
Copy link
Owner

krojew commented May 23, 2024

Looks good, but CI has failed. Can you take a look?

TransactionContext and SpanContext
@krojew
Copy link
Owner

krojew commented May 23, 2024

Thanks!

@krojew krojew merged commit 98dd711 into krojew:master May 23, 2024
1 check passed
@merc1031
Copy link
Author

It did issue a warning for the match statements.

Would you prefer if i added another PR addressing them?

They were of the form

warning: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
   --> src/model.rs:977:9
    |
977 | /         match named_values.get_by_name("target") {
978 | |             Some(valuable::Value::Structable(v)) => {
979 | |                 let mut visit = Target::default();
980 | |                 v.visit(&mut visit);
...   |
983 | |             _ => {}
984 | |         };
    | |_________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_match
help: try
    |
977 ~         if let Some(valuable::Value::Structable(v)) = named_values.get_by_name("target") {
978 +             let mut visit = Target::default();
979 +             v.visit(&mut visit);
980 +             self.target = Some(visit)
981 ~         };

@krojew
Copy link
Owner

krojew commented May 23, 2024

Sure, it would be great to have no warnings.

@merc1031
Copy link
Author

Ok!
ill issue another PR in the morning.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants