Skip to content

qlogwriter: implement the draft-12 trace header#5360

Merged
marten-seemann merged 1 commit intomasterfrom
qlogwriter
Oct 8, 2025
Merged

qlogwriter: implement the draft-12 trace header#5360
marten-seemann merged 1 commit intomasterfrom
qlogwriter

Conversation

@marten-seemann
Copy link
Copy Markdown
Member

Requires quiclog/qvis#90, otherwise our qlogs won't be accepted by qvis without a warning.

@marten-seemann marten-seemann requested a review from Copilot October 8, 2025 10:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 implements the draft-12 trace header format for qlog files, updating the qlogwriter package to use a simplified traceHeader structure instead of the previous nested topLevel and trace structures.

  • Replaces complex nested trace structures with a streamlined traceHeader implementation
  • Updates the reference time format to use RFC3339Nano string format with clock metadata instead of numeric milliseconds
  • Refactors tests to use a shared helper function and validate the new header format

Reviewed Changes

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

File Description
qlogwriter/writer.go Simplifies trace initialization by directly creating traceHeader instead of nested structures
qlogwriter/trace.go Implements new traceHeader struct replacing topLevel, trace, vantagePoint, and commonFields
qlogwriter/trace_test.go Consolidates test functions and updates assertions to validate new header format

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread qlogwriter/trace.go
VantagePoint vantagePoint
CommonFields commonFields
}
// The following fields are not required by the qlog draft anymore,
Copy link

Copilot AI Oct 8, 2025

Choose a reason for hiding this comment

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

The comment mentions 'qlog draft' but should specify 'draft-12' for clarity since there are multiple qlog drafts.

Suggested change
// The following fields are not required by the qlog draft anymore,
// The following fields are not required by qlog draft-12 anymore,

Copilot uses AI. Check for mistakes.
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 8, 2025

Codecov Report

❌ Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.04%. Comparing base (c2131eb) to head (b670eba).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
qlogwriter/writer.go 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5360      +/-   ##
==========================================
+ Coverage   83.02%   83.04%   +0.01%     
==========================================
  Files         156      156              
  Lines       18743    18723      -20     
==========================================
- Hits        15561    15547      -14     
  Misses       2563     2563              
+ Partials      619      613       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marten-seemann marten-seemann merged commit 33af127 into master Oct 8, 2025
50 of 53 checks passed
@marten-seemann marten-seemann deleted the qlogwriter branch October 11, 2025 05:58
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.

2 participants