Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove redundant length fields #102

Closed
marten-seemann opened this issue Jul 8, 2020 · 3 comments
Closed

remove redundant length fields #102

marten-seemann opened this issue Jul 8, 2020 · 3 comments

Comments

@marten-seemann
Copy link
Collaborator

Once we require hex-encoding for byte-fields (which, as far as I can see from other peoples' qlogs, is already the de-facto standard), we don't need length fields any more. This applies to the scil and dcil in the Header (as well as the proposed token length in #94), the NewTokenFrame, NewConnectionIDFrame (and probably also the QPACK events, but I'm not an expert on those).

@rmarx
Copy link
Contributor

rmarx commented Jul 8, 2020

I kind of disagree, as I think an important use case is where you'd not want to log the raw values but only the lengths.

We can argue whether that's the case for the CIDs of course, but for tokens and qpack data (and, obviously, payloads), I think people should have the option to not log the raw values.

This flows from privacy/security concerns, as well as storage efficiency for larger deployments.

I do think we could add text stating something like "if you log the raw values in hex, the length field is redundant, and tools SHOULD be able to derive length from hex"?

@marten-seemann
Copy link
Collaborator Author

marten-seemann commented Jul 8, 2020

Good point. I can see why you want to log tokens (since they create a correlation spanning two separate connections).

I do think we could add text stating something like "if you log the raw values in hex, the length field is redundant, and tools SHOULD be able to derive length from hex"?

I'd make this an even stronger statement: ""if you log the raw values in hex, the length field is redundant and SHOULD (or MUST?) be omitted. Tools MUST be able to derive length from hex".

rmarx pushed a commit that referenced this issue Jul 11, 2020
- Updated examples

- Started to decouple JSON from the main schema and describe mapping

- Removed option to specify time_units (fixed #95)

- Overall (partially) relates to: #10, #36, #39, #89, #102, #30
rmarx pushed a commit that referenced this issue Aug 19, 2020
8594230.

- Partially relates to #10, #36, #39, #89, #102, #30

- TODO: not at all sure about the QPACK data types, need outside input

- Last commit before switching to separate draft02 branch
rmarx pushed a commit that referenced this issue Nov 2, 2020
@rmarx
Copy link
Contributor

rmarx commented Nov 2, 2020

Fixed by above commits.

Main draft now explains how truncated raw byte values should be accompanied by a length field and that tools should be able to derive the length field from the raw byte values.

Event definitions draft references this explicitly when discussing the RawInfo approach.

@rmarx rmarx closed this as completed Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants