Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions crates/starknet_transaction_prover/src/server/request_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@ mod request_log_test;
/// HTTP header carrying the request id.
pub const REQUEST_ID_HEADER: &str = "x-request-id";

/// Request extension carrying the id this layer already validated/generated,
/// so a downstream layer (e.g. `RequestSpanLayer`) can reuse it on the
/// plaintext path instead of re-parsing and re-validating the header it just
/// set.
#[derive(Clone)]
pub(crate) struct RequestId(pub String);

/// tower [`Layer`] producing [`RequestLogService`].
#[derive(Clone, Copy, Default)]
pub struct RequestLogLayer;
Expand Down Expand Up @@ -78,6 +85,7 @@ where
let request_id = extract_or_generate_request_id(&request);
let id_header_value = request_id_header_value(&request_id);
request.headers_mut().insert(REQUEST_ID_HEADER, id_header_value.clone());
request.extensions_mut().insert(RequestId(request_id.clone()));
let is_health_probe =
request.method() == Method::GET && request.uri().path() == HEALTH_PATH;
let method = request.method().clone();
Expand Down
16 changes: 11 additions & 5 deletions crates/starknet_transaction_prover/src/server/request_span.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::server::request_log::{
extract_or_generate_request_id,
new_request_id,
request_id_header_value,
RequestId,
REQUEST_ID_HEADER,
};

Expand Down Expand Up @@ -69,11 +70,16 @@ where
request.headers_mut().insert(REQUEST_ID_HEADER, request_id_header_value(&fresh_id));
fresh_id
} else {
// Re-derives, via the shared validator, the exact id
// `RequestLogLayer` already assigned — the header is left
// untouched. This also keeps the layer correct standalone,
// e.g. in unit tests without `RequestLogLayer` upstream.
extract_or_generate_request_id(&request)
// Reuses the id `RequestLogLayer` already validated/generated via
// its request extension, avoiding a second header parse and
// validation pass per request. Falls back to re-deriving it (the
// header is left untouched either way) so the layer stays correct
// standalone, e.g. in unit tests without `RequestLogLayer`
// upstream.
request.extensions().get::<RequestId>().map_or_else(
|| extract_or_generate_request_id(&request),
|request_id| request_id.0.clone(),
)
};
self.inner.call(request).instrument(info_span!("http_request", request_id = %request_id))
}
Expand Down
40 changes: 38 additions & 2 deletions crates/starknet_transaction_prover/src/server/request_span_test.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use bytes::Bytes;
use http::{Method, Request};
use http::{Method, Request, Response, StatusCode};
use http_body_util::Full;
use jsonrpsee::server::HttpBody;
use tower::{Layer, ServiceExt};
use tower_ohttp::Decapsulated;
use tracing_test::traced_test;

use crate::server::middleware_test_utils::{echo_request_id_service, read_body_and_headers};
use crate::server::request_log::{RequestLogLayer, REQUEST_ID_HEADER};
use crate::server::request_log::{RequestId, RequestLogLayer, REQUEST_ID_HEADER};
use crate::server::request_span::RequestSpanLayer;

#[tokio::test]
Expand Down Expand Up @@ -43,6 +44,41 @@ async fn decapsulated_gets_fresh_id_discarding_inbound() {
assert!(uuid::Uuid::parse_str(&id).is_ok(), "must mint a fresh UUID, got {id:?}");
}

/// Proves the fast path actually reads the `RequestId` extension instead of
/// re-parsing the header: the two are set to different values here, which
/// `RequestLogLayer` never lets happen in production, and the span (checked
/// via a log line emitted inside it) must carry the extension's value, not
/// the header's.
#[tokio::test]
#[traced_test]
async fn plaintext_prefers_request_id_extension_over_header() {
let mut request = Request::builder()
.method(Method::POST)
.uri("/")
.header(REQUEST_ID_HEADER, "header-value")
.body(HttpBody::new(Full::new(Bytes::new())))
.expect("static body is infallible");
request.extensions_mut().insert(RequestId("extension-value".to_string()));

let logging_service = tower::service_fn(|_request: Request<HttpBody>| async move {
tracing::info!("handler invoked");
Ok::<_, std::convert::Infallible>(
Response::builder()
.status(StatusCode::OK)
.body(HttpBody::new(Full::new(Bytes::new())))
.expect("static body is infallible"),
)
});

RequestSpanLayer.layer(logging_service).oneshot(request).await.unwrap();

assert!(logs_contain("extension-value"), "span must carry the RequestId extension's value");
assert!(
!logs_contain("header-value"),
"the mismatched header value must not leak into the span"
);
}

/// The cross-layer plaintext contract: with `RequestLogLayer` (outer) stacked
/// over `RequestSpanLayer` (inner) and no inbound id, the id the outer layer
/// generates and echoes on the response must be the same id the inner layer
Expand Down
Loading