Skip to content

fix: log span_id next to trace_id#1225

Merged
ferhatelmas merged 1 commit into
masterfrom
ferhat/log-span
Jul 9, 2026
Merged

fix: log span_id next to trace_id#1225
ferhatelmas merged 1 commit into
masterfrom
ferhat/log-span

Conversation

@ferhatelmas

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Improvement for tracing logging

What is the current behavior?

#1224 added trace_id logging but it's not enough, because it doesn't know under which span to put logs.

What is the new behavior?

Similar to trace_id, log span_id as top level.

Additional context

For GC pressure, adapt get trace context to return full value and only slice for serialization without interim object.

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
@ferhatelmas ferhatelmas requested a review from a team as a code owner July 9, 2026 21:13
Copilot AI review requested due to automatic review settings July 9, 2026 21:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves tracing-related structured logging by promoting the W3C traceparent span identifier alongside the existing trace_id, and refactors traceparent parsing to reduce intermediate allocations.

Changes:

  • Refactor traceparent helper to return the validated full traceparent value (instead of slicing out only the trace id).
  • Add spanId to request log context and promote it to Logflare as top-level span_id.
  • Update request logging + Logflare + helper unit tests to cover spanId behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/internal/monitoring/request-context.ts Rename/refactor traceparent helper to return validated traceparent string.
src/internal/monitoring/request-context.test.ts Update unit tests for the new traceparent helper behavior.
src/internal/monitoring/logger.ts Extend RequestLog type to include spanId.
src/internal/monitoring/logflare.ts Promote spanId to top-level span_id in Logflare payload preparation.
src/internal/monitoring/logflare.test.ts Update Logflare payload tests to include span_id.
src/http/plugins/log-request.ts Parse span id from trace context and include it in request logs.
src/http/plugins/log-request.test.ts Update request logging tests to validate spanId threading/empty cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/http/plugins/log-request.ts
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 29050683086

Coverage decreased (-0.1%) to 79.325%

Details

  • Coverage decreased (-0.1%) from the base build.
  • Patch coverage: 9 of 9 lines across 3 files are fully covered (100%).
  • 20 coverage regressions across 3 files.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

20 previously-covered lines in 3 files lost coverage.

File Lines Losing Coverage Coverage
src/internal/http/agent.ts 17 32.56%
src/http/routes/s3/index.ts 2 82.99%
src/http/plugins/db.ts 1 81.82%

Coverage Stats

Coverage Status
Relevant Lines: 12633
Covered Lines: 10468
Line Coverage: 82.86%
Relevant Branches: 7323
Covered Branches: 5362
Branch Coverage: 73.22%
Branches in Coverage %: Yes
Coverage Strength: 424.07 hits per line

💛 - Coveralls

@ferhatelmas ferhatelmas merged commit d432f7d into master Jul 9, 2026
25 of 27 checks passed
@ferhatelmas ferhatelmas deleted the ferhat/log-span branch July 9, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants