-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
feat(rum): Capture browser navigator information #2966
Conversation
packages/tracing/src/transaction.ts
Outdated
*/ | ||
public setContexts(contexts: Contexts): void { | ||
this._contexts = contexts; | ||
} |
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.
Unsure if there's a better way for this.
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.
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've decided to remove this altogether.
size-limit report
|
e0d238d
to
0330779
Compare
@@ -327,3 +370,7 @@ export function _startChild(transaction: Transaction, { startTimestamp, ...ctx } | |||
...ctx, | |||
}); | |||
} | |||
|
|||
function isMeasurementValue(measurement: any): measurement is number { |
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.
Using is number
should be fine here as it's generic type. We had a lot of issues with env-specific ones though :( #2277
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.
Gotcha. I removed the usage of this TS feature.
* Capture the information of the user agent. | ||
*/ | ||
private _trackNavigator(transaction: Transaction): void { | ||
const navigator = global.navigator as null | (Navigator & NavigatorNetworkInformation & NavigatorDeviceMemory); |
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.
If you'd use if (!('navigator' in global))
would it still require adding information/device types? If so, we can leave it as is.
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. TypeScript does not provide the built-in interfaces for NavigatorNetworkInformation
and NavigatorDeviceMemory
.
a722fee
to
221360d
Compare
We capture information from
window.navigator
that can provide insights into the browser (user agent) environment: https://developer.mozilla.org/en-US/docs/Web/API/NavigatorCapture network connectivity information (http://wicg.github.io/netinfo/) such as:
rtt
- effective round-trip time estimate in millisecondsdownlink
- the effective bandwidth estimate in megabits per second, rounded to nearest multiple of 25 kilobits per secondDevice information such as:
These properties provide insights into why some transactions are longer than they typically are (e.g. someone is accessing Sentry using a poor Internet connection).