getsentry / sentry-go

The official Go SDK for Sentry (sentry.io)
https://docs.sentry.io/platforms/go/
MIT License
897 stars 211 forks source link

Remove AddGlobalEventProcessor and globalEventProcessors #147

Open rhcarvalho opened 4 years ago

rhcarvalho commented 4 years ago

I propose removing the undocumented exported function AddGlobalEventProcessor together with the unexported global value it operates on globalEventProcessors.

https://github.com/getsentry/sentry-go/blob/763dc7f49323f467d4c62ec15804723ec5350b37/client.go#L28-L32

Rationale

All Event Processors that ship with the SDK are wrapped as an Integration and are attached to a Client. Namely:

This is in agreement with our decision to instead of exposing EventProcessors in the SDKs to hide them behind an Integration. The reason for this was that users don't have to deal with EventProcessors themselves, they only need to enable/disable Integrations.

I have checked 169 of the public packages that import sentry-go and none uses AddGlobalEventProcessor.

After removing AddGlobalEventProcessor, the same effect can be obtained with Scope.AddEventProcessor or Client.AddEventProcessor.

Note that an EventProcessor may still have its own global state if it desires so.

Pros:

Cons:

rhcarvalho commented 4 years ago

@kamilogorek @bruno-garcia @HazAT for comments, if any.

bruno-garcia commented 4 years ago

I'd like to see this change land.

kamilogorek commented 4 years ago

Sounds good to me. I followed the implementation of JS SDK to the T at first, thus why some parts might be unnecessary now.

github-actions[bot] commented 1 year ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀