-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Allow metrics aggregator per client #10949
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
I'm half tempted to make it the last function parameter with a default of function increment(name: string, value: number = 1, data?: MetricData, client: Client = getClient()): void { |
size-limit report 📦
|
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.
I don't mind passing client
into the data bag, I'd rather not introduce an additional function argument.
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.
Do we have to backport this to v7?
/** | ||
* @ignore This is for internal use only. | ||
*/ | ||
getMetricsAggregatorForClient, |
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.
Actually should we export this as a separate method to help with treeshaking?
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.
The getMetricsAggregatorForClient
function is in the call chain for all the other Sentry.metrics.*
functions so there will be no impact to tree shaking.
Unlikely! |
This PR adds the ability to override the client used for metrics rather than being tied to using the single client
getClient()
returns.It also adds a
getMetricsAggregatorForClient
exported function which is required so the Electron SDK can access the current aggregator.sendEnvelope
was added to theClient
type so that all usages ofBaseClient<ClientOptions>
could be replaced withClient
.