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

Do not log context cancelled as error #392

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mhmxs
Copy link

@mhmxs mhmxs commented Jan 10, 2025

In my perspective context cancellation is not an error event, it is business as usual. WDYT?

@mhmxs mhmxs requested a review from a team as a code owner January 10, 2025 09:44
@mhmxs mhmxs force-pushed the dont-log-context-cancel branch from c11ff0a to 29a03b6 Compare January 10, 2025 09:45
@brandond
Copy link
Member

There are a LOT of places where functions that accept a Context may return an error. Other than a handful of specific places that caused a lot of log spam, we don't hide the error log if it is due to context cancellation. How did you single these out as places where we should suppress the error? Are you planning on adding any others?

@mhmxs
Copy link
Author

mhmxs commented Jan 10, 2025

@brandond

How did you single these out as places where we should suppress the error?

I ran 300h of various kubernetes e2e tests in the last two month and these were very annoying.

Are you planning on adding any others?

I didn't plan, because all sig-api-machinery|sig-apps|sig-auth|sig-instrumentation|sig-scheduling tests are running without a single context cancellation error message, so i'm happy, but what do you suggest?

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