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

Prepare for adoption #155

Merged
merged 11 commits into from May 15, 2021
Merged

Prepare for adoption #155

merged 11 commits into from May 15, 2021

Conversation

rmarx
Copy link
Contributor

@rmarx rmarx commented May 1, 2021

This PR tracks the changes necessary to prepare for adoption by the QUIC wg as defined in #137.

It currently contains all the text changes, still requiring a few other things after they are approved (remove old document files, switch master branch to main, submit updated docs as individual drafts for the adoption call.

@rmarx
Copy link
Contributor Author

rmarx commented May 1, 2021

For those of you who don't use a local checkout of this to view the resulting HTML after the changes, I've attached them separately:

pre-adoption-html-v1.zip

@rmarx rmarx linked an issue May 1, 2021 that may be closed by this pull request
@rmarx rmarx mentioned this pull request May 1, 2021
QPACK I-D [QUIC-QPACK]. QUIC events are defined in a separate document
[QLOG-QUIC].

Feedback and discussion welcome at [TODO](TODO). Readers are advised to refer to
Copy link
Member

@LPardue LPardue May 3, 2021

Choose a reason for hiding this comment

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

You could still point people to this repo in order to collect any adoption call feedback. Then, any and all issues can be brought across to the QUIC WG org when the repo is transferred. And we can update the URL to that new one when the adopted -00 gets published.

@@ -73,6 +73,8 @@ informative:

--- abstract

=== beginning the split into two separate documents for quicwg adoption ===
Copy link
Member

Choose a reason for hiding this comment

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

Is the intent to delete this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, this one is removed and replaced by the two new ones.

Copy link
Member

Choose a reason for hiding this comment

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

so uhh, why isn't it git rm'd ? :D

@rmarx
Copy link
Contributor Author

rmarx commented May 3, 2021

Two other points raised by Lucas:

  • Add more information about "previous versions" (e.g., two main versions in production, -01 and -02, those pre-date these adopted documents)
  • Define a new version codepoint for the adopted drafts (now it's still draft-03-wip, but we also can't just do draft-00 or we'll overlap soon. Maybe qlog-0x?)

@LPardue
Copy link
Member

LPardue commented May 5, 2021

Do we also need independent versioning of the different schema? How does qlog evolve over the years? For example, say the main schema got published as 1.0, and then work commenced on 2.0. Can there be a QUIC qlog event definitions 1.1 (which is updated say to support QUIC v2) and a 2.1 (which is updated to support QUIC v2 and a new main schema format)?

There's a lot of possible bikeshedding here. For now, maybe you just want to keep your linear versioning scheme (qlog 01, 02, and next 03) and make it clear that it is decoupled from any document draft id version.

Copy link
Collaborator

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

LGTM, but as @LPardue pointed out, don't we need to delete the old documents?

Copy link
Collaborator

@lnicco lnicco left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks Robin for doing this.

I have a couple minor comments and a generic comment about considering making the document more strict in the future.
Right now it reads like a "hands on experience" document, which is a great start but I think the end goal could be to make some of the text more strict to provide more precise guidelines to QLOG (and QLOG tools) implementors.

But that's a separate discussion.
Ship it!

@@ -753,14 +777,14 @@ JSON serialization for events grouped by four tuples and QUIC connection IDs:
events: [
{
time: 1553986553579,
protocol_type: "TCP_HTTPS2",
protocol_type: ["TCP", "HTTPS2"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is HTTPS2 and HTTP3 ?
If Secure is implicit with QUIC but not with TCP, maybe this should be ["TCP", "TLS", "HTTP2"] ?

TODO: pending QUIC working group discussion. This text reflects the initial (qlog
draft 01 and 02) setup.

There are several ways of defining qlog events. In practice, we have seen two main
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is fine for now, but it could be more prescriptive in the future.
Allowing total freedom in the events definition can make the development of generic tools harder.

event definitions [QLOG-H3].

This section defines some basic annotations and concepts the creators of event
definition documents should follow to ensure a measure of consistency, making it
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this a SHOULD ? not sure

@rmarx rmarx merged commit 514b0d4 into master May 15, 2021
@rmarx
Copy link
Contributor Author

rmarx commented May 15, 2021

Thanks all for the reviews!

@lnicco I fully agree that the current text isn't really fit for a proper I-D and especially eventual RFC. I'm hoping we can make things much stricter and clearer following wg consensus on some main points. I'm also hoping the new editors will help re-write some things when that happens ;)

I have just fixed the remarks (hopefully), merged the PR and tried to submit the new drafts to the datatracker.
I ran into some problems due to not being familiar with the process there though, doing something different for all 3 documents...

  • main-schema-03 should be published as normal (was somehow auto-posted to the QUIC list...)
  • quic-events-00 somehow ended up as having to go through a manual review process (editors got an email about that)
  • h3-events-00 has to be approved by the QUIC wg chairs first (no clue why/how that triggered, but @LPardue got an email for that, so...)

So you'll probably see some weird emails and the 2 event-definition documents are somewhat delayed behind main-schema, but hopefully things will be sorted soon!

@rmarx
Copy link
Contributor Author

rmarx commented May 17, 2021

After guidance from the QUIC wg chairs, I have now cancelled submission of:

  • draft-marx-qlog-quic-events-00
  • draft-marx-qlog-h3-events-00

And instead re-submitted as:

  • draft-marx-quic-qlog-quic-events-00
  • draft-marx-quic-qlog-h3-events-00

to make it clearer these are adoption candidates. These are both still pending approval from the QUIC wg chairs, so not quite published yet.

@rmarx
Copy link
Contributor Author

rmarx commented May 17, 2021

The soap continued... apparently this all triggered a bug in datatracker when trying to replace documents that are in pre-adoption with a wg (it's not quite a compiler bug, but I'll take it for now ;)).

The chosen solution is to submit the new drafts again, without indicating they replace the old joint h3+quic events draft. The idea is that the backlinking will be corrected post-adoption.

For clarity, these are the most recent drafts that the adoption call will be issued for:

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.

Prepare for adoption
4 participants