feat: better logging for API services #842
Conversation
|
Review changes with Changed Files
|
WalkthroughThe changes introduce a comprehensive observability and tracing infrastructure to the workspace. Dependencies for tracing (tracing, tracing-subscriber, tracing-actix-web) are added at the workspace level. Logging is migrated from env_logger to a tracing-based system with JSON formatting and custom root spans. Workspace members for derive macros and core libraries are also added. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Actix as Actix Web
participant CustomSpan as CustomRootSpanBuilder
participant TracingLogger as TracingLogger
participant ServiceHandler as Service Handler
participant Response as Response
Client->>Actix: HTTP Request
Actix->>CustomSpan: on_request_start(ServiceRequest)
CustomSpan->>CustomSpan: Extract headers<br/>(x-workspace, x-org-id,<br/>user-agent, method, path)
CustomSpan->>CustomSpan: Create Span<br/>via root_span!
CustomSpan-->>TracingLogger: Return Span
TracingLogger->>ServiceHandler: Process Request
ServiceHandler->>ServiceHandler: Record body bytes<br/>& latency
ServiceHandler-->>Response: Generate Response
TracingLogger->>CustomSpan: on_request_end(Span, Outcome)
CustomSpan->>CustomSpan: End Span<br/>(with timing)
Response-->>Client: HTTP Response<br/>(with x-request-id)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Datron
force-pushed
the
better-logging
branch
from
54ca0fc to
8e22eaa
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/service_utils/src/middlewares/request_response_logging.rs (1)
67-110: Gate body buffering behind thelog_body_as_jsonflag.Currently,
body_bytesgrows for all responses regardless oflog_body_as_json. When set to false (non-JSON content types), buffered data is discarded without logging, wasting memory on large HTML, image, or WASM responses.Suggested fix
Poll::Ready(Some(Ok(chunk))) => { - this.body_bytes.extend_from_slice(&chunk); + if this.log_body_as_json { + this.body_bytes.extend_from_slice(&chunk); + } Poll::Ready(Some(Ok(chunk))) }
🤖 Fix all issues with AI agents
In `@crates/context_aware_config/Cargo.toml`:
- Line 12: Update the remaining Cargo.toml declarations to use the workspace
dependency for actix-http to match crates/context_aware_config: change the root
Cargo.toml declaration for actix-http to use { workspace = true } instead of a
fixed version and update the actix-http entry in the experimentation_platform
crate to { workspace = true } as well; leave the superposition and
context_aware_config crate entries unchanged. Ensure the workspace dependency
syntax exactly matches the one already used in crates/context_aware_config
(actix-http = { workspace = true }) so all crates resolve the same workspace
member.
In `@crates/service_utils/src/middlewares/request_response_logging.rs`:
- Around line 82-110: The log message strings are inconsistent with the PR
contract: replace occurrences of "RequestSignal" with "Request" and
"GoldenSignal - No Body" with "GoldenSignal" inside the info! logging calls in
request_response_logging.rs (the branches handling response JSON parse
success/failure and the non-JSON/no-body branch where this.headers and
this.body_bytes are used); also add a boolean field (e.g., body_present =
!this.body_bytes.is_empty()) to the structured log entries so callers can
distinguish presence of a body without changing the top-level message; apply the
same changes to the other logging block referenced (the block around the 217-236
region) to keep labels consistent.
- Around line 175-179: The code currently logs raw request headers/body into the
HashMap `headers` (built from `req.headers()`), which can leak secrets;
implement a header allowlist and redaction utility (e.g.,
`redact_headers(headers: &HeaderMap) -> HashMap<String,String>`) that preserves
safe headers and replaces sensitive values (Authorization, Cookie, Set-Cookie,
X-Api-Key, etc.) with a constant like "[REDACTED]"; also implement
`redact_body(body: &str) -> String` that truncates long bodies to a configured
MAX_BODY_LOG_LEN and removes or masks obvious secrets, then use these utilities
wherever headers/bodies are collected (the `headers` variable building shown,
and the other header/body collection sites referenced around lines 210-235 and
258-263) and apply the same logic to response headers and response body before
logging (e.g., `response_headers`, response body extraction points).
- Around line 187-248: Avoid unconditionally buffering the request payload:
before reading payload into body_bytes (the req.into_parts()/payload loop),
check headers for Content-Length and Content-Type and only buffer when
Content-Length exists and is below a safe threshold (e.g., LOG_BODY_MAX_BYTES)
and Content-Type indicates JSON; otherwise skip reading the stream and leave
payload untouched so streaming endpoints aren’t blocked. If you do buffer,
continue to parse into request_body_value and construct new_payload as you
already do; if you skip buffering, set new_payload to the original payload (do
not consume it) and log minimal metadata only. Update code paths that reference
payload, body_bytes, request_body_value and new_payload accordingly so non-JSON
or large bodies are passed through unchanged.
- Around line 190-196: The loop that reads the request payload (using
payload.next()) currently logs an error and breaks on Err(e), allowing a
truncated body_bytes to be forwarded; instead, propagate the error to reject the
request immediately. Replace the break branch so the function returns an Err
constructed from the payload error (e.g., convert e into the middleware's error
type or into actix_web::Error) rather than continuing, ensuring the
reconstructed payload code path (the body_bytes -> reconstructed payload) is not
executed with partial data.
In `@crates/superposition/Cargo.toml`:
- Around line 40-44: The build breaks because main.rs calls
fmt::layer().flatten_current_span_on_top_level(true) but json-subscriber =
"0.2.7" does not expose that API; update the crate usage to match available APIs
or change the dependency: either remove the
flatten_current_span_on_top_level(true) call in main.rs and use supported
methods on fmt::layer() (e.g., flatten_event(), with_current_span(),
with_span_list(), with_target()), replace json-subscriber with a crate that
supports top-level span flattening (for example tracing_flat_json) and adapt
initialization accordingly, or change the dependency version in Cargo.toml for
json-subscriber to one that provides flatten_current_span_on_top_level; edit
Cargo.toml and main.rs consistently (look for the json-subscriber entry and the
fmt::layer() call) so the selected crate and code match.
In `@crates/superposition/src/log_span.rs`:
- Around line 15-26: The extracted header values (via the extractor_header
closure and variables workspace, org, user_agent) must be sanitized and
length-capped before being stored in span fields to avoid control chars and huge
values; update extractor_header (or post-process its result) to remove/replace
non-printable/control characters (e.g., allow only printable ASCII/Unicode
categories or replace with '?') and truncate the resulting String to a safe max
length (e.g., 256 chars) before calling unwrap_or_else so the span only receives
sanitized, bounded values; ensure the same sanitization+truncate logic is
applied to all header keys (x-workspace, x-org-id, user-agent) and keep
HeaderMap/HeaderValue usage unchanged except for applying this cleaning step.
In `@crates/superposition/src/main.rs`:
- Around line 292-294: RequestResponseLogger currently logs all request/response
headers and full bodies; update its logging path to (1) redact sensitive headers
(at minimum: "authorization", "cookie", "set-cookie" and any "bearer" token
values) by replacing their values with a fixed placeholder before logging, (2)
truncate logged request and response bodies to a configurable max length
(suggest default 4KB) and append an indicator like "...(truncated)" when cut,
and (3) add optional sampling for high-volume endpoints (make sampling
configurable and skip body logging or reduce frequency for sampled routes).
Implement these changes inside the RequestResponseLogger middleware (and any
helper functions it uses) so all header extraction/printing uses a
redact_headers(...) helper and body logging uses truncate_body(...)/sampling
checks before writing to the logger.
🧹 Nitpick comments (1)
crates/service_utils/src/middlewares/request_response_logging.rs (1)
250-251: Latency timer starts after the request body is fully read.If latency is meant to reflect client‑perceived time, start the timer before draining the request body; otherwise, rename the field (e.g.,
processing_latency_ms) or add a separatebody_read_msmetric.
|
|
||
| [dependencies] | ||
| actix-http = "3.3.1" | ||
| actix-http = { workspace = true } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List all actix-http dependency declarations to verify consistency.
rg -n --glob '**/Cargo.toml' 'actix-http\s*='Repository: juspay/superposition
Length of output: 310
Update root and experimentation_platform Cargo.toml to use workspace dependency for actix-http.
The change to crates/context_aware_config/Cargo.toml uses { workspace = true }, but Cargo.toml (root) and crates/experimentation_platform/Cargo.toml still pin actix-http = "3.3.1". Align all declarations:
- Root
Cargo.toml:76should define the workspace dependency crates/experimentation_platform/Cargo.toml:12should use{ workspace = true }- Keep
crates/superposition/Cargo.toml:40andcrates/context_aware_config/Cargo.toml:12as is (already correct)
🤖 Prompt for AI Agents
In `@crates/context_aware_config/Cargo.toml` at line 12, Update the remaining
Cargo.toml declarations to use the workspace dependency for actix-http to match
crates/context_aware_config: change the root Cargo.toml declaration for
actix-http to use { workspace = true } instead of a fixed version and update the
actix-http entry in the experimentation_platform crate to { workspace = true }
as well; leave the superposition and context_aware_config crate entries
unchanged. Ensure the workspace dependency syntax exactly matches the one
already used in crates/context_aware_config (actix-http = { workspace = true })
so all crates resolve the same workspace member.
| info!( | ||
| headers = ?this.headers, | ||
| body = %json_value, | ||
| latency = latency_ms, | ||
| "GoldenSignal" | ||
| ); | ||
| } | ||
| Err(_) => { | ||
| // JSON parsing failed, log as string fallback | ||
| let response_body = if this.body_bytes.is_empty() { | ||
| String::from("(empty)") | ||
| } else { | ||
| String::from_utf8_lossy(&this.body_bytes).into_owned() | ||
| }; | ||
| info!( | ||
| headers = ?this.headers, | ||
| body = %format!("(invalid JSON: {})", response_body), | ||
| latency = latency_ms, | ||
| "GoldenSignal" | ||
| ); | ||
| } | ||
| } | ||
| } else { | ||
| String::from_utf8(this.body_bytes.clone()).unwrap_or_else(|_| { | ||
| format!("(binary data, {} bytes)", this.body_bytes.len()) | ||
| }) | ||
| }; | ||
|
|
||
| trace!( | ||
| "RESPONSE: {} headers=[{}] body={}", | ||
| this.status, | ||
| this.headers.join(", "), | ||
| response_body | ||
| ); | ||
| // Non-JSON response, skip body entirely | ||
| info!( | ||
| headers = ?this.headers, | ||
| latency = latency_ms, | ||
| "GoldenSignal - No Body" | ||
| ); |
There was a problem hiding this comment.
Log labels don’t match the PR contract.
The PR description says request logs should be labeled "Request" and response logs "GoldenSignal", but the code emits "RequestSignal" and "GoldenSignal - No Body". This can break downstream indexing/queries. Align the message strings (and add a separate field like body_present if needed).
Also applies to: 217-236
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 82 - 110, The log message strings are inconsistent with the PR contract:
replace occurrences of "RequestSignal" with "Request" and "GoldenSignal - No
Body" with "GoldenSignal" inside the info! logging calls in
request_response_logging.rs (the branches handling response JSON parse
success/failure and the non-JSON/no-body branch where this.headers and
this.body_bytes are used); also add a boolean field (e.g., body_present =
!this.body_bytes.is_empty()) to the structured log entries so callers can
distinguish presence of a body without changing the top-level message; apply the
same changes to the other logging block referenced (the block around the 217-236
region) to keep labels consistent.
| let mut headers = HashMap::new(); | ||
| for (name, value) in req.headers().iter() { | ||
| if let Ok(value_str) = value.to_str() { | ||
| headers.push(format!("{}: {}", name, value_str)); | ||
| headers.insert(name.as_str().to_string(), value_str.to_string()); | ||
| } |
There was a problem hiding this comment.
Redact sensitive headers/body before logging.
Raw headers and bodies can leak secrets/PII (Authorization, Cookie, Set‑Cookie, API keys, tokens). Add allowlist/redaction and body truncation, and apply the same logic to response headers/bodies.
🔒 Example header redaction (apply similarly to response headers)
- for (name, value) in req.headers().iter() {
- if let Ok(value_str) = value.to_str() {
- headers.insert(name.as_str().to_string(), value_str.to_string());
- }
- }
+ for (name, value) in req.headers().iter() {
+ let key = name.as_str();
+ let key_lc = key.to_ascii_lowercase();
+ let is_sensitive = matches!(
+ key_lc.as_str(),
+ "authorization" | "cookie" | "set-cookie" | "x-api-key" | "proxy-authorization"
+ );
+ if let Ok(value_str) = value.to_str() {
+ headers.insert(
+ key.to_string(),
+ if is_sensitive { "[redacted]".to_string() } else { value_str.to_string() },
+ );
+ }
+ }Also applies to: 210-235, 258-263
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 175 - 179, The code currently logs raw request headers/body into the
HashMap `headers` (built from `req.headers()`), which can leak secrets;
implement a header allowlist and redaction utility (e.g.,
`redact_headers(headers: &HeaderMap) -> HashMap<String,String>`) that preserves
safe headers and replaces sensitive values (Authorization, Cookie, Set-Cookie,
X-Api-Key, etc.) with a constant like "[REDACTED]"; also implement
`redact_body(body: &str) -> String` that truncates long bodies to a configured
MAX_BODY_LOG_LEN and removes or masks obvious secrets, then use these utilities
wherever headers/bodies are collected (the `headers` variable building shown,
and the other header/body collection sites referenced around lines 210-235 and
258-263) and apply the same logic to response headers and response body before
logging (e.g., `response_headers`, response body extraction points).
| let (http_req, mut payload) = req.into_parts(); | ||
| let mut body_bytes = Vec::new(); | ||
|
|
||
| while let Some(chunk) = payload.next().await { | ||
| match chunk { | ||
| Ok(bytes) => body_bytes.extend_from_slice(&bytes), | ||
| Err(e) => { | ||
| warn!("Error reading request body: {}", e); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Convert body to string for logging | ||
| let request_body = if body_bytes.is_empty() { | ||
| String::from("(empty)") | ||
| } else { | ||
| String::from_utf8(body_bytes.clone()).unwrap_or_else(|_| { | ||
| format!("(binary data, {} bytes)", body_bytes.len()) | ||
| }) | ||
| }; | ||
| // Try to parse as JSON and log as structured data | ||
| let request_body_value = if !body_bytes.is_empty() { | ||
| serde_json::from_slice::<Value>(&body_bytes).ok() | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| trace!( | ||
| "REQUEST: {} {} query={} headers=[{}] body={}", | ||
| method, | ||
| uri, | ||
| if query_string.is_empty() { | ||
| "(none)" | ||
| if let Some(ref json) = request_body_value { | ||
| // Successfully parsed as JSON - log as structured | ||
| info!( | ||
| query = %if query_string.is_empty() { | ||
| "(none)".to_string() | ||
| } else { | ||
| &query_string | ||
| query_string.clone() | ||
| }, | ||
| headers.join(", "), | ||
| request_body | ||
| headers = ?headers, | ||
| body = ?json, | ||
| "RequestSignal" | ||
| ); | ||
|
|
||
| // Reconstruct the request with the body | ||
| #[rustfmt::skip] | ||
| let new_payload = if body_bytes.is_empty() { | ||
| Payload::None | ||
| } else { | ||
| // Not JSON - log as string | ||
| let request_body = if body_bytes.is_empty() { | ||
| String::from("(empty)") | ||
| } else { | ||
| let bytes = Bytes::from(body_bytes); | ||
| let stream = once(async move { Ok::<Bytes, PayloadError>(bytes) }); | ||
| Payload::from(Box::pin(stream) as Pin< Box< dyn futures_util::Stream< Item = Result<Bytes, PayloadError> > > >) | ||
| String::from_utf8_lossy(&body_bytes).into_owned() | ||
| }; | ||
| let new_req = ServiceRequest::from_parts(http_req, new_payload); | ||
|
|
||
| // Call the next service | ||
| res = service.call(new_req).await?; | ||
| } else { | ||
| // For GET/DELETE etc, don't extract body | ||
| trace!( | ||
| "REQUEST: {} {} query={} headers=[{}]", | ||
| method, | ||
| uri, | ||
| if query_string.is_empty() { | ||
| "(none)" | ||
| info!( | ||
| query = %if query_string.is_empty() { | ||
| "(none)".to_string() | ||
| } else { | ||
| &query_string | ||
| query_string.clone() | ||
| }, | ||
| headers.join(", ") | ||
| headers = ?headers, | ||
| body = %request_body, | ||
| "RequestSignal" | ||
| ); | ||
|
|
||
| // Call the next service | ||
| res = service.call(req).await?; | ||
| } | ||
|
|
||
| // Log response details | ||
| let status = res.status(); | ||
| let mut response_headers = Vec::new(); | ||
| let new_payload = if body_bytes.is_empty() { | ||
| Payload::None | ||
| } else { | ||
| let bytes = Bytes::from(body_bytes); | ||
| let stream = once(async move { Ok::<Bytes, PayloadError>(bytes) }); | ||
| Payload::from(Box::pin(stream) | ||
| as Pin< | ||
| Box<dyn futures_util::Stream<Item = Result<Bytes, PayloadError>>>, | ||
| >) | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 132
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | head -250Repository: juspay/superposition
Length of output: 10354
🏁 Script executed:
# Look for any guards before line 187
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | sed -n '1,200p'Repository: juspay/superposition
Length of output: 8107
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | sed -n '249,276p'Repository: juspay/superposition
Length of output: 1309
Skip request body buffering unless Content-Length is below a safe threshold and content type is JSON.
The middleware unconditionally buffers the entire request payload (lines 190–198) before passing it to the service, which blocks streaming endpoints and creates OOM/DoS vulnerability on large uploads. No size limits or content-type guards exist. Instead, follow the pattern used for response logging (line 267): add a Content-Length check and JSON content-type condition, and pass the original payload through untouched when these aren't met.
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 187 - 248, Avoid unconditionally buffering the request payload: before
reading payload into body_bytes (the req.into_parts()/payload loop), check
headers for Content-Length and Content-Type and only buffer when Content-Length
exists and is below a safe threshold (e.g., LOG_BODY_MAX_BYTES) and Content-Type
indicates JSON; otherwise skip reading the stream and leave payload untouched so
streaming endpoints aren’t blocked. If you do buffer, continue to parse into
request_body_value and construct new_payload as you already do; if you skip
buffering, set new_payload to the original payload (do not consume it) and log
minimal metadata only. Update code paths that reference payload, body_bytes,
request_body_value and new_payload accordingly so non-JSON or large bodies are
passed through unchanged.
| while let Some(chunk) = payload.next().await { | ||
| match chunk { | ||
| Ok(bytes) => body_bytes.extend_from_slice(&bytes), | ||
| Err(e) => { | ||
| warn!("Error reading request body: {}", e); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | sed -n '180,210p'Repository: juspay/superposition
Length of output: 1406
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | sed -n '160,195p'Repository: juspay/superposition
Length of output: 1630
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | head -30Repository: juspay/superposition
Length of output: 1103
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | sed -n '200,250p'Repository: juspay/superposition
Length of output: 2380
Return an error instead of breaking when payload reading fails.
When payload.next() returns an error, the code logs and breaks, which allows the truncated body_bytes to be forwarded to downstream handlers via the reconstructed payload (lines 242–248). This corrupts the request. Return the error to reject the request immediately.
🐛 Suggested fix
Err(e) => {
warn!("Error reading request body: {}", e);
- break;
+ return Err(Error::from(e));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| while let Some(chunk) = payload.next().await { | |
| match chunk { | |
| Ok(bytes) => body_bytes.extend_from_slice(&bytes), | |
| Err(e) => { | |
| warn!("Error reading request body: {}", e); | |
| break; | |
| } | |
| while let Some(chunk) = payload.next().await { | |
| match chunk { | |
| Ok(bytes) => body_bytes.extend_from_slice(&bytes), | |
| Err(e) => { | |
| warn!("Error reading request body: {}", e); | |
| return Err(Error::from(e)); | |
| } |
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 190 - 196, The loop that reads the request payload (using payload.next())
currently logs an error and breaks on Err(e), allowing a truncated body_bytes to
be forwarded; instead, propagate the error to reject the request immediately.
Replace the break branch so the function returns an Err constructed from the
payload error (e.g., convert e into the middleware's error type or into
actix_web::Error) rather than continuing, ensuring the reconstructed payload
code path (the body_bytes -> reconstructed payload) is not executed with partial
data.
| actix-http = { workspace = true } | ||
| tracing = { workspace = true } | ||
| tracing-subscriber = { workspace = true } | ||
| tracing-actix-web = { workspace = true } | ||
| json-subscriber = "0.2.7" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "Cargo.toml" -type f | head -10Repository: juspay/superposition
Length of output: 401
🏁 Script executed:
cat crates/superposition/Cargo.toml | head -50Repository: juspay/superposition
Length of output: 1670
🏁 Script executed:
cat crates/superposition/src/main.rs | head -100Repository: juspay/superposition
Length of output: 3555
🏁 Script executed:
cat ./Cargo.toml | grep -A 50 "\[workspace.dependencies\]"Repository: juspay/superposition
Length of output: 2048
🌐 Web query:
json-subscriber 0.2.7 crate tracing-subscriber 0.3.22 compatibility
💡 Result:
json-subscriber 0.2.7 should be compatible with tracing-subscriber 0.3.22 as long as json-subscriber depends on tracing-subscriber with a normal semver requirement in the 0.3.x line (i.e., ^0.3.*).
For example, json-subscriber 0.2.6 declares tracing-subscriber = "0.3.18" (which Cargo interprets as ^0.3.18, allowing upgrades up to <0.4.0), so Cargo can resolve it to 0.3.22 without issue. [1] tracing-subscriber 0.3.22 is a patch/minor update within the same 0.3 series. [2]
Practical note: If your app directly depends on tracing-subscriber = "0.3.22", Cargo should unify the graph to a single tracing-subscriber version (0.3.22), avoiding “two versions of the same crate” type-mismatch problems for Layer/Subscriber traits.
One caveat: I couldn’t find docs/source for json-subscriber 0.2.7 on docs.rs in this lookup, so I verified the dependency behavior using 0.2.6’s Cargo.toml (the key compatibility point is the ^0.3.x requirement). [1]
Sources: [1], [2]
🌐 Web query:
json-subscriber crate 0.2.7 flatten_current_span_on_top_level API documentation
💡 Result:
json-subscriber v0.2.7 API docs for flatten_current_span_on_top_level are not available on docs.rs because the 0.2.7 docs build failed there. (docs.rs)
In the last successful docs build (0.2.7-alpha.1), the corresponding API is on JsonLayer and is named:
JsonLayer::with_top_level_flattened_current_span(&mut self) -> &mut Self(docs.rs)- “Print all current span fields, each as its own top level member of the JSON.”
- Warning: you must ensure span field names don’t clash with other top-level JSON keys, or you may generate invalid JSON with duplicate keys; the docs recommend preferring
with_current_spaninstead. (docs.rs)
Example:
let mut layer = json_subscriber::JsonLayer::stdout();
layer.with_top_level_flattened_current_span();Citations:
- 1: https://docs.rs/json-subscriber
- 2: https://docs.rs/json-subscriber/0.2.7-alpha.1/json_subscriber/struct.JsonLayer.html
- 3: https://docs.rs/json-subscriber/0.2.7-alpha.1/json_subscriber/struct.JsonLayer.html
🌐 Web query:
json-subscriber fmt layer flatten_current_span_on_top_level method 0.2
💡 Result:
In json-subscriber 0.2.x, the method you’re looking for is named:
JsonLayer::with_top_level_flattened_current_span(&mut self) -> &mut Self— it “prints all current span fields, each as its own top level member of the JSON”, with a warning that key clashes can produce invalid JSON. Preferwith_current_span(...)if you don’t need top-level flattening. [1]
Example:
use tracing_subscriber::prelude::*;
let mut json = json_subscriber::JsonLayer::stdout();
json.with_top_level_flattened_current_span();
tracing_subscriber::registry()
.with(json)
.init();Related options on JsonLayer:
with_top_level_flattened_span_list()(flattens parent span fields onto the top level) [1]with_flattened_event()(flattens event fields onto the top level) [1]
If you specifically want “event + spans flattened onto top-level”, tracing-flat-json notes it’s similar to combining json-subscriber’s with_flattened_event and with_top_level_flattened_span_list. [2]
Sources
[1] json_subscriber::JsonLayer docs (methods incl. with_top_level_flattened_current_span) https://docs.rs/json-subscriber/0.2.6/json_subscriber/struct.JsonLayer.html
[2] tracing-flat-json crate docs (comparison to json-subscriber options) https://docs.rs/tracing-flat-json/latest/tracing_flat_json/
🌐 Web query:
json-subscriber 0.2.7 fmt module layer builder
💡 Result:
In json-subscriber (the fmt compatibility module), you have two “builder-style” entry points:
- Subscriber builder:
json_subscriber::fmt()returns afmt::SubscriberBuilder(liketracing_subscriber::fmt().json()), which you can configure and then.init(),.try_init(), or.finish(). [1][2] - Layer builder:
json_subscriber::fmt::layer::<S>()returns a JSONfmt::Layer<S>you can compose onto atracing_subscriber::registry()with.with(...). [3][4]
Example: fmt() (subscriber builder)
json_subscriber::fmt()
.with_max_level(tracing::Level::TRACE)
.with_current_span(false)
.init();json_subscriber outputs JSON by default; calling .json() is unnecessary (kept only for migration and deprecated/no-op). [1][2]
Example: fmt::layer() (layer builder)
use tracing_subscriber::prelude::*;
let json_layer = json_subscriber::fmt::layer()
.with_current_span(false)
.with_span_list(false);
tracing_subscriber::registry()
.with(json_layer)
.init();fmt::layer() exists mainly for tracing-subscriber compatibility; for lower-level/more control you can use JsonLayer directly. [3][4]
Note: I couldn’t confirm a docs.rs page specifically for version 0.2.7 from search results; the API above is from the crate docs and latest module docs. [1][4]
Sources:
[1] docs.rs crate overview (json_subscriber)
[2] json_subscriber::fmt::fmt() docs
[3] json_subscriber::fmt::layer() docs
[4] json_subscriber::fmt module docs
🌐 Web query:
json-subscriber fmt Layer flatten_current_span_on_top_level flatten_event with_target builder methods
💡 Result:
In json-subscriber’s fmt compatibility API, the builder-style methods you’re looking for are:
flatten_event(bool)— flattens the event fields into the top-level JSON object (equivalent totracing_subscriber::fmt().json().flatten_event(true)). This does not flatten span fields. [1][2]with_current_span(bool)— include/omit the"span"object. [1][2]with_span_list(bool)— include/omit the"spans"array. [1][2]with_target(bool)— include/omit the"target"field. [1]
Example:
json_subscriber::fmt()
.flatten_event(true)
.with_current_span(true)
.with_span_list(true)
.with_target(false)
.init();flatten_current_span_on_top_level / flatten_current_span_on_top_level / “flatten span(s) onto top-level” are not part of tracing-subscriber’s JSON formatter knobs, and they don’t appear in json-subscriber’s fmt::SubscriberBuilder API either—only event flattening is supported there. [1][2]
If you specifically want span fields flattened onto the top-level, consider a dedicated layer like tracing_flat_json, which emits JSON with event + span fields flattened at the top level. [3]
json-subscriber 0.2.7 is incompatible with the code in main.rs.
The fmt::layer() builder does not support flatten_current_span_on_top_level(). The available methods are flatten_event(), with_current_span(), with_span_list(), and with_target() only. The code will fail to compile. Either remove the flatten_current_span_on_top_level(true) call, use an alternative crate like tracing_flat_json that supports flattening spans to the top level, or pin to a different version of json-subscriber that provides this API.
🤖 Prompt for AI Agents
In `@crates/superposition/Cargo.toml` around lines 40 - 44, The build breaks
because main.rs calls fmt::layer().flatten_current_span_on_top_level(true) but
json-subscriber = "0.2.7" does not expose that API; update the crate usage to
match available APIs or change the dependency: either remove the
flatten_current_span_on_top_level(true) call in main.rs and use supported
methods on fmt::layer() (e.g., flatten_event(), with_current_span(),
with_span_list(), with_target()), replace json-subscriber with a crate that
supports top-level span flattening (for example tracing_flat_json) and adapt
initialization accordingly, or change the dependency version in Cargo.toml for
json-subscriber to one that provides flatten_current_span_on_top_level; edit
Cargo.toml and main.rs consistently (look for the json-subscriber entry and the
fmt::layer() call) so the selected crate and code match.
| let extractor_header = |headers: &HeaderMap, key: &str| { | ||
| headers | ||
| .get(key) | ||
| .and_then(|v| HeaderValue::to_str(v).ok()) | ||
| .map(String::from) | ||
| }; | ||
| let workspace = extractor_header(headers, "x-workspace") | ||
| .unwrap_or_else(|| "no-workspace-header".to_string()); | ||
| let org = extractor_header(headers, "x-org-id") | ||
| .unwrap_or_else(|| "no-org-header".to_string()); | ||
| let user_agent = extractor_header(headers, "user-agent") | ||
| .unwrap_or_else(|| "no-user-agent".to_string()); |
There was a problem hiding this comment.
Sanitize and cap header values before logging.
Raw header values can contain control characters or be very large, which risks log injection, downstream parsing errors, and ingestion blowups. Please sanitize and truncate before writing them into span fields.
🔒 Proposed fix (sanitize + truncate)
- let extractor_header = |headers: &HeaderMap, key: &str| {
- headers
- .get(key)
- .and_then(|v| HeaderValue::to_str(v).ok())
- .map(String::from)
- };
+ let sanitize = |value: &str| {
+ value
+ .chars()
+ .filter(|c| !c.is_control())
+ .take(256)
+ .collect::<String>()
+ };
+ let extractor_header = |headers: &HeaderMap, key: &str| {
+ headers
+ .get(key)
+ .and_then(|v| HeaderValue::to_str(v).ok())
+ .map(sanitize)
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let extractor_header = |headers: &HeaderMap, key: &str| { | |
| headers | |
| .get(key) | |
| .and_then(|v| HeaderValue::to_str(v).ok()) | |
| .map(String::from) | |
| }; | |
| let workspace = extractor_header(headers, "x-workspace") | |
| .unwrap_or_else(|| "no-workspace-header".to_string()); | |
| let org = extractor_header(headers, "x-org-id") | |
| .unwrap_or_else(|| "no-org-header".to_string()); | |
| let user_agent = extractor_header(headers, "user-agent") | |
| .unwrap_or_else(|| "no-user-agent".to_string()); | |
| let sanitize = |value: &str| { | |
| value | |
| .chars() | |
| .filter(|c| !c.is_control()) | |
| .take(256) | |
| .collect::<String>() | |
| }; | |
| let extractor_header = |headers: &HeaderMap, key: &str| { | |
| headers | |
| .get(key) | |
| .and_then(|v| HeaderValue::to_str(v).ok()) | |
| .map(sanitize) | |
| }; | |
| let workspace = extractor_header(headers, "x-workspace") | |
| .unwrap_or_else(|| "no-workspace-header".to_string()); | |
| let org = extractor_header(headers, "x-org-id") | |
| .unwrap_or_else(|| "no-org-header".to_string()); | |
| let user_agent = extractor_header(headers, "user-agent") | |
| .unwrap_or_else(|| "no-user-agent".to_string()); |
🤖 Prompt for AI Agents
In `@crates/superposition/src/log_span.rs` around lines 15 - 26, The extracted
header values (via the extractor_header closure and variables workspace, org,
user_agent) must be sanitized and length-capped before being stored in span
fields to avoid control chars and huge values; update extractor_header (or
post-process its result) to remove/replace non-printable/control characters
(e.g., allow only printable ASCII/Unicode categories or replace with '?') and
truncate the resulting String to a safe max length (e.g., 256 chars) before
calling unwrap_or_else so the span only receives sanitized, bounded values;
ensure the same sanitization+truncate logic is applied to all header keys
(x-workspace, x-org-id, user-agent) and keep HeaderMap/HeaderValue usage
unchanged except for applying this cleaning step.
| .wrap(RequestResponseLogger) | ||
| .wrap(TracingLogger::<CustomRootSpanBuilder>::new()) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the context around lines 292-294 in main.rs
cat -n crates/superposition/src/main.rs | sed -n '280,305p'Repository: juspay/superposition
Length of output: 1116
🏁 Script executed:
# Find the RequestResponseLogger implementation
fd -t f RequestResponseLogger -x cat {} 2>/dev/null || rg -l "RequestResponseLogger" --type rustRepository: juspay/superposition
Length of output: 46
🏁 Script executed:
# Search for the actual implementation of RequestResponseLogger
rg -n "struct RequestResponseLogger|impl.*RequestResponseLogger" --type rust -A 20Repository: juspay/superposition
Length of output: 6417
🏁 Script executed:
# Look for redaction, truncation, or sensitive header handling
rg -n "redact|truncate|Authorization|Cookie|Set-Cookie|sensitive|max_size|size_limit" --type rust crates/superposition/src/Repository: juspay/superposition
Length of output: 126
🏁 Script executed:
# Get file size first
wc -l crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 132
🏁 Script executed:
# Read the entire request_response_logging.rs file
cat -n crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 11464
Implement sensitive header redaction and response body size limits in RequestResponseLogger.
The middleware logs all request and response headers unconditionally (lines 176–179, 258–263) without filtering sensitive ones (Authorization, Cookie, Set-Cookie, Bearer tokens, etc.), and logs full request/response bodies without truncation (lines 200–237, 78–111). This exposes credentials and PII. Add header redaction for sensitive fields, truncate bodies to a reasonable size limit (e.g., 4KB), and consider sampling for high-volume endpoints.
🤖 Prompt for AI Agents
In `@crates/superposition/src/main.rs` around lines 292 - 294,
RequestResponseLogger currently logs all request/response headers and full
bodies; update its logging path to (1) redact sensitive headers (at minimum:
"authorization", "cookie", "set-cookie" and any "bearer" token values) by
replacing their values with a fixed placeholder before logging, (2) truncate
logged request and response bodies to a configurable max length (suggest default
4KB) and append an indicator like "...(truncated)" when cut, and (3) add
optional sampling for high-volume endpoints (make sampling configurable and skip
body logging or reduce frequency for sampled routes). Implement these changes
inside the RequestResponseLogger middleware (and any helper functions it uses)
so all header extraction/printing uses a redact_headers(...) helper and body
logging uses truncate_body(...)/sampling checks before writing to the logger.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@Cargo.toml`:
- Around line 76-79: The actix-http dependency in Cargo.toml is pinned to
"3.3.1" but actix-web 4.5.0 requires actix-http ^3.6; update the actix-http
constraint to a compatible version (e.g., change actix-http = "3.3.1" to
actix-http = "3.6.0" or actix-http = "^3.6") so the Cargo manifest accurately
expresses the requirement for actix-web 4.5.0 (refer to the actix-http entry and
the actix-web dependency declaration when making the change).
In `@crates/service_utils/src/middlewares/request_response_logging.rs`:
- Around line 104-110: The logs in request_response_logging.rs use inconsistent
labels and still emit entries for non-JSON responses; update the log label
values to exactly "Request" and "GoldenSignal" (replace occurrences like
"RequestSignal" and "GoldenSignal - No Body") and modify the non-JSON branch in
the middleware (the info! call inside the response handling block and the
similar block referenced around lines 217-235) to either skip emitting a
GoldenSignal log entirely for ignored/non-JSON bodies or emit a GoldenSignal
with an explicit ignored_body=true flag (add a boolean field like ignored_body
to the structured log). Ensure you update every info! invocation in this module
that reports request/response labels so they follow the PR contract ("Request"
for request logs, "GoldenSignal" for signal logs) and include the ignored_body
flag where applicable.
- Around line 175-179: The header-logging loop in request_response_logging.rs is
inserting raw header values from req.headers() (e.g., Authorization, Cookie,
Set-Cookie, API-Key) which can leak secrets; update the logic used in the loop
(or add a helper like redact_header) to replace sensitive header values with a
constant placeholder (e.g., "<REDACTED>") or only include an allowlist of safe
headers before calling headers.insert; apply the same redaction/allowlist change
to the corresponding response-header logging block referenced around lines
258-263 so both request and response header maps never contain raw secret
values.
- Around line 23-29: LoggingBody currently buffers the entire response body
(fields: inner, body_bytes, consumed, log_body_as_json), which can cause
unbounded memory use for large/streaming responses; add a capture limit and
truncation flag (e.g., max_capture_bytes: usize and truncated: bool) to the
LoggingBody struct and update the response-reading logic (the code around the
response buffering/consumption you see in the impl at lines ~67-103) to stop
appending once body_bytes.len() >= max_capture_bytes, set truncated=true, and
avoid further buffering while still streaming the remaining data from inner;
finally, include the truncated flag in the logged output so logs reflect that
the body was capped.
- Around line 188-205: The request read loop silently breaks on PayloadError and
buffers unlimited bytes; change it to propagate the error instead of breaking
(return a suitable error result when encountering PayloadError) and introduce a
MAX_BUFFER_SIZE constant (e.g. MAX_BODY_BYTES) to cap request buffering by
checking body_bytes.len() + chunk.len() and stop/mark truncated when exceeded;
ensure the reconstructed payload is only created when no read error occurred and
include a flag or truncated marker if the cap was hit. Apply the same
MAX_BUFFER_SIZE limit and truncation tracking in LoggingBody::poll_next when
collecting response chunks, and ensure both places return/propagate an error on
actual read failures (PayloadError) rather than silently continuing; reference
the variables/functions body_bytes, payload, request_body_value, PayloadError,
and the LoggingBody::poll_next method to locate changes.
In `@crates/superposition/src/main.rs`:
- Around line 291-294: The comment claiming middleware is "conditionally" added
is stale because RequestResponseLogger and
TracingLogger::<CustomRootSpanBuilder> are always wrapped; fix this by wrapping
those middleware in a Condition (or update the comment). Specifically, replace
the unconditional
.wrap(RequestResponseLogger).wrap(TracingLogger::<CustomRootSpanBuilder>::new())
with conditional wrappers using middleware::Condition (e.g.,
Condition::new(<dev-flag>, RequestResponseLogger) and Condition::new(<dev-flag>,
TracingLogger::<CustomRootSpanBuilder>::new())), and add the necessary import
for Condition; alternatively, simply change the comment to reflect that the
logging middleware is added unconditionally.
- Around line 66-77: The tracing subscriber setup currently misses a LogTracer
bridge and a fallback filter, so log crate events (log::info!, log::error!,
etc.) are dropped and EnvFilter::from_default_env() defaults to ERROR when
RUST_LOG is unset; fix by initializing tracing_log::LogTracer::init() before
building the tracing_subscriber and change the EnvFilter construction to use a
fallback (e.g., EnvFilter::from_default_env().add_directive(Level::INFO.into())
or EnvFilter::try_from_default_env().unwrap_or_else(|_|
"info".parse().unwrap())) so info-level logs are retained, and ensure the
tracing-log (or tracing_log) crate is added to Cargo.toml; keep using the
existing fmt::layer() configuration.
| actix-http = "3.3.1" | ||
| tracing = "0.1.44" | ||
| tracing-subscriber = { version = "0.3.22", features = ["env-filter", "fmt", "json"] } | ||
| tracing-actix-web = "0.7.21" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check actix-web 4.5.0 dependencies to verify actix-http compatibility
echo "=== actix-web 4.5.0 dependencies ==="
curl -s "https://crates.io/api/v1/crates/actix-web/4.5.0" | jq '.crate | {name, version}'
curl -s "https://crates.io/api/v1/crates/actix-web/4.5.0/dependencies" | jq '.dependencies[] | select(.crate_id == "actix-http") | {crate_id, req, kind}'
echo ""
echo "=== tracing-actix-web 0.7.21 dependencies ==="
curl -s "https://crates.io/api/v1/crates/tracing-actix-web/0.7.21/dependencies" | jq '.dependencies[] | {crate_id, req, kind}' | head -50
echo ""
echo "=== Checking tracing ecosystem compatibility ==="
curl -s "https://crates.io/api/v1/crates/tracing-subscriber/0.3.22/dependencies" | jq '.dependencies[] | select(.crate_id == "tracing") | {crate_id, req, kind}'Repository: juspay/superposition
Length of output: 1141
🏁 Script executed:
#!/bin/bash
echo "=== uuid 1.20.0 release notes and features ==="
curl -s "https://crates.io/api/v1/crates/uuid/1.20.0" | jq '{name: .crate.name, version: .crate.version, created_at: .crate.created_at}'
echo ""
echo "=== Checking if uuid 1.20.0 has 'js' feature ==="
curl -s "https://crates.io/api/v1/crates/uuid/1.20.0/features" | jq 'keys' 2>/dev/null || echo "Feature endpoint not available"
# Try to get the full crate data which includes features
curl -s "https://crates.io/api/v1/crates/uuid/1.20.0" | jq '.crate | {name, version, description}'
echo ""
echo "=== Checking uuid changelog between 1.3.4 and 1.20.0 ==="
# Check if there's a significant version jump
echo "Version jump: 1.3.4 → 1.20.0 is a major version increase (1.x jump)"Repository: juspay/superposition
Length of output: 422
🏁 Script executed:
#!/bin/bash
# Let's check if there's a Cargo.toml in the repo we can examine
echo "=== Checking Cargo.toml for context ==="
if [ -f Cargo.toml ]; then
echo "Lines 70-85 of Cargo.toml:"
sed -n '70,85p' Cargo.toml
else
echo "Cargo.toml not found at root"
fi
echo ""
echo "=== Checking Cargo.lock if it exists ==="
if [ -f Cargo.lock ]; then
echo "Dependencies mentioned exist in lock file"
grep -A 2 "name = \"actix-http\"" Cargo.lock | head -3
grep -A 2 "name = \"tracing\"" Cargo.lock | head -3
grep -A 2 "name = \"tracing-actix-web\"" Cargo.lock | head -3
else
echo "No Cargo.lock found"
fiRepository: juspay/superposition
Length of output: 1261
Fix actix-http version constraint to satisfy actix-web 4.5.0 requirements.
The actix-http dependency is pinned to 3.3.1, which does not satisfy the version requirement ^3.6 imposed by actix-web 4.5.0. While Cargo resolves this to 3.6.0 at lock time, the Cargo.toml constraint is incorrect. Update to actix-http = "3.6.0" or use actix-http = "^3.6" to properly express the version constraint.
The remaining tracing dependencies (tracing 0.1.44, tracing-subscriber 0.3.22, tracing-actix-web 0.7.21) are compatible and appropriately chosen for the JSON logging infrastructure. The uuid upgrade to 1.20.0 with the "js" feature is compatible and within the same major version.
🤖 Prompt for AI Agents
In `@Cargo.toml` around lines 76 - 79, The actix-http dependency in Cargo.toml is
pinned to "3.3.1" but actix-web 4.5.0 requires actix-http ^3.6; update the
actix-http constraint to a compatible version (e.g., change actix-http = "3.3.1"
to actix-http = "3.6.0" or actix-http = "^3.6") so the Cargo manifest accurately
expresses the requirement for actix-web 4.5.0 (refer to the actix-http entry and
the actix-web dependency declaration when making the change).
| pub struct LoggingBody<B> { | ||
| inner: B, | ||
| status: StatusCode, | ||
| headers: Vec<String>, | ||
| headers: HashMap<String, String>, | ||
| body_bytes: Vec<u8>, | ||
| consumed: bool, | ||
| start_time: Instant, | ||
| log_body_as_json: bool, |
There was a problem hiding this comment.
Cap response body buffering to avoid unbounded memory use
LoggingBody buffers the full response body before logging. Large or streaming responses can spike memory and latency. Consider capping captured bytes and flagging truncation.
🔧 Suggested cap + truncation flag
+const MAX_LOG_BYTES: usize = 64 * 1024;
pub struct LoggingBody<B> {
inner: B,
headers: HashMap<String, String>,
body_bytes: Vec<u8>,
consumed: bool,
+ body_truncated: bool,
start_time: Instant,
log_body_as_json: bool,
}
impl<B> LoggingBody<B> {
fn new(
body: B,
headers: HashMap<String, String>,
start_time: Instant,
log_body_as_json: bool,
) -> Self {
Self {
inner: body,
headers,
body_bytes: Vec::new(),
consumed: false,
+ body_truncated: false,
start_time,
log_body_as_json,
}
}
}
...
match inner.poll_next(cx) {
Poll::Ready(Some(Ok(chunk))) => {
- this.body_bytes.extend_from_slice(&chunk);
+ if this.body_bytes.len() < MAX_LOG_BYTES {
+ let remaining = MAX_LOG_BYTES - this.body_bytes.len();
+ let to_copy = remaining.min(chunk.len());
+ this.body_bytes.extend_from_slice(&chunk[..to_copy]);
+ if to_copy < chunk.len() {
+ this.body_truncated = true;
+ }
+ } else {
+ this.body_truncated = true;
+ }
Poll::Ready(Some(Ok(chunk)))
}
...
Ok(json_value) => {
info!(
headers = ?this.headers,
body = %json_value,
+ body_truncated = this.body_truncated,
latency = latency_ms,
"GoldenSignal"
);
}Also applies to: 67-103
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 23 - 29, LoggingBody currently buffers the entire response body (fields:
inner, body_bytes, consumed, log_body_as_json), which can cause unbounded memory
use for large/streaming responses; add a capture limit and truncation flag
(e.g., max_capture_bytes: usize and truncated: bool) to the LoggingBody struct
and update the response-reading logic (the code around the response
buffering/consumption you see in the impl at lines ~67-103) to stop appending
once body_bytes.len() >= max_capture_bytes, set truncated=true, and avoid
further buffering while still streaming the remaining data from inner; finally,
include the truncated flag in the logged output so logs reflect that the body
was capped.
| } else { | ||
| String::from_utf8(this.body_bytes.clone()).unwrap_or_else(|_| { | ||
| format!("(binary data, {} bytes)", this.body_bytes.len()) | ||
| }) | ||
| }; | ||
|
|
||
| trace!( | ||
| "RESPONSE: {} headers=[{}] body={}", | ||
| this.status, | ||
| this.headers.join(", "), | ||
| response_body | ||
| ); | ||
| // Non-JSON response, skip body entirely | ||
| info!( | ||
| headers = ?this.headers, | ||
| latency = latency_ms, | ||
| "GoldenSignal - No Body" | ||
| ); |
There was a problem hiding this comment.
Align log message labels with the PR contract ("Request"/"GoldenSignal")
The PR objectives specify Request and GoldenSignal, but the code emits RequestSignal and GoldenSignal - No Body, and non‑JSON responses still emit a log entry. This can break downstream indexing. Prefer consistent labels and add a flag for ignored bodies (or skip logging entirely for non‑JSON).
✅ Suggested label alignment
- "RequestSignal"
+ "Request"
);
} else {
...
- info!(
+ info!(
query = %if query_string.is_empty() {
"(none)".to_string()
} else {
query_string.clone()
},
headers = ?headers,
body = %request_body,
- "RequestSignal"
+ "Request"
);
}
...
- info!(
- headers = ?this.headers,
- latency = latency_ms,
- "GoldenSignal - No Body"
- );
+ info!(
+ headers = ?this.headers,
+ latency = latency_ms,
+ body_ignored = true,
+ "GoldenSignal"
+ );Also applies to: 217-235
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 104 - 110, The logs in request_response_logging.rs use inconsistent labels
and still emit entries for non-JSON responses; update the log label values to
exactly "Request" and "GoldenSignal" (replace occurrences like "RequestSignal"
and "GoldenSignal - No Body") and modify the non-JSON branch in the middleware
(the info! call inside the response handling block and the similar block
referenced around lines 217-235) to either skip emitting a GoldenSignal log
entirely for ignored/non-JSON bodies or emit a GoldenSignal with an explicit
ignored_body=true flag (add a boolean field like ignored_body to the structured
log). Ensure you update every info! invocation in this module that reports
request/response labels so they follow the PR contract ("Request" for request
logs, "GoldenSignal" for signal logs) and include the ignored_body flag where
applicable.
| let mut headers = HashMap::new(); | ||
| for (name, value) in req.headers().iter() { | ||
| if let Ok(value_str) = value.to_str() { | ||
| headers.push(format!("{}: {}", name, value_str)); | ||
| headers.insert(name.as_str().to_string(), value_str.to_string()); | ||
| } |
There was a problem hiding this comment.
Redact sensitive headers before logging
Logging raw Authorization, Cookie, Set-Cookie, or API key headers can leak secrets/PII. Consider redaction or an allowlist before inserting into the log map.
🔐 Example redaction helper
+fn redact_header(name: &str, value: &str) -> String {
+ match name.to_ascii_lowercase().as_str() {
+ "authorization" | "cookie" | "set-cookie" | "x-api-key" => "[REDACTED]".to_string(),
+ _ => value.to_string(),
+ }
+}
...
for (name, value) in req.headers().iter() {
if let Ok(value_str) = value.to_str() {
- headers.insert(name.as_str().to_string(), value_str.to_string());
+ let name_str = name.as_str();
+ headers.insert(name_str.to_string(), redact_header(name_str, value_str));
}
}
...
for (name, value) in res.headers().iter() {
if let Ok(value_str) = value.to_str() {
- response_headers
- .insert(name.as_str().to_string(), value_str.to_string());
+ let name_str = name.as_str();
+ response_headers.insert(
+ name_str.to_string(),
+ redact_header(name_str, value_str),
+ );
}
}Also applies to: 258-263
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 175 - 179, The header-logging loop in request_response_logging.rs is
inserting raw header values from req.headers() (e.g., Authorization, Cookie,
Set-Cookie, API-Key) which can leak secrets; update the logic used in the loop
(or add a helper like redact_header) to replace sensitive header values with a
constant placeholder (e.g., "<REDACTED>") or only include an allowlist of safe
headers before calling headers.insert; apply the same redaction/allowlist change
to the corresponding response-header logging block referenced around lines
258-263 so both request and response header maps never contain raw secret
values.
| let mut body_bytes = Vec::new(); | ||
|
|
||
| while let Some(chunk) = payload.next().await { | ||
| match chunk { | ||
| Ok(bytes) => body_bytes.extend_from_slice(&bytes), | ||
| Err(e) => { | ||
| warn!("Error reading request body: {}", e); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Convert body to string for logging | ||
| let request_body = if body_bytes.is_empty() { | ||
| String::from("(empty)") | ||
| } else { | ||
| String::from_utf8(body_bytes.clone()).unwrap_or_else(|_| { | ||
| format!("(binary data, {} bytes)", body_bytes.len()) | ||
| }) | ||
| }; | ||
| // Try to parse as JSON and log as structured data | ||
| let request_body_value = if !body_bytes.is_empty() { | ||
| serde_json::from_slice::<Value>(&body_bytes).ok() | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 132
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | head -250Repository: juspay/superposition
Length of output: 10354
🏁 Script executed:
cat -n crates/service_utils/src/middlewares/request_response_logging.rs | tail -50Repository: juspay/superposition
Length of output: 2287
🏁 Script executed:
rg "MAX_LOG_BYTES" crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 46
🏁 Script executed:
rg "MAX_LOG_BYTES|max.*bytes|buffer.*limit|size.*cap" crates/service_utils/src/Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
rg "MAX_LOG_BYTES|MAX.*BYTES|const.*\d+.*=.*\d+" crates/service_utils/src/middlewares/Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
rg "response.*cap|body.*limit|size.*cap" crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 46
🏁 Script executed:
git log --oneline -20 crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 124
🏁 Script executed:
rg "truncate|cap|max.*size|buffer" crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 46
🏁 Script executed:
fd -e rs . crates/service_utils/src/ | xargs rg "const MAX_LOG_BYTES|const.*LOG.*BYTES"Repository: juspay/superposition
Length of output: 48
🏁 Script executed:
git show 8e22eaad:crates/service_utils/src/middlewares/request_response_logging.rs | head -280Repository: juspay/superposition
Length of output: 9532
🏁 Script executed:
fd -e rs . crates/service_utils/ | xargs rg "const|static.*=" | grep -i "size\|buffer\|limit\|max\|bytes"Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
find . -name "*.rs" -o -name "*.toml" | xargs rg "MAX.*BYTES|BUFFER.*SIZE|LOG.*SIZE" 2>/dev/null | head -20Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
rg "struct.*Config|Config.*{" crates/service_utils/src/ | head -20Repository: juspay/superposition
Length of output: 175
🏁 Script executed:
find crates/ -name "*.rs" | xargs rg "8192|16384|32768|65536" | grep -i "size\|buffer\|max\|limit" | head -10Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
rg "struct Config" crates/service_utils/src/Repository: juspay/superposition
Length of output: 46
🏁 Script executed:
rg "extend_from_slice|body_bytes" crates/service_utils/src/middlewares/request_response_logging.rsRepository: juspay/superposition
Length of output: 929
🏁 Script executed:
python3 << 'EOF'
# Check if response body has any size limiting mechanism
import re
with open('crates/service_utils/src/middlewares/request_response_logging.rs', 'r') as f:
content = f.read()
# Find LoggingBody poll_next implementation
poll_next_section = content[content.find('fn poll_next'):content.find('fn poll_next') + 1500]
print("Response body handling in poll_next:")
print(poll_next_section)
print("\n" + "="*50)
# Check if request body has any limiting
request_loop = content[content.find('while let Some(chunk) = payload.next()'):content.find('while let Some(chunk) = payload.next()') + 400]
print("\nRequest body loop:")
print(request_loop)
EOFRepository: juspay/superposition
Length of output: 150
Propagate payload read errors and cap request body size
The error handling at line 193-196 has a critical flaw: when PayloadError occurs, the loop breaks but the partial body_bytes collected up to that point are still reconstructed into a new Payload (lines 239-248) and passed downstream (line 249). This silently masks read failures, allowing the downstream service to process a truncated request body without awareness of the error.
Additionally, request body buffering is unlimited, creating OOM risk for large or streaming payloads. Response body buffering also lacks limits (line 69 in LoggingBody::poll_next).
To fix:
- Return an error on
PayloadErrorinstead of breaking silently - Define a reasonable size cap constant and apply it to both request and response body collection, tracking truncation status if needed
🤖 Prompt for AI Agents
In `@crates/service_utils/src/middlewares/request_response_logging.rs` around
lines 188 - 205, The request read loop silently breaks on PayloadError and
buffers unlimited bytes; change it to propagate the error instead of breaking
(return a suitable error result when encountering PayloadError) and introduce a
MAX_BUFFER_SIZE constant (e.g. MAX_BODY_BYTES) to cap request buffering by
checking body_bytes.len() + chunk.len() and stop/mark truncated when exceeded;
ensure the reconstructed payload is only created when no read error occurred and
include a flag or truncated marker if the cap was hit. Apply the same
MAX_BUFFER_SIZE limit and truncation tracking in LoggingBody::poll_next when
collecting response chunks, and ensure both places return/propagate an error on
actual read failures (PayloadError) rather than silently continuing; reference
the variables/functions body_bytes, payload, request_body_value, PayloadError,
and the LoggingBody::poll_next method to locate changes.
| // Initialize tracing subscriber with custom JSON formatter | ||
| tracing_subscriber::registry() | ||
| .with(EnvFilter::from_default_env()) | ||
| .with( | ||
| fmt::layer() | ||
| .with_current_span(true) | ||
| .flatten_current_span_on_top_level(true) | ||
| .flatten_event(true) | ||
| .with_span_list(false) | ||
| .with_target(false), | ||
| ) | ||
| .init(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and read the relevant lines
cat -n crates/superposition/src/main.rs | head -100Repository: juspay/superposition
Length of output: 4255
🏁 Script executed:
# Look for log crate usage in the codebase
rg "log::" --type rust | head -20Repository: juspay/superposition
Length of output: 2216
🏁 Script executed:
# Check Cargo.toml dependencies for logging-related crates
fd "Cargo.toml" | xargs rg "tracing|log|env_logger" -A 2Repository: juspay/superposition
Length of output: 3817
🏁 Script executed:
# Verify EnvFilter behavior in tracing-subscriber documentation
web_search
tracing-subscriber EnvFilter from_default_env default log levelRepository: juspay/superposition
Length of output: 171
🏁 Script executed:
# Search for tracing-log in the codebase
rg "tracing[_-]log" --type tomlRepository: juspay/superposition
Length of output: 122
🏁 Script executed:
# Check if there are any other logging initialization patterns
rg "LogTracer|tracing_log" --type rustRepository: juspay/superposition
Length of output: 46
🌐 Web query:
tracing-subscriber EnvFilter from_default_env default level RUST_LOG unset
💡 Result:
tracing_subscriber::filter::EnvFilter::from_default_env() reads the RUST_LOG env var (EnvFilter::DEFAULT_ENV). If RUST_LOG is unset, empty, or contains only invalid directives, it falls back to a default directive that enables ERROR level (i.e., you’ll only see error! events by default). [1]
If you want a different fallback when RUST_LOG is unset/empty, use the builder, e.g. set INFO as the default directive: [2]
use tracing_subscriber::filter::{EnvFilter, LevelFilter};
let filter = EnvFilter::builder()
.with_default_directive(LevelFilter::INFO.into())
.from_env_lossy();Sources:
[1] EnvFilter::from_default_env docs (fallback to ERROR when RUST_LOG unset/empty/invalid)
[2] EnvFilter Builder::with_default_directive docs
🏁 Script executed:
# Check if tracing-log is explicitly in workspace dependencies
cat Cargo.toml | rg -A 50 "\[workspace.dependencies\]"Repository: juspay/superposition
Length of output: 2048
🏁 Script executed:
# Verify the actual dependency tree for tracing-log
rg "name = \"tracing-log\"" -A 5 Cargo.lock | head -20Repository: juspay/superposition
Length of output: 270
Add LogTracer bridge to capture log crate events and set a default filter level
The codebase extensively uses the log crate (log::error!, log::info!), but without initializing LogTracer, these events will be dropped since the tracing subscriber won't receive them. Additionally, EnvFilter::from_default_env() defaults to ERROR level when RUST_LOG is unset, which will suppress info-level logs from your new tracing setup. Initialize LogTracer and provide a fallback filter to ensure all events are captured.
🔧 Suggested fix
+use tracing_log::LogTracer;
use tracing_subscriber::{EnvFilter, layer::SubscriberExt, util::SubscriberInitExt};
...
- tracing_subscriber::registry()
- .with(EnvFilter::from_default_env())
+ let env_filter = EnvFilter::try_from_default_env()
+ .unwrap_or_else(|_| EnvFilter::new("info"));
+ LogTracer::init().expect("failed to init LogTracer");
+ tracing_subscriber::registry()
+ .with(env_filter)
.with(
fmt::layer()
.with_current_span(true)
.flatten_current_span_on_top_level(true)
.flatten_event(true)
.with_span_list(false)
.with_target(false),
)
.init();Also add tracing-log to crates/superposition/Cargo.toml dependencies if not already present.
🤖 Prompt for AI Agents
In `@crates/superposition/src/main.rs` around lines 66 - 77, The tracing
subscriber setup currently misses a LogTracer bridge and a fallback filter, so
log crate events (log::info!, log::error!, etc.) are dropped and
EnvFilter::from_default_env() defaults to ERROR when RUST_LOG is unset; fix by
initializing tracing_log::LogTracer::init() before building the
tracing_subscriber and change the EnvFilter construction to use a fallback
(e.g., EnvFilter::from_default_env().add_directive(Level::INFO.into()) or
EnvFilter::try_from_default_env().unwrap_or_else(|_| "info".parse().unwrap()))
so info-level logs are retained, and ensure the tracing-log (or tracing_log)
crate is added to Cargo.toml; keep using the existing fmt::layer()
configuration.
| // Conditionally add request/response logging middleware for development | ||
| .wrap(Condition::new(log_enabled!(Level::Trace), RequestResponseLogger)) | ||
| .wrap(Logger::default()) | ||
| .wrap(RequestResponseLogger) | ||
| .wrap(TracingLogger::<CustomRootSpanBuilder>::new()) | ||
| }) |
There was a problem hiding this comment.
Update the stale “conditionally add” comment
The middleware is added unconditionally now; the comment is misleading. Either update the comment or wrap the logging middleware in a Condition.
🤖 Prompt for AI Agents
In `@crates/superposition/src/main.rs` around lines 291 - 294, The comment
claiming middleware is "conditionally" added is stale because
RequestResponseLogger and TracingLogger::<CustomRootSpanBuilder> are always
wrapped; fix this by wrapping those middleware in a Condition (or update the
comment). Specifically, replace the unconditional
.wrap(RequestResponseLogger).wrap(TracingLogger::<CustomRootSpanBuilder>::new())
with conditional wrappers using middleware::Condition (e.g.,
Condition::new(<dev-flag>, RequestResponseLogger) and Condition::new(<dev-flag>,
TracingLogger::<CustomRootSpanBuilder>::new())), and add the necessary import
for Condition; alternatively, simply change the comment to reflect that the
logging middleware is added unconditionally.
| .get(header::CONTENT_TYPE) | ||
| .and_then(|ct| ct.to_str().ok()) | ||
| .map(|ct| { | ||
| ct.starts_with("application/json") |
There was a problem hiding this comment.
Might want to use the mime crate here for the content-type strings list.
There was a problem hiding this comment.
Mime crate would not be needed
Actix has constants for such values
ContentType::APPLICATION_JSON or something similar is there
We are using it already in code
| // Conditionally add request/response logging middleware for development | ||
| .wrap(Condition::new(log_enabled!(Level::Trace), RequestResponseLogger)) | ||
| .wrap(Logger::default()) | ||
| .wrap(RequestResponseLogger) |
There was a problem hiding this comment.
we cant log frontend data, local server just dies responding to the requests
Problem
logging for our saas version is not adequate
Solution
Provider json logs with proper format for indexing in elastic/opensearch/other indexes. some points:
GoldenSignaland and request logs are labelled asRequestbody, `latencySummary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.