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

Qlog 0 2 mystery #518

Closed
wants to merge 11 commits into from
Closed

Qlog 0 2 mystery #518

wants to merge 11 commits into from

Conversation

agrover
Copy link
Contributor

@agrover agrover commented Apr 2, 2020

I have been working on rebasing our in-progress qlog PR (473) and also moving to qlog 0.2.0 crate version. In doing so I'm having some extreme weirdness. The changes needed for qlog 0.2 are nothing special, but for some reason cause two other, seemingly-unrelated spots of our code to start throwing errors!

  • Build this PR, including last commit that fixes other spots: Compiles.
  • Build this PR minus the top commit. Doesn't compile.
  • Build this PR minus the top two commits, the second-from-top commit encompassing the changes for qlog 0.2: Compiles

What's going on here?

Andy Grover and others added 11 commits April 1, 2020 15:43
Add optional qlog context when creating a Connection.

Pass qlog context into Connection from neqo-client. Add cmdline option
to save qlog to a file.

Implement initial trace hooks for client connected start and packet
received. Still a lot more to do.

Pull out frame and pkt type conversion into helper functions.

Move stuff into common and implement server logging on connection close

Move Role and a little qlog stuff into common.

Server: Catch the connection closed event and use that to generate
qlog output. Get the connection ID so we can write separate logs for
each server connection.
In order to include a NeqoQlogRef (and by extension a NeqoQlog)
in a struct that derives Debug, NeqoQlog must itself implement
the Debug trait.
Propogate optional NeqoQlogRefs to the various Http3
subsystems for them to use and (perhaps) pass to the
connections and other components that they instantiate
that can log qlog data.
Add optional NeqoQlogRefs as fields in the Encoder and
Decoder structures. Also update the parameters to their
new() functions for setting those fields.
Besides refactoring, there are several other changes:
1. Add a test
2. Fix the implementation to match the current qlog spec
for formatting a number as a hexadecimal string.
@agrover agrover added bug Something isn't working work-in-progress Incomplete PR that should not be merged yet labels Apr 2, 2020
@agrover
Copy link
Contributor Author

agrover commented Apr 2, 2020

This is with either rustc 1.41.1 or 1.42, btw.

@agrover
Copy link
Contributor Author

agrover commented Apr 2, 2020

It's this: rust-lang/rust#46257

@agrover agrover closed this Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working work-in-progress Incomplete PR that should not be merged yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants