-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add live performance counters #12343
Conversation
sdkVersion, | ||
skuId: SKU_ID, | ||
skuToken: this.skuToken, | ||
userId: this.anonId |
Check failure
Code scanning / CodeQL
Insecure randomness
sdkVersion, | ||
skuId: SKU_ID, | ||
"enabled.telemetry": false, | ||
userId: this.anonId |
Check failure
Code scanning / CodeQL
Insecure randomness
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.
Overall this looks good to me! However, could you please ensure that we don't accidentally drop any payload from the existing events such as MapLoadEvent
, MapSessionAPI
, and TurnstileEvent
?
6ce835b
to
e9c4e0b
Compare
* Add resource performance counters * Add mapbox URL regexs * Fix inverted gl VENDOR/RENDERER * Add unit tests * Add unit tests * Add unit tests * Fix lint * Add map constructor option enablePerformanceMetricsCollection * Add unit tests for metrics collection map option * Add unit tests for live_performance * Fix lint * Fix lint * Apply review comments * Address some review comments * Update style format * Remove harwdare concurrency * Address review comments * Revert unused * Store fullLoad before sending the event * Fix lint
* Add resource performance counters * Add mapbox URL regexs * Fix inverted gl VENDOR/RENDERER * Add unit tests * Add unit tests * Add unit tests * Fix lint * Add map constructor option enablePerformanceMetricsCollection * Add unit tests for metrics collection map option * Add unit tests for live_performance * Fix lint * Fix lint * Apply review comments * Address some review comments * Update style format * Remove harwdare concurrency * Address review comments * Revert unused * Store fullLoad before sending the event * Fix lint Co-authored-by: Karim Naaji <karim.naaji@gmail.com>
I'm a little concerned about the lack of communication from Mapbox about this feature. It looks like this has some serious implications for our product's privacy policy. I understand that this is incredibly valuable data to help improve the product - I'm just not happy that this has been essentially snuck into a release, defaulting to collecting quite detailed data. |
@rafraser thanks for flagging your concern, we're updating our release notes to provide additional clarity. Mapbox takes data privacy and security very seriously. In line with our privacy by design practices, the GL JS live performance counter was carefully designed so that user-level metrics and identifiers are not collected. We welcome any follow-on questions you may have regarding our ongoing commitment to data privacy and security. |
This PR adds live (production build) performance counters recorded through telemetry to our events API. The server schema and addition is described in https://github.com/mapbox/event-schema/pull/242 for server side ingestion (which should be merged prior to merging this PR). These counters will serve to monitor performance on real environment, and allow us to support the resource bundling effort.
Here is an example of the counters, metadata and attributes that are being collected:
attributes
counters
metadata
Most of the context specific information is necessary to bucket performance traces in comparable groups. For example, we wouldn't want to compare viewport sizes that are widely different for rendering performance, but it is probably ok to do so for network and resource fetching performance comparisons.
Some of the attributes also provide us functionality usage information, such as terrain, fog and projection being used. Since these may use very different code paths in our codebase, it's important for us to bucket them separately when comparing and retrieving performance knowledge.
For now, this PR focuses on resource fetching information, for fonts, style, tilejson, sprites and code. Time to render content (TTRC) and rendering performance counters will be introduced separately.
resource fetching range
Resource loading that are seen from chrome performance tab usually involves the timing including both the fetching and the processing of the received data:
in this PR the timing as referred to
spriteTransferStart
andspriteTransferEnd
refers to thenetworkTransfer
time mentioned above ranged from the earliest request for a resource category (css, javascript, sprites...) to the latest. For example, if two sprites network transfer duration were observed between the ranges(15ms, 80ms)
and(20ms, 100ms)
, we will return the range(15ms, 100ms)
.interaction range
One addition to this PR is the introduction of interaction range timings
I have considered adding an interaction history to our metrics, so that we can differentiate between a map that has interactions (where loading is similar to the context of benchmap, where you just load a map without any interaction), and not (for example a more complex interaction of user zooming in, panning... before the becomes idle). An interaction timeline would could have a format like this:
which would give, for example:
But for now, something as simple as:
is sufficient for our uses. It is less granular and only tells us the start and end of an any interaction, but that's enough for us to bucket all maps that do not have interaction on first load prior to being idle so that we compare them together.
The main idea being that we don't want to compare map performance for maps that don't have similar initial interaction behaviors and adding a full history of interaction will not be useful in order to do that bucketing. For example, we'd like to know when a map does a lot of extra tile loads before becoming idle due to excessive panning from the user since performance may be fairly different from a map without it.
It's important to note that this interaction range may be rounded to the ms due to the use of
performance.now()
.opting-out
This PR adds a map constructor option to opt-out of performance collection.
limitations
decodedBodySize
andtransferSize
), this also limits information about network transfer sizes for cors resourcesbenchmark
Launch Checklist
mapbox-gl-js
changelog:<changelog>Add live performance counters</changelog>