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

Don't trace gRPC "Canceled" errors #1353

Merged
merged 1 commit into from
Sep 14, 2022

Conversation

caboteria
Copy link
Contributor

Description

We were getting a lot of log chaff at TRACE level during normal operation because gRPC returns a "context canceled" error periodically but it's not really an error for us because we immediately loop around again.

This commit uses the gRPC status.Code() method to identify these non-errors. It also uses errors.Is() instead of == to compare error objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Issue link

#1068

How Has This Been Tested?

  • Added unit testing to cover
  • Tested manually
  • Tested by integration testing
  • Have not tested

Deploy cmd-nse-icmp-responder, see periodic "context cancelled" errors, deploy build with patched SDK, verify that errors aren't present.

Types of changes

  • Bug fix
  • New functionallity
  • Documentation
  • Refactoring
  • CI

Only relevant when running at TRACE log level.

Copy link
Member

@denis-tingaikin denis-tingaikin 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.

Could you sign the commit?

@caboteria
Copy link
Contributor Author

I think that I signed the commit but please let me know if something's wrong as I'm new to git signatures.

@denis-tingaikin
Copy link
Member

@caboteria Seems like it still requires to sign the commit. See at DCO check https://github.com/networkservicemesh/sdk/pull/1353/checks?check_run_id=8351758816

In short, you can just do something simple like

git reset --soft HEAD^
git commit -m 'TODO' **-s**

We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>
@ghost
Copy link

ghost commented Sep 14, 2022

Ah, I get it. Unfortunately confusing git terminology! I was signing the commit (-S) but it sounds like you want me to signoff (-s) the commit. It should be all set now, I did both.

@denis-tingaikin
Copy link
Member

@acnodal-tc It's a useful fix. Thanks!

@denis-tingaikin denis-tingaikin merged commit 97e00ec into networkservicemesh:main Sep 14, 2022
nsmbot pushed a commit to networkservicemesh/cmd-cluster-info-k8s that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/sdk-kernel that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr-proxy that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-registry-proxy-dns that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-nse-vfio that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-ipam-vl3 that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-nse-remote-vlan that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-admission-webhook-k8s that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-map-ip-k8s that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-nsc-init that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
nsmbot pushed a commit to networkservicemesh/cmd-registry-memory that referenced this pull request Sep 14, 2022
…k@main

PR link: networkservicemesh/sdk#1353

Commit: 97e00ec
Author: toby cabot
Date: 2022-09-14 19:33:40 -0400
Message:
  - Don't trace gRPC "Canceled" errors (#1353)
We were getting a *lot* of log chaff at TRACE level during normal
operation because gRPC returns a "context canceled" error periodically
but it's not really an error for us because we immediately loop around
again.

This commit uses the gRPC status.Code() method to identify these
non-errors. It also uses errors.Is() instead of == to compare error
objects. This handles wrapped errors.

https://pkg.go.dev/google.golang.org/grpc/status#Code

https://github.com/golang/go/wiki/ErrorValueFAQ#how-should-i-change-my-error-handling-code-to-work-with-the-new-features

Signed-off-by: Toby Cabot <[email protected]>

Signed-off-by: Toby Cabot <[email protected]>
Signed-off-by: NSMBot <[email protected]>
@ghost
Copy link

ghost commented Sep 15, 2022

Thank you for your patience in helping me get the patch in shape to merge!

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