Skip to content
This repository was archived by the owner on Aug 30, 2023. It is now read-only.

Conversation

@metlos
Copy link
Contributor

@metlos metlos commented Feb 25, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Currently, Sentry only allows for callback-style initialization using OptionsConfiguration interface passed into the Sentry.init() methods. This PR makes available to users also the initialization method that merely accepts the SentryOptions instance.

Additionally, I've removed the unnecessary generic parameter on the init method that was made public, because it naturally accepts any subclass of SentryOptions just by inheritance rules.

💡 Motivation and Context

The callback-based initialization is limiting in the injection-based environments like Spring or CDI where instances are produced and injected rather than modified through a callback.

💚 How did you test it?

N/A

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

@bruno-garcia
Copy link
Member

@metlos if we do this there's no way anymore to run code on the object before we give the user a chance to change things. So to know in what is framework config vs user config becomes more complicated.
Is there a way to just give these frameworks the object we created instead of letting them give us the instance?

@metlos
Copy link
Contributor Author

metlos commented Feb 27, 2020

if we do this there's no way anymore to run code on the object before we give the user a chance to change things. So to know in what is framework config vs user config becomes more complicated.
Is there a way to just give these frameworks the object we created instead of letting them give us the instance?

So basically you need to distinguish between Sentry framework config vs user config? So what about just using 2 different objects for that?

@bruno-garcia
Copy link
Member

@metlos everything gets set to an instance of SentryOptions but if a user programatically sets a value, that must take presedence (override) anything set by some framework integration.

So if the frameworks read a JSON config and binding the values to SentryOptions we need that to happen before applying the user defined callback (programaticaly defined settings).

@metlos
Copy link
Contributor Author

metlos commented Feb 27, 2020

Ok, let's keep this PR open and I'll try to come up with a reasonably "natural" of doing things in Spring that would not require this change. If I am not able to come with something similar to what we have in sentry-java, we can discuss it further here..

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leave a review until we make a decision on this.
As per @metlos last comment

@marandaneto
Copy link
Contributor

@marandaneto marandaneto closed this Sep 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants