[otlp-exporter] refactor of sending requests on web

[llc1123]

• This only affects the JavaScript OpenTelemetry library

Currently, sendBeacon is used when 1. no headers are configured and 2. the browser supports. xhr is used otherwise.

According to the spec, a user-agent header should be present in all requests.

My suggestion:

• Always try to carry the user-agent header.
• If there are other headers, always use fetch.
• Use fetch with keepalive=true as the first option.
• Use sendBeacon if keepalive is not supported. (Drop user-agent)
• The fetch polyfill doesn't implement keepalive, so it is safe if such polyfill is applied.
• Remove xhr because it is too old and everyone should use polyfills if they want to support legacy browsers.

Update:

Only the latest browsers support custom user agent. So no need to fallback from sendBeacon to fetch.

const browserSupportsKeepalive = 'keepalive' in new Request('')
const headers = {'user-agent': 'otlp-exporter'}
if (!browserSupportsKeepalive && Object.keys(headers).length === 1) {
  sendBeacon()
} else {  
  fetch(url, {keepalive: true, headers})
}

cc @MSNev @dyladan @pichlermarc

[MSNev]

I'd change the order as fetch with keepalive also has the same 64kb limit as sendBeacon, unless your in a browser environment and you have detected that the page is unloading / potentially being hibernated (visibilitychange state to hide)

So my preferred order would be (if available)

• XHR
• fetch
• fetch (with keepalive)
• sendBeacon

There is NOTHING wrong with XHR and XHR is supported in all browser environments, so I don't agree with

Remove xhr because it is too old and everyone should use polyfills if they want to support legacy browsers.

[llc1123]

There is NOTHING wrong with XHR

It increases code complexity and bundle sizes. Modern libraries (there are too many out there) use fetch directly and would ask users to use polyfills (or a custom fetcher passed to the constructor) if needed.

and you have detected that the page is unloading / potentially being hibernated

That is not easy to implement and I think is unnecessary because:

• It is hard to determine when to use which one.
• The 64K limit is almost impossible to reach for telemetry use case.
• The current implementation is using sendBeacon as the first option if no headers configured.

[MSNev]

That is not easy to implement and I think is unnecessary because:

No it's not and it is necessary, I do this ALL of the time as part of Microsoft ApplicationInsights (and it's internal variant) to make sure that all events submitted are flushed. -- it is a MUST.

The 64K limit is almost impossible to reach for telemetry use case.

For today with the basic telemetry that is sent by OTel and your use case, (maybe) but when applications start adding their own content or batch multiple events this happens ALL of the time.

The current implementation is using sendBeacon as the first option if no headers configured.

Yes, this will be address as part of #3062, as it should not be.

[llc1123]

I still don't see the point of not using a polyfill here.

[MSNev]

We should not "require" someone to use a polyfill, it should work out of the box.

https://caniuse.com/mdn-api_xmlhttprequest
https://caniuse.com/?search=fetch

[eligao]

https://caniuse.com/fetch

Fetch is supported in all mainstream browsers since 5 years ago. I don't see any benefit to stick to manually checking fetch then fallback to XHR, that is exacy what polyfills do.
In practice, anyone serious enough wants to support legacy browsers in production should have polyfills already.
In the worst case, we can still give some warning logs with suggestions to add missing fetch, it's not perfect but still more elegant than redoing what polyfills already do.

it should work out of the box.

You're right, but not without preconditions. It's time to let go of XHR.

[dyladan]

So my preferred order would be (if available)

• XHR
• fetch
• fetch (with keepalive)
• sendBeacon

Can you elaborate on when each of the items in the proposed list would be used? Looks to me like XHR would always be selected if this is the priority because it's supported essentially everywhere.

---

As far as the polyfill question goes, in the past we've always leaned towards supporting old runtimes where the effort disadvantage is low, or the runtime in question has significant users. We have never really defined the bar for either of these criteria.

We should not "require" someone to use a polyfill, it should work out of the box.

https://caniuse.com/mdn-api_xmlhttprequest
https://caniuse.com/?search=fetch

From this it looks like using fetch by default drops support for opera mini and IE, together representing about 1.5% of browser users. For some reason caniuse shows "unknown" support for XHR on a few browsers including opera mini and UC browser for Android? Even if we assume those browsers do support XHR, I would question how many developers meet all of the following criteria:

• Want to use OpenTelemetry
• Require support for opera mini or IE
• Can not or will not install a fetch polyfill

To be clear, 1.5% of browser users is a lot of people when you consider that the class of "browser users" is billions of people, but is it reasonable to expect that developers targeting them will use polyfills?

At this point I don't think I have made up my own mind either way.

One more note is that dropping XHR may be considered a breaking change. I say this not because we can't do it, but because if we don't drop support for XHR before 1.0 we will be forced to support it until at least 2.0.

[llc1123]

I would question how many developers meet all of the following criteria:

I would like to say that number is definitely ZERO.

No one can build a web app that runs on such browsers without polyfills.

Let's say we keep the XHR code here. Okay, the user don't need a fetch polyfill. What about Object.assign? What about many other JavaScript features that IE doesn't have native support?

In most web projects, you just cannot avoid using polyfills to work on legacy browsers. Libraries usually don't have such compatibility. Even if we support them, other libraries don't.

Do we really need to keep our code compatible with these legacy browsers just for "work out of box", which actually doesn't work at all? Especially when we are also actively dropping support for deprecated libraries and EOL Node runtimes on the Node side.

One more note is that dropping XHR may be considered a breaking change. I say this not because we can't do it, but because if we don't drop support for XHR before 1.0 we will be forced to support it until at least 2.0.

The OLTP exporters are still experimental. I don't think we need to wait for 2.0 to do the breaking changes. It doesn't work on legacy browsers without polyfills anyway.

As far as the polyfill question goes, in the past we've always leaned towards supporting old runtimes where the effort disadvantage is low, or the runtime in question has significant users.

The point is: Just drop support for legacy APIs. Polyfills exist for a reason.

To my use case, I am using ES Modules and import maps. Which is only natively supported on the latest browsers. But there always are solutions for supporting legacy browsers. It is meaningless taking care of the compatibility issues on the library side.

[dyladan]

The OLTP exporters are still experimental. I don't think we need to wait for 2.0 to do the breaking changes. It doesn't work on legacy browsers without polyfills anyway.

Yes I know they're still experimental. That's why I said now is the time to do this (before 1.0) if we're going to.

[MSNev]

Sorry, I thought I responded to this already, but looks like I failed to commit the comment.

Can you elaborate on when each of the items in the proposed list would be used? Looks to me like XHR would always be selected if this is the priority because it's supported essentially everywhere.

Normal events (before unload detected)

• XHR - Browser always
• Fetch - Node (20 or with polyfill)

After navigation away (unload) detected

• fetch (with keeplive), if supported always
• sendBeacon (if supported)
• (unlikely but final fallback) - synchronous XHR (this would ONLY occur on an older runtime)

Once the options are available there SHOULD also be a configuration to allow the application initializing the SDK to choose which API they should use.

For completeness there SHOULD also be http support (for node) so that we create a single exporter which can support all environments rather than multiple exporters)... But that might be another discussion.

[MSNev]

Let's say we keep the XHR code here. Okay, the user don't need a fetch polyfill. What about Object.assign? What about many other JavaScript features that IE doesn't have native support?

There are "lots" and as part of ApplicationInsights I've been dealing with all of these without requiring the usage of babel. This is also part of another discussion around defining what the browser matrix is or should be.

There are different ways to solve this problem, one of which (that I use) is to use a library that also addresses minification as part of the same solution like using this library which provides "aliases" for native functions to avoid Object.xxxxx where the alias can be minified and this one also supports simplified polyfills for the most used features that are not supported on older runtimes, this library only supports a minimum of ES5 (which is the minimum OTel should support (ie. we should NOT attempt to support ES3))

And to more fully answer your question, for ApplicationInsights we have a packaging level final check which stops use from using API's that are not fully / commonly supported for ES5 for the latest release.

And for the early (currently most used) release where we support ES3, where we use a combination of tsconfig.json target as es3 (so that TypeScript stops use using some API's (like Promose) and then we block these during packaging and finally we also polyfill on the fly during packaging (see the tokens with replace definitions)

[llc1123]

This is a community-maintained open-source project. It should follow the standard of most OSS libs, not the "Microsoft-preferred" standard.

As I already said before, there are many solutions to support legacy browsers, and keeping these legacy codes in the library is not a good idea. What is the point of "work out of box" when other libs used in the project don't support legacy platforms?

There are different ways to solve this problem, one of which (that I use) is to use a library that also addresses minification as part of the same solution like using this library which provides "aliases" for native functions to avoid Object.xxxxx where the alias can be minified and this one also supports simplified polyfills for the most used features that are not supported on older runtimes, this library only supports a minimum of ES5 (which is the minimum OTel should support (ie. we should NOT attempt to support ES3))

This IS a polyfill. So why not for fetch?

[github-actions]

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

[github-actions]

This issue was closed because it has been stale for 14 days with no activity.

[myieye]

We tend to lose traces due to the size constraints of the Beacon API. Perhaps we could tweak the export parameters better, but it seems to me that it really would be worth it to refactor these requests and make them more fail-safe.

I'd love to see these get reopened.

[pichlermarc]

We tend to lose traces due to the size constraints of the Beacon API. Perhaps we could tweak the export parameters better, but it seems to me that it really would be worth it to refactor these requests and make them more fail-safe.

I'd love to see these get reopened.

@myieye I am working on #4116; it will take a while because we have 9 OTLP exporters for Node.js and 6 for browsers (15 total) and that's a lot of work as it basically amounts to a rewrite.

In the long term, we'll be able to let people choose what they want to use to use for exporting. The first step, though will be to refactor them with feature-parity the existing ones. In the meantime, I'd like to ask people to please refrain from opening any exporter-related PRs that are not strict bug fixes to avoid duplicate work.