-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(tracemetrics): Add trace metrics behind an experiments flag #17883
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
base: develop
Are you sure you want to change the base?
Conversation
This allows the js sdk to send in new trace metric protocol items, although this code is experimental since the schema may still change. Most of this has been copied from logs so some parts may need to be modified / removed later (eg. buffer) but this should allow us to start on UI work by sending in larger amounts of data from sentry js app to test grouping / aggregations etc.
…dd-experimental-metrics
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
* @param value - The value of the metric attribute. | ||
* @returns The serialized metric attribute. | ||
*/ | ||
export function metricAttributeToSerializedMetricAttribute(value: unknown): SerializedMetricAttributeValue { |
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.
nit: We can move this into a shared util with logs
|
||
captureSerializedMetric(client, serializedMetric); | ||
|
||
client.emit('afterCaptureMetric', metric); |
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.
q: Why no beforeCaptureMetric
hook?
@k-fish looks good! I'll hijack your PR so we can land this today |
type: 'trace_metric', | ||
item_count: items.length, | ||
content_type: 'application/vnd.sentry.items.trace-metric+json', | ||
} as MetricContainerItem[0], |
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.
Bug: Metric Type Mismatch Causes Processing Issues
The createMetricEnvelope
test expects the envelope item header type to be 'metric'
, but the createMetricContainerEnvelopeItem
function correctly sets it to 'trace_metric'
. This test expectation mismatch points to a potential ambiguity in the system regarding the canonical envelope item type for metrics, which could affect processing if 'trace_metric'
is internally mapped to 'metric'
in data category handling.
Summary
This allows the js sdk to send in new trace metric protocol items, although this code is experimental since the schema may still change. Most of this has been copied from logs so some parts may need to be modified / removed later (eg. api, buffer) but this should allow us to start on UI work by sending in larger amounts of data from sentry js app to test grouping / aggregations etc.