OpenTelemetry: Improve Span Quality

[lforst]

Goals

1. Improve the quality of OpenTelemetry spans emitted by Next.js
2. Reduce noise in emitted spans

Non-Goals

1. We do not want to add additional instrumentation

Background

We are trying to adopt OpenTelemetry, however, there is a lot of potential to improve the data that is produced by Next.js telemetry. Currently, Next.js generates a lot of noise, and in some situations even wrong data.

What is the status quo?

• Next.js creates random spans like "NextServer.getRequestHandler" that cannot be influenced by users of the framework leading to noise.
• The top-level spans are not well named (i.e. "NextServer.getRequestHandler") making the resulting data hard to scan by humans. Ideally, the top-level spans are always something meaningful like GET /api/content.
• Next.js appends random suffixes to the descriptions of route spans like GET /api/content/route to requests to GET /api/content or GET /api/content/error when there was an error during the request.
• Span durations for requests on the pages router are completely off, since they are terminated when the Next.js request handlers resolve, and not when res.end() is called.

Proposal

I propose the following. Of course, this is up for discussion 😄:

• All top-level (root) spans that Next.js emits for incoming requests should be a semantic HTTP span with a low-cardinality description (ie. parameterized route)
• Descriptions of semantic HTTP request spans should not receive suffixes like /route or /error.
• HTTP spans should span the request from when it was received, until when res.end() was called.
• Next.js should not emit any spans that cannot, directly or indirectly, be influenced by users of the framework
• Next.js should either commit to instrumenting native request APIs or leave it out entirely. Currently, the fetch API is instrumented while http.request() or http.get() aren't.
• Edge runtime API handlers should be semantic otel spans

[dvoytenko]

Mostly agree with everything here. My recommendation:

1. Separate verbose vs non-verbose
2. Clean up non-verbose mode
3. Completely redo (or even remove) verbose mode

[dcramer]

Adding my two cents that any span that looks like this is effectively useless and confusing to me:

[dvoytenko]

Curious. I generally agree. Although I usually see these spans longer. Probably this is the issue of when the modules have already been loaded. IMHO, they shouldn't be included in that case, since no real work is being done.

The "start response" below is definitely not useful, although it'd be good to see the TTFB-like info presented.

[dcramer]

In this particular case I think part of the problem is the information is hidden in the next attributes, rather than actually propagated up as core otel attributes (e.g. the operation name).

It still doesnt mean anything to me, but I had to cycle between these a few times to even realize that next.segment changed.

[dvoytenko]

Is there a core otel attribute for the operation name?

[dcramer]

Going to ask someone else to chime in as maybe otel doesnt provide as clean of a seperation as Sentry does (we have our own nomenclature thats not quite a 1:1 overlap unfortunately). I tried to dig into the docs but I only saw name, which I'm guessing is what you're leveraging here.

[dvoytenko]

Unfortunately, yes, I couldn't find clear standard names for these. I know that Sentry and DataDog use some conventions. But I don't even know if they agree on the meaning of them. Certainly, though, if there's a good naming to fallback to, we can make changes.

[morley92]

All top-level (root) spans that Next.js emits for incoming requests should be a semantic HTTP span with a low-cardinality description (ie. parameterized route)

I was about to raise a separate discussion for this exact point! There is a small list of semantic attributes added by NextJS.

I'm really missing user_agent.original at the moment, feels like this should come out of the box?

[morley92]

Raised a PR to add this - #68339

[stephenliang]

In case anybody was trying to figure out why NextServer.getRequestHandler is showing up as the root span, this span is supposed to be filtered out in the allow list, but if the environment variable NEXT_OTEL_VERBOSE=1 then the allow list is not used and the span becomes the root one.