Uh oh!
There was an error while loading. Please reload this page.
feat(tracing): Track PerformanceResourceTiming.renderBlockingStatus - #7127
Merged
Conversation
As per https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus the resource timing has information about whether an asset is blocking render or not, which is useful for determining which assets need to be addressed for fixing critical path.
k-fish
requested review from
a team, AbhiPrasad and lforst
and removed request for
a teamFebruary 9, 2023 17:38
k-fish added a commit
to getsentry/develop
that referenced
this pull request
Feb 9, 2023
Adds mention of new data on resource spans, see getsentry/sentry-javascript#7127
Contributor
size-limit report 📦
|
| data['Decoded Body Size'] = entry.decodedBodySize; | ||
| } | ||
| if ('renderBlockingStatus' in entry) { | ||
| data['Render Blocking Status'] = entry.renderBlockingStatus; |
Contributor
There was a problem hiding this comment.
so, I would like our data fields to move toward being similar to otel span attributes in formatting, lowercase and snake case, with period separators similar to span operation.
Can we use resource.render_blocking_status instead here?
see trace semantic conventions: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/README.md
MemberAuthor
There was a problem hiding this comment.
I was actually going to mention this, it was wild there are spaces etc. Very in 👍
Contributor
There was a problem hiding this comment.
yeah not ideal we have Decoded Body Size, maybe we can change this too though? I kept it as is for backwards compat sake, but if we can update the detectors we can change it here as well.
MemberAuthor
There was a problem hiding this comment.
If you want to look at changing these existing fields, we'd have to have both for a while (maybe 30d), are we fine with that?
Contributor
Replay SDK metrics 🚀
develop | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Revision | LCP | CLS | CPU | JS heap avg | JS heap max | netTx | netRx | netCount | netTime |
|---|---|---|---|---|---|---|---|---|---|
| 538c3a6 | +63.63 ms | +0.00 ms | +14.14 pp | +1.07 MB | +2.16 MB | +2.6 kB | +41 B | +1 | +112.08 ms |
| fc7b716 | +52.44 ms | +0.00 ms | +14.74 pp | +1.09 MB | +2.21 MB | +2.68 kB | +41 B | +1 | +89.64 ms |
*) pp - percentage points - an absolute difference between two percentages.
Last updated: Thu, 09 Feb 2023 20:39:45 GMT
Last updated: Thu, 09 Feb 2023 20:39:45 GMT
| 'Transfer Size': 1097, | ||
| 'Encoded Body Size': 797, | ||
| 'Decoded Body Size': 1885, | ||
| 'resource.render_blocking_status': 'non-blocking', |
Contributor
There was a problem hiding this comment.
One more bikeshed convo :p
Right now the blocked_main_thread span data key is set in the mobile sdks, to get File I/O perf issues working.
Could we re-use that? I'm fine with not - this works with me as well.
Contributor
There was a problem hiding this comment.
MemberAuthor
There was a problem hiding this comment.
To be clear you want to re-use the same key? I don't know about that, I'd prefer more closely matching the vendor implementation we're scraping from otherwise there will be work in the product to constantly explain the differences, vs. letting these keys be self-evident
Contributor
There was a problem hiding this comment.
Yup makes sense, let's keep this unique to browser!
AbhiPrasad
approved these changes
Feb 10, 2023
PerformanceResourceTiming.renderBlockingStatusk-fish added a commit
to getsentry/develop
that referenced
this pull request
Feb 13, 2023
* docs(sdk): Update render blocking status. Adds mention of new data on resource spans, see getsentry/sentry-javascript#7127
AbhiPrasad pushed a commit
that referenced
this pull request
Feb 14, 2023
…#7127) * ref(perf): Add renderBlockingStatus As per https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus the resource timing has information about whether an asset is blocking render or not, which is useful for determining which assets need to be addressed for fixing critical path.
ramchaik pushed a commit
to ramchaik/sentry-javascript
that referenced
this pull request
Feb 14, 2023
…getsentry#7127) * ref(perf): Add renderBlockingStatus As per https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus the resource timing has information about whether an asset is blocking render or not, which is useful for determining which assets need to be addressed for fixing critical path.
mjq-sentry pushed a commit
to getsentry/sentry
that referenced
this pull request
Feb 22, 2023
…44981) As of browser SDK 7.38.0, we now [log the render-blocking status](getsentry/sentry-javascript#7127) reported by the browser in resource spans' `data` hash. (Background info: [MDN](https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus)). If a value is present and that span is `non-blocking`, ignore our heuristics and treat the resource as non-blocking in the Large Render Blocking Asset detector. This commit also updates each test case to start from a valid event and then minimally mutate it into an invalid one, because I was losing confidence that each test was really testing what it should.
a-hariti pushed a commit
to getsentry/sentry-docs
that referenced
this pull request
Jun 8, 2024
* docs(sdk): Update render blocking status. Adds mention of new data on resource spans, see getsentry/sentry-javascript#7127
stephanie-anderson pushed a commit
to getsentry/sentry-docs
that referenced
this pull request
Jun 10, 2024
* docs(sdk): Update render blocking status. Adds mention of new data on resource spans, see getsentry/sentry-javascript#7127
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As per https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/renderBlockingStatus the resource timing has information about whether an asset is blocking render or not, which is useful for determining which assets need to be addressed for fixing critical path.