docs(go): Remove retired telemetry kill switches - #19129
Conversation
Update Go onboarding, logging, metrics, and configuration docs for the removal of DisableLogs and DisableMetrics in sentry-go. Co-Authored-By: GPT-5 Codex <noreply@openai.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Give the Logs product toggle an executable NewLogger example instead of only describing the default behavior. Co-Authored-By: GPT-5 Codex <noreply@openai.com>
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Some small questions/suggestions
| err := sentry.Init(sentry.ClientOptions{ | ||
| Dsn: "___PUBLIC_DSN___", | ||
| // Note: Structured Logs are enabled by default. To disable them, set DisableLogs: true. | ||
| // Note: Structured Logs are enabled by default. |
There was a problem hiding this comment.
l: I would just remove this whole line; keeping it implies that disabling logs is possible, but I guess it is not
| // Note: Structured Logs are enabled by default. |
| // visit: https://docs.sentry.io/platforms/go/data-management/data-collected/ for more info | ||
| SendDefaultPII: true, | ||
| // Logs are enabled by default. To disable them, set DisableLogs: true. | ||
| // Logs are enabled by default. |
There was a problem hiding this comment.
l: same as above
| // Logs are enabled by default. |
|
|
||
| // ___PRODUCT_OPTION_START___ logs | ||
| logger := sentry.NewLogger(context.Background()) | ||
| logger.Info().Emit("User example action completed") | ||
| // ___PRODUCT_OPTION_END___ logs |
There was a problem hiding this comment.
m: This is just an example, right? Or, are any of these lines needed to enable logs?
If logs are enabled by default and these are just examples, I would lean towards not even including the logs checkbox at all. Otherwise, keeping makes sense.
There was a problem hiding this comment.
Yeah just examples. I thought it might be nicer to keep the checkbox and show a small snippet. That's also what js are doing
There was a problem hiding this comment.
I am not sure that makes sense to do; I would be concerned that users get confused and think this is a necessary step, not just an example.
But fair enough, if you would like to merge the PR like this, it is fine with me
There was a problem hiding this comment.
Fair argument, since people can just use an integration rather than the NewLogger api. I'll remove the toggle then.
2 participants
DESCRIBE YOUR PR
Go onboarding, logging, metrics, and configuration docs now match sentry-go#1392. The removed
DisableLogsandDisableMetricsoptions are no longer shown, and selecting Logs now adds a concisesentry.NewLoggerinvocation.IS YOUR CHANGE URGENT?
PRE-MERGE CHECKLIST