Skip to content

Commit

Permalink
more tracing logic fixes - traces are being split due to async instru…
Browse files Browse the repository at this point in the history
…mentation logic

Signed-off-by: Paul Scoropan <[email protected]>
  • Loading branch information
pscoro committed Nov 15, 2024
1 parent fae6a85 commit daa6ca6
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 9 deletions.
6 changes: 4 additions & 2 deletions src/clients.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ use futures::Stream;
use ginepro::LoadBalancedChannel;
use hyper_util::rt::TokioExecutor;
use tonic::{metadata::MetadataMap, Request};
use tracing::{debug, instrument};
use tracing::{debug, instrument, Span};
use tracing_opentelemetry::OpenTelemetrySpanExt;
use url::Url;

use crate::{
Expand Down Expand Up @@ -335,7 +336,8 @@ pub fn is_valid_hostname(hostname: &str) -> bool {
/// Turns a gRPC client request body of type `T` and header map into a `tonic::Request<T>`.
/// Will also inject the current `traceparent` header into the request based on the current span.
fn grpc_request_with_headers<T>(request: T, headers: HeaderMap) -> Request<T> {
let headers = with_traceparent_header(headers);
let ctx = Span::current().context();
let headers = with_traceparent_header(&ctx, headers);
let metadata = MetadataMap::from_headers(headers);
Request::from_parts(metadata, Extensions::new(), request)
}
Expand Down
7 changes: 5 additions & 2 deletions src/clients/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use hyper_rustls::HttpsConnector;
use hyper_util::client::legacy::connect::HttpConnector;
use serde::{de::DeserializeOwned, Serialize};
use tracing::{debug, error, instrument, Instrument, Span};
use tracing_opentelemetry::OpenTelemetrySpanExt;
use url::Url;

use super::{Client, Error};
Expand Down Expand Up @@ -130,7 +131,8 @@ impl HttpClient {
headers: HeaderMap,
body: impl Serialize,
) -> Result<Response, Error> {
let headers = trace::with_traceparent_header(headers.to_owned());
let ctx = Span::current().context();
let headers = trace::with_traceparent_header(&ctx, headers.to_owned());
let mut builder = hyper::http::request::Builder::new()
.method(method)
.uri(url.as_uri());
Expand Down Expand Up @@ -160,7 +162,8 @@ impl HttpClient {
.await
.map_err(|e| Error::internal("sending client request failed", e))?
.into();
trace_context_from_http_response(&response);
let span = Span::current();
trace_context_from_http_response(&span, &response);
Ok(response)
}
None => Err(builder.body(body).err().map_or_else(
Expand Down
9 changes: 4 additions & 5 deletions src/utils/trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,11 @@ pub fn on_outgoing_eos(trailers: Option<&HeaderMap>, stream_duration: Duration,
/// vendor-specific trace context.
/// Used by both gRPC and HTTP requests since `tonic::Metadata` uses `http::HeaderMap`.
/// See https://www.w3.org/TR/trace-context/#trace-context-http-headers-format.
pub fn with_traceparent_header(headers: HeaderMap) -> HeaderMap {
pub fn with_traceparent_header(ctx: &opentelemetry::Context, headers: HeaderMap) -> HeaderMap {
let mut headers = headers.clone();
let ctx = Span::current().context();
global::get_text_map_propagator(|propagator| {
// Injects current `traceparent` (and by default empty `tracestate`)
propagator.inject_context(&ctx, &mut HeaderInjector(&mut headers))
propagator.inject_context(ctx, &mut HeaderInjector(&mut headers))
});
headers
}
Expand All @@ -354,12 +353,12 @@ pub fn with_traceparent_header(headers: HeaderMap) -> HeaderMap {
/// tracing span context (i.e. use `traceparent` as parent to the current span).
/// Defaults to using the current context when no `traceparent` is found.
/// See https://www.w3.org/TR/trace-context/#trace-context-http-headers-format.
pub fn trace_context_from_http_response(response: &http::Response) {
pub fn trace_context_from_http_response(span: &Span, response: &http::Response) {
let ctx = global::get_text_map_propagator(|propagator| {
// Returns the current context if no `traceparent` is found
propagator.extract(&HeaderExtractor(response.headers()))
});
Span::current().set_parent(ctx);
span.set_parent(ctx);
}

/// Extracts the `traceparent` header from a gRPC response's metadata and uses it to set the current
Expand Down

0 comments on commit daa6ca6

Please sign in to comment.