Skip to content

feat(core): Decouple metrics aggregation from client #10596

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

Closed

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Feb 10, 2024

Part of the work for #10595

This PR decouples metrics aggregation from the client and stores the aggregator globally the first time it is required.

This PR does not remove captureAggregateMetrics from the BaseClient or make sendEnvelope public as this can occur in a later PR. This will remove the remaining metrics code from the most basic tree-shaken Sentry configuration.

The Node metrics implementation remains in @sentry/core because it's required for Deno and Vercel-Edge which don't ref. @sentry/node.

There's a load of extra deletions and cleanup that can occur for v8 when the integration is deleted. For example BrowserMetricsAggregator can probably move to @sentry/browser.

@timfish timfish marked this pull request as ready for review February 10, 2024 17:46
@timfish timfish requested a review from AbhiPrasad February 10, 2024 17:49
@AbhiPrasad AbhiPrasad deleted the branch getsentry:metrics-summary February 12, 2024 15:16
@AbhiPrasad AbhiPrasad closed this Feb 12, 2024
@timfish timfish deleted the feat/decouple-metrics branch February 12, 2024 23:52
AbhiPrasad pushed a commit that referenced this pull request Feb 16, 2024
This is a re-opening of #10596 which was closed automatically after the
branch is was based off was closed.

Part of the work for #10595

This PR decouples metrics aggregation from the client and stores the
aggregator globally the first time it is required.

This PR does not remove `captureAggregateMetrics` from the `BaseClient`
or make `sendEnvelope` public as this can occur in a later PR. This will
remove the remaining metrics code from the most basic tree-shaken Sentry
configuration.

The default (node) metrics implementation remains in `@sentry/core`
because it's required for Deno and Vercel-Edge which don't ref.
`@sentry/node`.

There's a load of extra deletions and cleanup that can occur for v8 when
the integration is deleted. For example `BrowserMetricsAggregator` can
probably move to `@sentry/browser`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants