-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Add optional Sentry support #2386
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
Conversation
|
@disq I'm wondering if it would not be better to have DSNs hardcoded in plugins as we used to have (trough a SDK option) rather than having them injected on-demand since the latter could cause mix-matches between plugins. This way we can opt-in Sentry, but the DSNs would be plugin-tied rather than dynamic. |
| Long: servePluginShort, | ||
| Args: cobra.NoArgs, | ||
| RunE: func(cmd *cobra.Command, _ []string) error { | ||
| doSentry, _ := strconv.ParseBool(os.Getenv("CQ_SENTRY_ENABLED")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the thing to enable sentry.
| Args: cobra.NoArgs, | ||
| RunE: func(cmd *cobra.Command, _ []string) error { | ||
| doSentry, _ := strconv.ParseBool(os.Getenv("CQ_SENTRY_ENABLED")) | ||
| if doSentry && noSentry { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For future us, but we'll probably want to remove noSentry.
| ServerName: "oss", // set to "oss" on purpose to avoid sending any identifying information | ||
| // https://docs.sentry.io/platforms/go/configuration/options/#removing-default-integrations | ||
| Integrations: func(integrations []sentry.Integration) []sentry.Integration { | ||
| var filteredIntegrations []sentry.Integration | ||
| for _, integration := range integrations { | ||
| if integration.Name() == "Modules" { | ||
| continue | ||
| } | ||
| filteredIntegrations = append(filteredIntegrations, integration) | ||
| } | ||
| return filteredIntegrations | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very old code :)
Part of https://linear.app/cloudquery/issue/ENG-2544/
Goes with cloudquery/plugin-pb-go#600Brings back sentry init and panic handling from #1724