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

Switch to trace logging for frame recieved messages #765

Closed
wants to merge 1 commit into from

Conversation

orf
Copy link

@orf orf commented Apr 13, 2024

Enabling DEBUG tracing/logging when using h2 results in a very, very large volume of messages being logged: 3,706 messages when making a small number of requests.

Emitting a message on every frame received feel much more like it should be trace messages, not debug messages. This PR switches them to trace instead.

Screenshot 2024-04-13 at 20 15 12

@seanmonstar
Copy link
Member

I get that it's a lot of frames, and they might not be that interesting, but then again, I do think the frame events are more important than all the other events emitted at trace level. Especially if you're trying to debug what the communication to the peer was.

@orf
Copy link
Author

orf commented Apr 14, 2024

Perhaps there’s a middle ground here? “Uninteresting” frames, like data frames, could be logged with trace, but less-common ones could be logged as debug messages? Ones that would indicate interesting situations for debugging.

And maybe higher-level things, like “request completed: sent this many bytes, received this many bytes”. Right now, it’s just tracing the happy-path: connections are working, data is being sent. The signal-to-noise ratio seems really off.

Personally, and with someone with little http2 experience, I’m not sure how these messages would help me debug communication issues. It only contains the stream ID and not other relevant information like the size of the frame, which I’d assume would be useful to debug issues?

@seanmonstar
Copy link
Member

The length would be a valuable thing to add.

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