Skip to content

Commit 48d356b

Browse files
kapoorabhishek24glitch003CopilotGarandor
authored
Feature: Add request_id in logs (#55)
* first pass at adding request id to logs * Update rust/lit-node/lit-node/src/main.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * change how we set the request id on the span * clippy * Refactor spans behaviour * Fix fallback ID * Add Rocket fairings * Remove redundant tracing * optimise tests * Address feedback * Remove fallback UUID * Leverage Dashmap * build lit-os to rebuild cargo.lock * rebuild lit actions and lit-node * remove correlation_id from /web/job_status/v2 instrumentation since it's only on this one endpoint * Revert "remove correlation_id from /web/job_status/v2 instrumentation since it's only on this one endpoint" This reverts commit 7c4f5f9. --------- Co-authored-by: Chris Cassano <chris@litprotocol.com> Co-authored-by: Chris Cassano <1285652+glitch003@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Adam Reif <adam@litprotocol.com>
1 parent ac869c6 commit 48d356b

File tree

17 files changed

+727
-161
lines changed

17 files changed

+727
-161
lines changed

rust/lit-actions/Cargo.lock

Lines changed: 19 additions & 17 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/lit-core/Cargo.lock

Lines changed: 1 addition & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/lit-core/lit-api-core/src/context/mod.rs

Lines changed: 35 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,15 @@ use std::collections::HashMap;
22
use std::fmt;
33
use std::future::Future;
44

5-
use opentelemetry::propagation::{Extractor, Injector, TextMapPropagator};
6-
use opentelemetry_sdk::propagation::TraceContextPropagator;
5+
use lit_observability::logging::set_request_context;
6+
use opentelemetry::propagation::Injector;
77
use rocket::Request;
88
use rocket::request::{FromRequest, Outcome};
99
use semver::Version;
1010
use serde::{Deserialize, Serialize};
1111
use serde_json::{Map, Value};
1212
use tokio::task::futures::TaskLocalFuture;
1313
use tokio::task_local;
14-
use tracing::{Span, info_span};
15-
use tracing_opentelemetry::OpenTelemetrySpanExt;
16-
use uuid::Uuid;
1714

1815
use crate::error::{EC, Error, Result, conversion_err_code, validation_err_code};
1916

@@ -27,77 +24,6 @@ task_local! {
2724
pub static TRACING: Box<dyn Tracer>;
2825
}
2926

30-
/// The TracingSpan request guard creates a new tracing span for the request. If the request
31-
/// contains a parent span ID, it will be used as the parent of this new span. Otherwise, a new
32-
/// root span will be created.
33-
#[allow(dead_code)]
34-
#[derive(Clone, Debug)]
35-
pub struct TracingSpan {
36-
span: Span,
37-
}
38-
39-
impl TracingSpan {
40-
pub fn new(span: Span) -> Self {
41-
Self { span }
42-
}
43-
44-
pub fn span(&self) -> &Span {
45-
&self.span
46-
}
47-
}
48-
49-
#[rocket::async_trait]
50-
impl<'r> FromRequest<'r> for TracingSpan {
51-
type Error = crate::error::Error;
52-
53-
async fn from_request(
54-
req: &'r rocket::Request<'_>,
55-
) -> rocket::request::Outcome<Self, Self::Error> {
56-
// Extract the propagated context
57-
let propagator = TraceContextPropagator::new();
58-
// Initialize some container to hold the header information.
59-
let mut carrier = HashMap::new();
60-
// Transfer header information from request to carrier.
61-
for header in req.headers().iter() {
62-
carrier.insert(header.name().to_string(), header.value().to_string());
63-
}
64-
// Extract the context from the carrier
65-
let context = propagator.extract(&HeaderExtractor::from(&carrier));
66-
67-
// Initialize a new span with the propagated context as the parent
68-
let req_method = req.method();
69-
let req_path = req.uri().path();
70-
let new_span = info_span!(
71-
"handle_request",
72-
method = req_method.as_str(),
73-
path = req_path.to_string(),
74-
);
75-
new_span.set_parent(context);
76-
77-
Outcome::Success(TracingSpan { span: new_span })
78-
}
79-
}
80-
81-
pub struct HeaderExtractor<'a> {
82-
headers: &'a HashMap<String, String>,
83-
}
84-
85-
impl<'a> From<&'a HashMap<String, String>> for HeaderExtractor<'a> {
86-
fn from(headers: &'a HashMap<String, String>) -> Self {
87-
HeaderExtractor { headers }
88-
}
89-
}
90-
91-
impl<'a> Extractor for HeaderExtractor<'a> {
92-
fn get(&self, key: &str) -> Option<&'a str> {
93-
self.headers.get(key).map(|v| v.as_str())
94-
}
95-
96-
fn keys(&self) -> Vec<&str> {
97-
self.headers.keys().map(|v| v.as_str()).collect()
98-
}
99-
}
100-
10127
pub struct HeaderInjector<'a> {
10228
headers: &'a mut HashMap<String, String>,
10329
}
@@ -167,10 +93,13 @@ impl<'r> FromRequest<'r> for Tracing {
16793
type Error = crate::error::Error;
16894

16995
async fn from_request(req: &'r Request<'_>) -> Outcome<Self, Self::Error> {
170-
let correlation_id =
171-
extract_correlation_id(req).unwrap_or_else(|| format!("LD-{}", Uuid::new_v4()));
96+
let (request_id, correlation_id) = extract_request_and_correlation_ids(req);
97+
98+
// Set request context for log injection; no fallback IDs.
99+
set_request_context(request_id, correlation_id.clone());
172100

173-
let mut tracing = Self::new(correlation_id);
101+
// For the Tracing struct, use empty string if no correlation_id was provided.
102+
let mut tracing = Self::new(correlation_id.unwrap_or_default());
174103
apply_req_tracing_fields(req, &mut tracing);
175104

176105
Outcome::Success(tracing)
@@ -227,7 +156,16 @@ impl<'r> FromRequest<'r> for TracingRequired {
227156
type Error = crate::error::Error;
228157

229158
async fn from_request(req: &'r Request<'_>) -> Outcome<Self, Self::Error> {
230-
if let Some(correlation_id) = extract_correlation_id(req) {
159+
let (request_id, correlation_id) = extract_request_and_correlation_ids(req);
160+
161+
// TracingRequired requires at least one header
162+
if let Some(correlation_id) = correlation_id {
163+
// Preserve distinct values when both headers are present
164+
let request_id = request_id.unwrap_or_else(|| correlation_id.clone());
165+
166+
// Set request context (span extensions + OTel attributes) for consistency
167+
set_request_context(Some(request_id), Some(correlation_id.clone()));
168+
231169
let mut tracing = Self::new(correlation_id);
232170
apply_req_tracing_fields(req, &mut tracing);
233171

@@ -257,12 +195,23 @@ where
257195
TRACING.scope(Box::new(tracing), f)
258196
}
259197

260-
pub(crate) fn extract_correlation_id(req: &Request<'_>) -> Option<String> {
261-
req.headers()
262-
.get(HEADER_KEY_X_CORRELATION_ID)
263-
.next()
264-
.or_else(|| req.headers().get(HEADER_KEY_X_REQUEST_ID).next())
265-
.map(|val| val.to_string())
198+
/// Extracts both request_id and correlation_id from headers, preserving distinct values.
199+
/// Returns (request_id, correlation_id) tuple.
200+
/// - request_id: X-Request-Id header, falls back to X-Correlation-Id
201+
/// - correlation_id: X-Correlation-Id header, falls back to X-Request-Id
202+
pub(crate) fn extract_request_and_correlation_ids(
203+
req: &Request<'_>,
204+
) -> (Option<String>, Option<String>) {
205+
let x_request_id = req.headers().get(HEADER_KEY_X_REQUEST_ID).next().map(|v| v.to_string());
206+
let x_correlation_id =
207+
req.headers().get(HEADER_KEY_X_CORRELATION_ID).next().map(|v| v.to_string());
208+
209+
// request_id: prefer X-Request-Id, fall back to X-Correlation-Id
210+
let request_id = x_request_id.clone().or_else(|| x_correlation_id.clone());
211+
// correlation_id: prefer X-Correlation-Id, fall back to X-Request-Id
212+
let correlation_id = x_correlation_id.or(x_request_id);
213+
214+
(request_id, correlation_id)
266215
}
267216

268217
pub(crate) fn apply_req_tracing_fields(req: &Request<'_>, tracing: &mut (impl Tracer + 'static)) {

rust/lit-core/lit-api-core/src/server/hyper/handler/router.rs

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ use hyper::body::Bytes;
1010
use hyper::http::HeaderValue;
1111
use hyper::{HeaderMap, Method, Request as HyperRequest, Response as HyperResponse};
1212
use tracing::debug;
13-
use uuid::Uuid;
1413

1514
use crate::context::{HEADER_KEY_X_CORRELATION_ID, HEADER_KEY_X_REQUEST_ID, Tracing, with_context};
1615

@@ -129,13 +128,10 @@ impl Default for Router {
129128
}
130129
}
131130

132-
// Get Tracing from request headers.
131+
// Get Tracing from request headers; no fallback ID generation.
133132
fn get_tracing_from_request_header(headers: HeaderMap<HeaderValue>) -> Tracing {
134-
if let Some(correlation_id) = extract_correlation_id(headers) {
135-
Tracing::new(correlation_id)
136-
} else {
137-
Tracing::new(Uuid::new_v4().simple().to_string())
138-
}
133+
let correlation_id = extract_correlation_id(headers).unwrap_or_default();
134+
Tracing::new(correlation_id)
139135
}
140136

141137
fn extract_correlation_id(headers: HeaderMap<HeaderValue>) -> Option<String> {

rust/lit-core/lit-observability/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ proxy-collector = []
1414
channels = ["dep:flume"]
1515

1616
[dependencies]
17+
dashmap = "6"
1718
derive_more.workspace = true
1819
flume = { version = "0.11", optional = true }
1920
hyper-util.workspace = true
2021
nu-ansi-term = { version = "0.50.1" }
2122
opentelemetry.workspace = true
22-
opentelemetry-appender-tracing = { version = "0.5.0", default-features = false }
2323
opentelemetry-otlp = { workspace = true, features = ["logs"] }
2424
opentelemetry-semantic-conventions.workspace = true
2525
opentelemetry_sdk = { workspace = true, features = ["logs"] }

rust/lit-core/lit-observability/src/lib.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,10 @@ use std::str::FromStr;
33
pub use config::LitObservabilityConfig;
44
use error::unexpected_err;
55
use lit_core::config::LitConfig;
6-
use logging::init_logger_provider;
6+
use logging::{ContextAwareOtelLogLayer, CustomEventFormatter, init_logger_provider};
77
use metrics::init_metrics_provider;
88
use net::init_tonic_exporter_builder;
99
use opentelemetry::trace::TracerProvider;
10-
use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge;
1110

1211
use opentelemetry_sdk::logs::LoggerProvider;
1312
use opentelemetry_sdk::metrics::SdkMeterProvider;
@@ -80,8 +79,7 @@ pub async fn create_providers(
8079
};
8180
let logger_provider = init_logger_provider(tonic_exporter_builder, resource.clone())?;
8281

83-
// Create a new OpenTelemetryTracingBridge using the above LoggerProvider.
84-
let tracing_bridge_layer = OpenTelemetryTracingBridge::new(&logger_provider);
82+
let context_aware_log_layer = ContextAwareOtelLogLayer::new(&logger_provider);
8583

8684
// Add a tracing filter to filter events from crates used by opentelemetry-otlp.
8785
// The filter levels are set as follows:
@@ -101,10 +99,12 @@ pub async fn create_providers(
10199
.add_directive("h2=error".parse().unwrap())
102100
.add_directive("reqwest=error".parse().unwrap());
103101

102+
let custom_formatter = CustomEventFormatter::default();
103+
104104
let sub = tracing_subscriber::registry()
105105
.with(level_filter)
106-
.with(fmt::layer())
107-
.with(tracing_bridge_layer)
106+
.with(fmt::layer().event_format(custom_formatter))
107+
.with(context_aware_log_layer)
108108
.with(MetricsLayer::new(meter_provider.clone()))
109109
.with(OpenTelemetryLayer::new(tracer));
110110

0 commit comments

Comments
 (0)