-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(tracing): Change resource span op name and add data #2816
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 4 commits
f45595c
943e297
1dffa3e
49a5d6e
bfa8cca
63013d6
bf45791
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 |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| /* eslint-disable max-lines */ | ||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
| import { SpanContext } from '@sentry/types'; | ||
| import { getGlobalObject, logger } from '@sentry/utils'; | ||
|
|
@@ -211,10 +212,17 @@ function addMeasureSpans( | |
| return measureStartTimestamp; | ||
| } | ||
|
|
||
| export interface ResourceEntry extends Record<string, unknown> { | ||
| initiatorType?: string; | ||
| transferSize?: number; | ||
| encodedBodySize?: number; | ||
| decodedBodySize?: number; | ||
| } | ||
|
|
||
| /** Create resource related spans */ | ||
| function addResourceSpans( | ||
| export function addResourceSpans( | ||
| transaction: Transaction, | ||
| entry: Record<string, any>, | ||
| entry: ResourceEntry, | ||
| resourceName: string, | ||
| startTime: number, | ||
| duration: number, | ||
|
|
@@ -226,14 +234,26 @@ function addResourceSpans( | |
| return undefined; | ||
| } | ||
|
|
||
| const data: Record<string, any> = {}; | ||
| if ('transferSize' in entry) { | ||
| data.transferSize = entry.transferSize; | ||
| } | ||
| if ('encodedBodySize' in entry) { | ||
| data.encodedBodySize = entry.encodedBodySize; | ||
| } | ||
| if ('decodedBodySize' in entry) { | ||
| data.decodedBodySize = entry.decodedBodySize; | ||
| } | ||
|
|
||
|
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. We also need a timing (such as
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. We also could include some sort of comparison (eg.
Member
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. I will leave this for the future, merging this in for now. Opening a ticket. |
||
| const startTimestamp = timeOrigin + startTime; | ||
| const endTimestamp = startTimestamp + duration; | ||
|
|
||
| _startChild(transaction, { | ||
| description: `${entry.initiatorType} ${resourceName}`, | ||
| description: resourceName, | ||
| endTimestamp, | ||
| op: 'resource', | ||
| op: entry.initiatorType ? `resource.${entry.initiatorType}` : 'resource', | ||
| startTimestamp, | ||
| data, | ||
| }); | ||
|
|
||
| return endTimestamp; | ||
|
|
||
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.
Given that arbitrary data fields are not special-cased in the UI to make them read nice, would it be desirable here to use a "pretty" format like
data["Transfer Size"]? cc @dashedSpecifically:
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.
Yep that should work fine.