-
-
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 📦
|
AbhiPrasad
left a comment
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.
AbhiPrasad
left a comment
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
getMetricsAggregatorForClientexported function which is required so the Electron SDK can access the current aggregator.sendEnvelopewas added to theClienttype so that all usages ofBaseClient<ClientOptions>could be replaced withClient.