-
Notifications
You must be signed in to change notification settings - Fork 200
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
telemetry: improve stack traces, error names, contexts #844
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Package redact implements functions to redact sensitive information from errors. A basic example is: name := "Alex" id := 5 err := redact.Errorf("error getting user %s with ID %d", name, Safe(id)) fmt.Println(err) // error getting user Alex with ID 5 fmt.Println(redact.Error(err)) // error getting user <redacted string> with ID 5 See the package docs and tests for more examples. This will allow us to get more information in Sentry without sending any identifying information.
mikeland73
approved these changes
Mar 30, 2023
Make `safeError` satisfy `fmt.Format` so that it can print stack traces the same way pkgs/errors does. Don't double redact errors so that already-redacted output isn't replaced with a placeholder.
Clean up and fix telemetry so that it can be actually useful. This commit is rather large because telemetry happens to touch a lot of different things. Example of what it looks like in Sentry: https://jetpack-io.sentry.io/issues/4047374127 Client ------ - Remove unnecessary telemetry configuration, such as `InitOpts`. Make the `build` package the source of truth for version information and link-time variables. Generate a global execution ID once at startup. - Standardize the contexts to use names and values recognized by Sentry. - Remove arbitrary CLI args and DEVBOX_ environment variables from the context, as these were leaking information. Errors ------ - In combination with the `redact` package, errors can now include data that has been marked as safe for telemetry. - Errors that haven't been marked as safe have a more helpful placeholder containing the chain of error types up until the first exported error. Stack Traces ------------ - Use the correct file and function names. This allows Sentry to group errors correctly. - Don't record every wrapped error. This doesn't make sense in Go since an error is a value containing the entire message. Otherwise you end up with a bunch of error messages that repeat themselves.
gcurtis
force-pushed
the
gcurtis/sentry-errors
branch
from
March 30, 2023 23:02
8720793
to
799c1c1
Compare
mikeland73
added a commit
that referenced
this pull request
Sep 27, 2024
## Summary We're running out of Sentry events and it looks like the culprit is devbox overlogging, including a lot of user errors that should not be logged. This PR #844 removed code that avoided logging user errors. I think this was a bug? If programmer wants a user error to be logged, they should use `WithLoggedUserMessage` instead. ## How was it tested? Untested
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clean up and fix telemetry so that it can be actually useful. This commit is rather large because telemetry happens to touch a lot of different things.
Example of what it looks like in Sentry: https://jetpack-io.sentry.io/issues/4047374127
Depends on #841.
Client
InitOpts
. Make thebuild
package the source of truth for version information and link-time variables. Generate a global execution ID once at startup.Errors
redact
package, errors can now include data that has been marked as safe for telemetry.Stack Traces