Closed ColinRosati closed 6 months ago
Hey @ColinRosati great suggestion that makes a lot sense. Thanks you for that!
Do you would like to implement it / make a contribution? Otherwise I can quickly tackle it today because the change isn't bug and I can include it in this weeks release.
Note: This prop is currently only available in Chrome but given the big market share of Chrome it's already valuable to add.
Cheers, Marco
@ColinRosati Any other feedback about the instrumentation is highly welcome. It's a very opinionated implementation so feedback is amazingly helpful.
Cheers, Marco
Hey @codecapitano I would love to push the suggestion. Let me open a PR for this one 🙌
Hey @codecapitano I would love to push the suggestion. Let me open a PR for this one 🙌
Great, thank you so much 🚀 I'll take care of the docs then.
PS: Please also update the changelog (slips through quite often)
@codecapitano Looks like my PR CI is blocked and says to contact admin. Are you able to help there?
Description
Add response status to faro resource event
FaroResourceTiming
. This would be really helpful to create metrics based on specific http statusesProposed solution
We already have resource status available in scope https://github.com/grafana/faro-web-sdk/blob/c33caee495826c6c73443114c791ef2158af15ef/packages/web-sdk/src/instrumentations/performance/performanceUtils.ts#L47
Prosed solution would be to add one more field
responseStatus
Proposal
### packages/web-sdk/src/instrumentations/performance/performanceUtils.ts ```js export function calculateFaroResourceTiming(resourceEntryRaw: PerformanceResourceTiming): FaroResourceTiming { const { connectEnd, connectStart, decodedBodySize, domainLookupEnd, domainLookupStart, duration, encodedBodySize, fetchStart, initiatorType, name, nextHopProtocol, redirectEnd, redirectStart, // @ts-expect-error the renderBlockingStatus property is not available in all browsers renderBlockingStatus: rbs, requestStart, responseEnd, responseStart, // @ts-expect-error the renderBlockingStatus property is not available in all browsers responseStatus, // responseStatus already available to use secureConnectionStart, transferSize, workerStart, } = resourceEntryRaw; return { name: name, duration: toFaroPerformanceTimingString(duration), tcpHandshakeTime: toFaroPerformanceTimingString(connectEnd - connectStart), dnsLookupTime: toFaroPerformanceTimingString(domainLookupEnd - domainLookupStart), tlsNegotiationTime: toFaroPerformanceTimingString(requestStart - secureConnectionStart), responseStatus, // <------------- add here new responseStatus field with any normalization redirectTime: toFaroPerformanceTimingString(redirectEnd - redirectStart), requestTime: toFaroPerformanceTimingString(responseStart - requestStart), responseTime: toFaroPerformanceTimingString(responseEnd - responseStart), fetchTime: toFaroPerformanceTimingString(responseEnd - fetchStart), serviceWorkerTime: toFaroPerformanceTimingString(fetchStart - workerStart), decodedBodySize: toFaroPerformanceTimingString(decodedBodySize), encodedBodySize: toFaroPerformanceTimingString(encodedBodySize), cacheHitStatus: getCacheType(), renderBlockingStatus: toFaroPerformanceTimingString(rbs) as FaroResourceTiming['renderBlockingStatus'], protocol: nextHopProtocol, initiatorType: initiatorType, // TODO: add in future iteration, ideally after nested objects are supported by the collector. // serverTiming: resourceEntryRaw.serverTiming, }; ```Context
Faro docs say we are using this spec which include responseStatus https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus
It is confusion that we are missing this field