fix(observability): switch tracing to JSON formatter#103
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR enables JSON-formatted tracing output and elevates provider non-success response logs from info to warn in Azure and OpenAI providers. ChangesJSON Logging Infrastructure
Provider error logs: switch info -> warn
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
🔒 Container Vulnerability Scan (hub-migrations - amd64)Click to expand results |
Contributor
🔒 Container Vulnerability Scan (hub-migrations - arm64)Click to expand results |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/providers/mod.rs (1)
4-4: ⚡ Quick winKeep
error_loggingcrate-internal.
pub mod error_logging;exposes an internal helper module as public API. Prefer crate-scoped visibility to avoid accidental external coupling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/mod.rs` at line 4, The module error_logging is intended to be crate-internal but is currently exported via `pub mod error_logging;`; change the declaration to `mod error_logging;` so it is not part of the public API, and update any internal references (use/error_logging::* or crate::error_logging) within the crate if necessary to preserve access from sibling modules while preventing external visibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/providers/error_logging.rs`:
- Around line 64-80: The code currently logs the fallback raw_body verbatim when
parsed.is_none(), which can leak sensitive data; change the raw_body value used
in the tracing::error call to a redacted version instead of the raw truncated
string (e.g., call a redaction helper like
redact_oneline(truncate_oneline(&body, RAW_BODY_MAX_BYTES)) or replace with a
fixed placeholder/hash) so that in the block where parsed.is_none() you still
respect RAW_BODY_MAX_BYTES but pass a sanitized value to tracing::error; update
references to raw_body in the tracing::error invocation (and any surrounding
variables) so only the redacted/safe string is emitted.
---
Nitpick comments:
In `@src/providers/mod.rs`:
- Line 4: The module error_logging is intended to be crate-internal but is
currently exported via `pub mod error_logging;`; change the declaration to `mod
error_logging;` so it is not part of the public API, and update any internal
references (use/error_logging::* or crate::error_logging) within the crate if
necessary to preserve access from sibling modules while preventing external
visibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8497ba3-bfd9-472a-9065-a102b045fdb7
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomlsrc/main.rssrc/providers/anthropic/provider.rssrc/providers/azure/provider.rssrc/providers/error_logging.rssrc/providers/mod.rssrc/providers/openai/provider.rs
Contributor
🔒 Container Vulnerability Scan (hub - amd64)Click to expand results |
Contributor
🔒 Container Vulnerability Scan (hub - arm64)Click to expand results |
Datadog's Agent maps stdout uniformly to status:info regardless of the tracing-subscriber level prefix, so tracing::error!() calls have been silently landing on status:info in production. Multi-line tracing messages also fragment across multiple Datadog entries because the Agent splits stdout on '\n'. Switching tracing_subscriber::fmt() to its JSON formatter emits one JSON object per event. Datadog's default pipeline auto-detects JSON, maps the "level" field to status, and lifts every field into a facet. That fixes both symptoms in one place, for every log call site. Scoped intentionally to just the formatter switch so we can verify the Datadog pipeline behaves as expected in staging before touching any application-level error logging. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
doronkopit5
force-pushed
the
dk/informative-errors-on-400
branch
from
May 28, 2026 12:20
55fe46a to
edd9d74
Compare
doronkopit5
changed the title
Contributor
|
Actionable comments posted: 0 |
…ace in prod Production runs at RUST_LOG=WARN, so the OpenAI and Azure chat-completion upstream-error logs emitted via info! were filtered out entirely — which is why the upstream 400 response body never appeared in production. Bump them to warn! so they clear the prod threshold and become visible, with no RUST_LOG / Octopus / cluster changes. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Lockfile-only patch bumps that clear the conservatively-fixable Dependabot security alerts (no Cargo.toml changes): - openssl 0.10.75 -> 0.10.80 (+ openssl-sys 0.9.116): 7 advisories - aws-lc-rs 1.15.3 -> 1.17.0 (aws-lc-sys 0.36.0 -> 0.41.0): 5 advisories - rustls-webpki 0.103.8 -> 0.103.13 (GHSA-82j2-j2ch-gfr8) - time 0.3.45 -> 0.3.47 (CVE-2026-25727) - bytes 1.11.0 -> 1.11.1 (CVE-2026-25541) - quinn-proto 0.11.13 -> 0.11.14 (CVE-2026-31812) - rand 0.8.5 -> 0.8.6, 0.9.2 -> 0.9.3 (GHSA-cq8v-f236-94qc) Verified with cargo fmt --check, cargo build, cargo clippy -- -D warnings. Deferred (require major, non-conservative upgrades): - hickory-proto 0.26.1 (reqwest pins ^0.25; hickory-dns feature not enabled) - rustls-webpki 0.101.7/0.102.8 (AWS SDK + yup-oauth2 major upgrades) - rand 0.7.3 (dev-only, surf/wiremock test stack) Co-Authored-By: Claude Opus 4.8 <[email protected]>
galzilber
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Switch
tracing_subscriber::fmt()to its JSON formatter so each log event becomes one JSON line. Datadog's default pipeline auto-detects JSON, mapslevel→status, and lifts every field into a facet.Two long-standing production gaps share the same root cause: the Datadog Agent maps stdout uniformly to
status:inforegardless of the tracing-subscriber ANSI level prefix, and it splits stdout on\nso multi-line tracing messages fragment. Both disappear with JSON output.Intentionally scoped to just the formatter switch — no provider-level error logging restructured in this PR. Once we verify the JSON pipeline behaves as expected in staging (i.e.
tracing::error!lands onstatus:errorand structured fields show up as facets), follow-up branches can rely on it.Test plan
cargo checkcleancargo test --libclean (no behavioural changes outside the formatter)tracing::error!line (e.g.tower_http::trace::on_failure: response failed ...) now appears in Datadog as JSON, taggedstatus:error, withtarget,level,fields.*available as facetsservice:hublog pipeline before promoting to prod🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores