feat(observability): add X-Trace-Id response header#50
Conversation
- add TraceIdResponseHeaderFilter and wire it into SecurityFilterChain - include fallback to MDC traceId - add unit tests for tracer/mdc/no-trace scenarios - document header usage in API, README troubleshooting, and changelog
There was a problem hiding this comment.
Pull request overview
Adds an HTTP response correlation header (X-Trace-Id) to improve client ↔ server diagnostics by exposing the active trace id (from Micrometer Tracer.currentSpan() with MDC fallback), and documents this behavior.
Changes:
- Introduces
TraceIdResponseHeaderFilterto setX-Trace-Idon responses, with MDC fallback. - Registers the new filter in
SecurityConfigbeforeAuthenticationFilter. - Adds unit tests and updates API/README/CHANGELOG documentation to mention
X-Trace-Id.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/lt/satsyuk/security/TraceIdResponseHeaderFilter.java | New filter that resolves trace id and sets X-Trace-Id (including a post-chain “late” attempt). |
| src/test/java/lt/satsyuk/security/TraceIdResponseHeaderFilterTest.java | Unit tests for span-based trace id, MDC fallback, and missing trace id behavior. |
| src/main/java/lt/satsyuk/config/SecurityConfig.java | Wires the new filter into the Spring Security filter chain. |
| README.md | Adds guidance for correlating errors with logs/traces via X-Trace-Id. |
| API.md | Documents the X-Trace-Id response header and includes it in a 429 example. |
| CHANGELOG.md | Notes addition of X-Trace-Id for correlation (including error responses). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| filterChain.doFilter(request, response); | ||
|
|
||
| if (!response.containsHeader(TRACE_ID_HEADER)) { | ||
| String lateTraceId = resolveTraceId(); | ||
| if (StringUtils.hasText(lateTraceId)) { | ||
| response.setHeader(TRACE_ID_HEADER, lateTraceId); | ||
| } | ||
| } |
There was a problem hiding this comment.
doFilterInternal tries to set the header after filterChain.doFilter(...), but by that point the response may already be committed (e.g., error handlers writing/flushing the body). In that case, setHeader will be ignored by the servlet container and the “lateTraceId” path won’t actually add the header. Consider guarding the post-chain header write with !response.isCommitted() or using an on-commit response wrapper/header-writer mechanism so the header is reliably written before commit.
There was a problem hiding this comment.
Fixed in 3bdca67. Added a commit guard for the late write path: !response.isCommitted() before attempting to set X-Trace-Id after filterChain.doFilter(...).
| if (!response.containsHeader(TRACE_ID_HEADER)) { | ||
| String lateTraceId = resolveTraceId(); | ||
| if (StringUtils.hasText(lateTraceId)) { | ||
| response.setHeader(TRACE_ID_HEADER, lateTraceId); | ||
| } | ||
| } |
There was a problem hiding this comment.
The post-chain “lateTraceId” branch is new behavior but isn’t covered by tests. Add a unit test where resolveTraceId() returns null before doFilter, then returns a non-empty trace id after the chain runs (e.g., via a custom FilterChain that updates the tracer/MDC) to verify the header gets added in this scenario.
There was a problem hiding this comment.
Added in 3bdca67. New unit test addsHeaderWhenTraceIdAppearsAfterFilterChain covers the post-chain scenario where trace id appears during the chain and then gets written to X-Trace-Id.
| If request fails, check response header `X-Trace-Id` and use it to find correlated logs/traces. | ||
|
|
||
| --- | ||
|
|
||
| ### 🔎 Correlating errors with logs/traces | ||
| The API returns `X-Trace-Id` response header for request correlation. | ||
|
|
||
| Use this header value when investigating `401`, `403`, and `429` responses in Grafana/Tempo/Loki. |
There was a problem hiding this comment.
README states the API “returns X-Trace-Id response header”, but the implementation only sets it when a trace id is available (span/MDC present). To avoid overpromising, consider aligning wording with API.md (e.g., “may include” / “when available”).
| If request fails, check response header `X-Trace-Id` and use it to find correlated logs/traces. | |
| --- | |
| ### 🔎 Correlating errors with logs/traces | |
| The API returns `X-Trace-Id` response header for request correlation. | |
| Use this header value when investigating `401`, `403`, and `429` responses in Grafana/Tempo/Loki. | |
| If request fails and the response includes `X-Trace-Id`, use it to find correlated logs/traces. | |
| --- | |
| ### 🔎 Correlating errors with logs/traces | |
| The API may include an `X-Trace-Id` response header for request correlation when a trace id is available. | |
| Use this header value, when present, when investigating `401`, `403`, and `429` responses in Grafana/Tempo/Loki. |
There was a problem hiding this comment.
Updated in 3bdca67. README wording is now conditional: X-Trace-Id is documented as present when included/available, matching the implementation behavior.
- guard late header write with !response.isCommitted() - add unit test for post-chain trace id availability - align README wording with optional header presence
|
What\n- add TraceIdResponseHeaderFilter to include X-Trace-Id in HTTP responses\n- register the filter in SecurityConfig before AuthenticationFilter\n- resolve trace id from Tracer.currentSpan() with MDC(traceId) fallback\n- add unit tests for span, MDC fallback, and missing trace id cases\n- document X-Trace-Id in API.md, README.md, and CHANGELOG.md\n\n## Why\nImprove request diagnostics and correlation between client errors and server logs/traces, including security/rate-limit responses (401, 403, 429).\n\n## Verification\n- run: mvn -Dtest=TraceIdResponseHeaderFilterTest test -DskipITs\n- result: passed (3 tests, 0 failures)