-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(metrics): initialise MULTIPLEXED_METRIC_ROUTING_KEY for routing #19096
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
Changes from 19 commits
97bafa8
ab12a06
b77918f
9b635b1
a7632a9
37d54c2
00f559f
c2faff6
8a82bfd
d989887
d9854cb
f845c7b
1c836c5
350f662
d38b5f5
cb71961
222973e
6baf564
33c0d25
23800bf
b861ed2
e95310b
35c776a
58fccc9
cf7811c
a41729a
5f1fab4
fdccbd9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,7 @@ export { | |
| setHttpStatus, | ||
| makeMultiplexedTransport, | ||
| MULTIPLEXED_TRANSPORT_EXTRA_KEY, | ||
|
harshit078 marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still a breaking change |
||
| MULTIPLEXED_METRIC_ROUTING_KEY, | ||
| moduleMetadataIntegration, | ||
| supabaseIntegration, | ||
| instrumentSupabaseClient, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { getEnvelopeEndpointWithUrlEncodedAuth } from '../api'; | ||
| import type { Envelope, EnvelopeItemType, EventItem } from '../types-hoist/envelope'; | ||
| import type { Event } from '../types-hoist/event'; | ||
| import type { SerializedMetric, SerializedMetricContainer } from '../types-hoist/metric'; | ||
| import type { BaseTransportOptions, Transport, TransportMakeRequestResponse } from '../types-hoist/transport'; | ||
| import { dsnFromString } from '../utils/dsn'; | ||
| import { createEnvelope, forEachEnvelopeItem } from '../utils/envelope'; | ||
|
|
@@ -16,6 +17,7 @@ interface MatchParam { | |
| * @param types Defaults to ['event'] | ||
| */ | ||
| getEvent(types?: EnvelopeItemType[]): Event | undefined; | ||
| getMetric(): SerializedMetric | undefined; | ||
| } | ||
|
|
||
| type RouteTo = { dsn: string; release: string }; | ||
|
|
@@ -27,6 +29,8 @@ type Matcher = (param: MatchParam) => (string | RouteTo)[]; | |
| */ | ||
| export const MULTIPLEXED_TRANSPORT_EXTRA_KEY = 'MULTIPLEXED_TRANSPORT_EXTRA_KEY'; | ||
|
|
||
| export const MULTIPLEXED_METRIC_ROUTING_KEY = 'sentry.routing'; | ||
|
|
||
| /** | ||
| * Gets an event from an envelope. | ||
| * | ||
|
|
@@ -47,7 +51,71 @@ export function eventFromEnvelope(env: Envelope, types: EnvelopeItemType[]): Eve | |
| } | ||
|
|
||
| /** | ||
| * Creates a transport that overrides the release on all events. | ||
| * Gets a metric from an envelope. | ||
| * | ||
| * This is only exported for use in tests and advanced use cases. | ||
| */ | ||
| export function metricFromEnvelope(env: Envelope): SerializedMetric | undefined { | ||
|
harshit078 marked this conversation as resolved.
Outdated
|
||
| let metric: SerializedMetric | undefined; | ||
|
|
||
| forEachEnvelopeItem(env, (item, type) => { | ||
| if (type === 'trace_metric') { | ||
| const container = Array.isArray(item) ? (item[1] as SerializedMetricContainer) : undefined; | ||
| const containerItems = container?.items; | ||
| if (containerItems) { | ||
| metric = containerItems[0]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wouldn't this mean we always just pull the first metric?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes it would mean we pull the first metric. What I'm thinking now after your comment is that I can add a logic which will check if all metrics and route to same or multiple destinations. Whats your opinion ? |
||
| } | ||
| } | ||
| return !!metric; | ||
| }); | ||
|
|
||
| return metric; | ||
| } | ||
|
|
||
| /** | ||
| * Applies the release to all metrics in an envelope. | ||
| */ | ||
| function applyReleaseToMetrics(envelope: Envelope, release: string): void { | ||
| forEachEnvelopeItem(envelope, (item, type) => { | ||
| if (type === 'trace_metric') { | ||
| const container = Array.isArray(item) ? (item[1] as SerializedMetricContainer) : undefined; | ||
| if (container?.items) { | ||
| container.items = container.items.map(metric => ({ | ||
| ...metric, | ||
| attributes: { | ||
| ...metric.attributes, | ||
| 'sentry.release': { type: 'string', value: release }, | ||
| }, | ||
| })); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * It strips routing attributes from all metrics in an envelope. | ||
| * This prevents the routing information from being sent to Sentry. | ||
| */ | ||
| function stripRoutingAttributesFromMetrics(envelope: Envelope): void { | ||
| forEachEnvelopeItem(envelope, (item, type) => { | ||
| if (type === 'trace_metric') { | ||
| const container = Array.isArray(item) ? (item[1] as SerializedMetricContainer) : undefined; | ||
| const containerItems = container?.items; | ||
| if (containerItems) { | ||
| for (const metric of containerItems) { | ||
| if (metric.attributes && MULTIPLEXED_METRIC_ROUTING_KEY in metric.attributes) { | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| const { [MULTIPLEXED_METRIC_ROUTING_KEY]: _routing, ...restAttributes } = metric.attributes; | ||
| metric.attributes = restAttributes; | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
harshit078 marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Creates a transport that overrides the release on all events and metrics. | ||
| */ | ||
| function makeOverrideReleaseTransport<TO extends BaseTransportOptions>( | ||
| createTransport: (options: TO) => Transport, | ||
|
|
@@ -64,6 +132,9 @@ function makeOverrideReleaseTransport<TO extends BaseTransportOptions>( | |
| if (event) { | ||
| event.release = release; | ||
| } | ||
|
|
||
| applyReleaseToMetrics(envelope, release); | ||
|
|
||
| return transport.send(envelope); | ||
| }, | ||
| }; | ||
|
|
@@ -109,6 +180,27 @@ export function makeMultiplexedTransport<TO extends BaseTransportOptions>( | |
| ) { | ||
| return event.extra[MULTIPLEXED_TRANSPORT_EXTRA_KEY]; | ||
| } | ||
| const metric = args.getMetric(); | ||
| if (metric?.attributes?.[MULTIPLEXED_METRIC_ROUTING_KEY]) { | ||
| const routingAttr = metric.attributes[MULTIPLEXED_METRIC_ROUTING_KEY]; | ||
| let routingValue: unknown; | ||
| if (typeof routingAttr === 'object' && routingAttr !== null && 'value' in routingAttr) { | ||
| routingValue = routingAttr.value; | ||
| if (typeof routingValue === 'string') { | ||
| try { | ||
| routingValue = JSON.parse(routingValue); | ||
| } catch { | ||
| return []; | ||
|
harshit078 marked this conversation as resolved.
harshit078 marked this conversation as resolved.
|
||
| } | ||
| } | ||
| } else { | ||
| routingValue = routingAttr; | ||
| } | ||
|
|
||
| if (Array.isArray(routingValue)) { | ||
| return routingValue as RouteTo[]; | ||
| } | ||
| } | ||
| return []; | ||
| }); | ||
|
|
||
|
|
@@ -142,7 +234,11 @@ export function makeMultiplexedTransport<TO extends BaseTransportOptions>( | |
| return eventFromEnvelope(envelope, eventTypes); | ||
| } | ||
|
|
||
| const transports = actualMatcher({ envelope, getEvent }) | ||
| function getMetric(): SerializedMetric | undefined { | ||
| return metricFromEnvelope(envelope); | ||
| } | ||
|
|
||
| const transports = actualMatcher({ envelope, getEvent, getMetric }) | ||
|
cursor[bot] marked this conversation as resolved.
cursor[bot] marked this conversation as resolved.
|
||
| .map(result => { | ||
|
sentry[bot] marked this conversation as resolved.
|
||
| if (typeof result === 'string') { | ||
| return getTransport(result, undefined); | ||
|
|
@@ -152,6 +248,8 @@ export function makeMultiplexedTransport<TO extends BaseTransportOptions>( | |
| }) | ||
| .filter((t): t is [string, Transport] => !!t); | ||
|
|
||
| stripRoutingAttributesFromMetrics(envelope); | ||
|
cursor[bot] marked this conversation as resolved.
|
||
|
|
||
| // If we have no transports to send to, use the fallback transport | ||
| // Don't override the DSN in the header for the fallback transport. '' is falsy | ||
| const transportsWithFallback: [string, Transport][] = transports.length ? transports : [['', fallbackTransport]]; | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.