DEV Community

Cover image for What broke when I moved Grafana Faro to OpenTelemetry JS 3.0
Aditya Reddy
Aditya Reddy

Posted on

What broke when I moved Grafana Faro to OpenTelemetry JS 3.0

TL;DR: I opened the three PRs that move Grafana Faro's web SDK onto OpenTelemetry JS SDK 3.0. The version bump was the easy part. What took the time was a deep import the new exports maps reject, and then the tests. Jest was quietly running the Node build, and a disabled XHR instrumentation turned out to still be making spans. All three PRs are open and waiting on review.

The Faro epic on GitHub, Migrate to OpenTelemetry JS SDK 3.x, with the plan split into three steps

Why Faro had to move at all

OpenTelemetry JS SDK 3.0, experimental 0.300 and API 1.10 came out on Oct 20. They only work with each other, so you can't take one and leave the rest. Faro was on 2.x, 0.223 and API 1.9.

That sounds like something you can sit on for a while, and for the stable 2.x packages you can, they're supported until about Oct 2027. The problem showed up somewhere else. The contrib browser instrumentations (document load, user interaction, long task) had already moved to core ^3.0.0. Install one of those next to @grafana/faro-web-tracing and npm resolves @opentelemetry/api to 1.9.1, cause Faro asks for <1.10.0. Instrumentation 0.300 imports logs from the API, and 1.9.1 doesn't have it. So your app breaks on a package you didn't even touch.

The Faro team wrote this up as an epic with the plan. I picked up the steps.

Three PRs, stacked

I could have done it in one PR. It would have been one big diff touching about 30 files, and nobody wants to review that.

PR What it does Ships in
#2358 Moves tracing setup to @opentelemetry/sdk-trace any 2.x minor
#2363 Removes deep imports into OTel packages any 2.x minor
#2364 The actual 3.0 / 0.300 / 1.10 bump open decision

The first two change nothing for users and work on 2.x, so they can merge whenever. Only the last one raises what your build needs. Stacking them means the risky diff is small and the boring ones can go first.

WebTracerProvider is gone

SDK 3.x drops @opentelemetry/sdk-trace-web and @opentelemetry/sdk-trace-base. Everything Faro used from them is already in @opentelemetry/sdk-trace 2.12, which is why this step can land on 2.x.

The one real change is that WebTracerProvider.register() used to do several things for you. With the plain TracerProvider you do them yourself, in order:

-    provider.register({
-      propagator: new FaroPausablePropagator(/* ... */),
-      contextManager: options.contextManager,
-    });
+    trace.setGlobalTracerProvider(provider);
+    propagation.setGlobalPropagator(new FaroPausablePropagator(/* ... */));
+
+    const contextManager = options.contextManager ?? new StackContextManager();
+    contextManager.enable();
+    context.setGlobalContextManager(contextManager);
Enter fullscreen mode Exit fullscreen mode

The order matters more than it looks. The context manager has to be enabled before it's registered, and a second Faro instance must not register again over the first. I wrote tests for each of those paths, cause that's exactly the kind of thing that works on the happy path and breaks for the one app that passes its own propagator.

Side effect I didn't plan for: the tracing IIFE went from 90,474 to 87,299 bytes minified. Dropping a package you don't use is a nice way to lose 3 KB.

The exports map says no

This is the one that would have broken the upgrade if you just bumped versions.

All the 0.300 packages declare an exports map in package.json. Faro had imports straight into build/src of @opentelemetry/otlp-transformer, for the OTLP payload types and for createExportTraceServiceRequest and JSON_ENCODER. With an exports map, any subpath that isn't listed fails to resolve. It doesn't matter if the file is sitting right there.

Node fails with ERR_PACKAGE_PATH_NOT_EXPORTED and tsc fails with TS2307 when importing from otlp-transformer 0.300's build/src

Pointing the paths at the new dist/ layout doesn't help either. Unlisted is unlisted.

So I moved the OTLP types (IResourceSpans, IKeyValue, ESpanKind and the rest) into @grafana/faro-core, and swapped the exporter over to the public JsonTraceSerializer:

- import { JSON_ENCODER } from '@opentelemetry/otlp-transformer/build/src/common/utils';
- import { createExportTraceServiceRequest } from '@opentelemetry/otlp-transformer/build/src/trace/internal';
+ import { JsonTraceSerializer } from '@opentelemetry/otlp-transformer';

- const traceEvent = createExportTraceServiceRequest(spans, JSON_ENCODER);
+ const traceEvent: TraceEvent = JSON.parse(
+   new TextDecoder().decode(JsonTraceSerializer.serializeRequest(spans))
+ );
Enter fullscreen mode Exit fullscreen mode

That's one extra encode and parse per batch. I don't love it, though it's the public API in both 0.223 and 0.300 and the payload that goes out is the same. A snapshot test of the trace payload and the derived faro.tracing.* event now pins that down.

One detail I'd point out to anyone doing the same thing. I didn't copy ESpanKind as a TypeScript enum. TraceEvent is public through faro.api.pushTraces, and enums from two different declarations aren't assignable to each other. If Faro shipped its own enum, any app building payloads with otlp-transformer's enum would stop compiling. A const object with a numeric union type keeps both working.

This step also took @opentelemetry/otlp-transformer out of core and the OTLP transport, they only used its types. Without that, every Faro user would pick up 0.300's engines.node >=22.15.0, not just the ones using tracing.

The bump, and what it found

The version change itself is a few lines in package.json. core, resources and sdk-trace to ^3.0.0, the experimental packages to ^0.300.0, the API to ^1.10.0. Typecheck passed without a single change. That was honestly a bit suspicious.

Jest was running the Node build

Then Jest. Under Jest 30 with jsdom, OTel's packages resolve to their Node builds, not the browser ones. Nothing fails. You just test the wrong code. This is a one-line test that prints SDK_INFO from @opentelemetry/core 3.0, run once without the resolver and once with it:

The same jsdom test reports telemetry.sdk.language nodejs without the resolver and webjs with it, and passes both times

SDK_INFO reporting nodejs inside a browser test environment is the tell. The 3.x migration guide has a Jest resolver for this, and adding it flips it to webjs. If you have your own Jest setup with OTel 3.x and jsdom, check this before you trust your green tests.

A disabled instrumentation that kept tracing

Running the tests against the real XMLHttpRequestInstrumentation (the old ones mocked it) turned up a bug that wasn't about 3.0 at all. FaroXhrInstrumentation overrides _patchOpen, and the override didn't check _enabled like the parent does. disable() leaves open patched, so a disabled Faro XHR instrumentation kept creating spans.

The new XHR test fails on the #2363 tree with one span after disable(), and all three tests pass on #2364

It's a one-line check. It's also the kind of bug that lives for years cause the unit tests mock the exact thing that would catch it.

Size and tests

yarn build output for the tracing IIFE: 74.82 kB and 22.19 kB gzipped on #2363, 76.68 kB and 22.55 kB on #2364

The tracing bundle grows from 74.8 kB to 76.7 kB (22.1 to 22.5 kB gzipped) against #2363. The web-sdk and core bundles stay the same size, and there's no Node platform code in the browser bundle.

On the #2364 head the full suite passes, 1,218 unit tests across seven packages with one skipped.

yarn quality:test on #2364: every package passes, 204, 56, 82, 193, 646, 31 and 6 tests

What I checked outside the repo

Unit tests only tell you so much for a change like this, so I packed the packages and tried them in real apps:

  • A webpack 5.54 app and a Vite 7 app both build and run. Fetch and XHR spans get exported with telemetry.sdk.language set to webjs, and the faro.tracing.* events still go out.
  • The .d.ts files typecheck with both bundler and node16 module resolution.
  • On the other end, Alloy's faro.receiver uses the collector's ptrace.JSONUnmarshaler, so span attributes with object, array and bytes values still parse.

I also ran my own otel-js-upgrade codemod as a dry run on the #2363 tree. It proposed the same version bumps and no code changes. Which is the right answer, and kind of the point of this post: the parts that took time are the ones no codemod can see.

What's still open

The 3.0 PR has no ! in the title on purpose. Whether this ships as Faro v3 or a 2.x minor is still an open call in the epic. Faro's own API doesn't change, but the floor for your build does: webpack 5.54+, Jest 29.4+ or Parcel with packageExports, and Node 22.15+ for installs. Those are old versions to still be on, though a minor release breaking someone's build is still a minor release breaking someone's build.

There's one thing I left alone. sendFaroEvents turns attribute values into strings with String(), so array and object values come out as [object Object] in the derived events. Arrays already did that on 2.x, 3.x just lets objects through too. That's a small follow up if the maintainers want JSON there.

PR #2364 on GitHub, the OpenTelemetry JS SDK 3.0 upgrade, open and waiting on review

After this, the next step is #2340, replacing Faro's fetch and XHR instrumentations with the ones from the OpenTelemetry Browser SIG. That changes trace data, so it's a bigger conversation.

If you're on Faro today

Until the upgrade ships, if you add contrib browser instrumentations next to Faro, pin the last versions on 2.x:

npm i @opentelemetry/instrumentation-document-load@0.68.0 \
  @opentelemetry/instrumentation-user-interaction@0.67.0 \
  @opentelemetry/instrumentation-long-task@0.67.0
Enter fullscreen mode Exit fullscreen mode

That keeps @opentelemetry/api on one version and your app out of the logs import error.

Top comments (0)