Skip to content

Commit

Permalink
make naming clear
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhiPrasad committed Aug 18, 2022
1 parent 1ccd07e commit de92e6c
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions text/0003-browser-js-built-in-metrics.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
* Start Date: 2022-08-18
* RFC Type: feature
* RFC PR: https://github.com/getsentry/rfcs/pull/3
* RFC Status: draft
* RFC Status: active

# Expanding Built-In Performance Metrics for Browser JavaScript.

# Summary

This RFC details expanding list of built-in performance metrics for the browser JavaScript SDK, with additional data the Browser SDK captures. It propose adding two metrics that are already captured by the SDK: `connection.rtt` and `connection.downlink`, and three brand new metrics that are not yet captured, `deviceMemory`, `hardwareConcurrency`, and `longTaskCount`.
This RFC details expanding list of built-in performance metrics for the browser JavaScript SDK, with additional data the Browser SDK captures. It propose adding two metrics that are already captured by the SDK: `connection.rtt` and `connection.downlink`, and three brand new metrics that are not yet captured, `device.memory`, `hardware.concurrency`, and `long_task.count`.

# Background

Expand Down Expand Up @@ -36,13 +36,13 @@ Either we choose to promote these two values to built-in measurements, or we rem

## New Data

In the same PR that added `connection.rtt` and `connection.downlink`, we also added support for grabbing [`deviceMemory`](https://developer.mozilla.org/en-US/docs/Web/API/Navigator/deviceMemory) and [`hardwareConcurrency`](https://developer.mozilla.org/en-US/docs/Web/API/Navigator/hardwareConcurrency). Currently these are set as [tags on the transaction](https://github.com/getsentry/sentry-javascript/blob/74db5275d8d5a28cfb18c5723575ea04c5ed5f02/packages/tracing/src/browser/metrics/index.ts#L405-L411). This should become performance metrics, as SDKs should not be settings tags like this one events, and there is value in seeing these numeric values on transactions.
In the same PR that added `connection.rtt` and `connection.downlink`, we also added support for grabbing [`deviceMemory`](https://developer.mozilla.org/en-US/docs/Web/API/Navigator/deviceMemory) and [`hardwareConcurrency`](https://developer.mozilla.org/en-US/docs/Web/API/Navigator/hardwareConcurrency). Currently these are set as [tags on the transaction](https://github.com/getsentry/sentry-javascript/blob/74db5275d8d5a28cfb18c5723575ea04c5ed5f02/packages/tracing/src/browser/metrics/index.ts#L405-L411). This should become performance metrics, as SDKs should not be settings tags like this one events, and there is value in seeing these numeric values on transactions. Here we would also rename `deviceMemory` -> `device.memory` and `hardwareConcurrency` -> `hardware.concurrency`.

Another additional option is to move all [browser Navigator](https://developer.mozilla.org/en-US/docs/Web/API/Navigator) related fields into a brand new SDK context - that is avaliable to all events, not just performance ones.

[Long Tasks](https://developer.mozilla.org/en-US/docs/Web/API/Long_Tasks_API) are JavaScript tasks that take 50ms or longer to execute. They are considered problematic because JavaScript is single threaded, so blocking the main thread is a big performance hit. In the SDK, [we track individual long task occurences as spans](https://github.com/getsentry/sentry-javascript/blob/74db5275d8d5a28cfb18c5723575ea04c5ed5f02/packages/tracing/src/browser/metrics/index.ts#L54-L59) and record them onto transactions.

In Sentry, we've been recording [`longTaskCount` on transactions](https://github.com/getsentry/sentry/blob/20780a5bdd988daa44825ce3c295452c280a9add/static/app/utils/performanceForSentry.tsx#L125) as a Custom Performance Metric for the Sentry frontend. So far, tracking the `longTaskCount` has been valuable as it allows us at a high level to see the most problematic transactions when looking at CPU usage. Since we already record long task spans in the SDK, it should be fairly easy to generate the count as a measurement, and promote into a built-in measurement.
In Sentry, we've been recording [`longTaskCount` on transactions](https://github.com/getsentry/sentry/blob/20780a5bdd988daa44825ce3c295452c280a9add/static/app/utils/performanceForSentry.tsx#L125) as a Custom Performance Metric for the Sentry frontend. So far, tracking the `longTaskCount` has been valuable as it allows us at a high level to see the most problematic transactions when looking at CPU usage. Since we already record long task spans in the SDK, it should be fairly easy to generate the count as a measurement, and promote into a built-in measurement. Here we would use `long_task.count` as the measurement name instead of `longTaskCount` that we used for internal testing.

## Decisions

Expand Down

0 comments on commit de92e6c

Please sign in to comment.