Skip to content
This repository has been archived by the owner on May 30, 2024. It is now read-only.

Workaround to prevent transport: http2Server.HandleStreams failed to … #1

Merged

Conversation

tony-assembly
Copy link

@tony-assembly tony-assembly commented Sep 29, 2021

…read frame issue

@bmon @awilmore

This is the second part of the noisy logs for health checks, what are your thoughts on this, supposedly this works, and I have verified it locally, this is a fork of the original health check cmd that we build and use and I have applied only a small change.

This should prevent the lines:

[transport] transport: http2Server.HandleStreams failed to read frame: read tcp 127.0.0.1:8080->127.0.0.1:34024: read: connection reset by peer

the alternative is to do the same thing for this line with sumo, but I think this one would be fairly critical if it happens outside of health checks right?

let me know on your thoughts

@awilmore
Copy link

awilmore commented Nov 4, 2021

@tony-assembly the log filter is applied to an entire collector, so all resources running in a cluster will be subjected to the filter, and we could end up losing logs for non-related things.

In this case I'd go for the code change.

Copy link

@bmon bmon left a comment

Choose a reason for hiding this comment

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

I agree. While it's a shame to have to maintain a fork, the initial codebase and change are simple enough, so I think you've got the right approach here.

@tony-assembly
Copy link
Author

Thanks team! I will merge this in, are you guys happy to action whatever we need to do to replace the current health check with this?

@tony-assembly tony-assembly merged commit c2ae74d into master Nov 5, 2021
@tony-assembly tony-assembly deleted the feature/workaround-to-prevent-read-frame-error branch November 5, 2021 01:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants