[Discussion] Requiring PerformanceObserver in the next release?

[chancancode]

Currently, the fetch instrumentation (presumably the XHR one, and others – I didn't check exhaustively) tries to handle the case where PerformanceObserver is not available.

I'm wondering if there are any appetite on discontinuing support for environments without PerformanceObserver, or at least severely degrade the functionality without it. This would probably have to involve specifying the minimum inclusion of PerformanceObserver.supportedEntryTypes – for fetch this would be resource, but other instrumentation may want to enforce other types.

Support

• caniuse.com
• MDN

As far as browsers and Node.js is concerned, support have landed for ages. I think it mostly comes down to whether this is to require this when some of those "non-traditional" environments – Deno, Bun, cloudflare workers, and all of those vendor-specific custom runtime environments. Most of those aren't officially supported right now anyway, but if we do this, it can be an additional barrier to landing support for those environments.

Motivation

I'll speak to the specific case of the fetch instrumentation here, but my intention is to make this a general issue and for maintainers of other instrumentations to chime in and see how this could affect/benefit their use cases.

First, this configuration, along way the associated alternate code paths, can go away.

More importantly: we currently have essentially a spin-loop that reads/drains a clone of the response body, just to make sure we can include the time it takes to stream the response body into account for the span's timing. This is an adjacent issue to #5281 and exhibit some similar problems. It was brought up in the last SIG meeting as something potentially problematic that we'd like to change.

According to the spec:

HTTP client spans SHOULD end sometime after the HTTP response headers are fully read (or when they fail to be read). This may or may not include reading the response body.

So we can actually stop doing that from a spec-compliance perspective and always end the span when the fetch promise resolves. That being said, I think that would also severely limit the usefulness of the instrumentation in the real world – I suspect most real world consumers/use cases would really like to have that portion included in the span timing (though we should at least make it configurable with a hook that can decide based on the specific request – it's also perfectly reasonable that you don't want this for streaming responses).

That information is actually available in the resource timing entries without doing that problematic spin loop, we just need a way to reliably get that timing information from the performance entries, and PerformanceObserver gives us that. It's also a more performant/less resource intensive way to do things in general, as the browsers specifically designed this API for these use cases in mind.

If we can require PerformanceObserver (and the "resource" entry type), I can envision a refactor of the fetch instrumentation to be more fundamentally based around that, rather than hooking in as an optional afterthought.

Possible Compromises

Technically, this the fetch instrumentation is only for browsers, so the considerations of unconventional environments like Deno doesn't really apply. It's also an experimental package. But then again, my intention is to open up this issue more widely to gauge interest in other instrumentations, potentially tying it to the 2.0 major release for the stable packages.

I can also envision a refactor of the network related instrumentations (fetch, xhr, node-fetch) to share more of a common core, as we are reimplementing a lot of things right now and they behave pretty inconsistently/with pretty different configurations, when they are all instrumenting something really similar.

A possible path is to make this a soft requirement, in that the instrumentation won't blow up (throw) without it, but may degrade in functionality in environments without it – certain features may not be supported, omitting certain spans, all the way to silently doing nothing at all.

In the case of the fetch instrumentation, I can see we limiting the features that takes into account of the response body timing, as well as the "network events", to environments with PerformanceObserver + "resource" type only. Using it in environments without that will just give you basic spans we get form timing the fetch() call itself, which will only account for time up to where the response headers are fully read.