Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[instrumentation-fetch] Add a requestHook to @opentelemetry/instrumentation-fetch #5084

Open
patricklafrance opened this issue Oct 21, 2024 · 4 comments

Comments

@patricklafrance
Copy link

patricklafrance commented Oct 21, 2024

Is your feature request related to a problem? Please describe.

I am trying to nest some HTTP GET spans created by @opentelemetry/instrumentation-fetch automated instrumentation under a span created manually.

The resulting trace should be something like this:

image

In the above screenshot, the HTTP GET spans created by @opentelemetry/instrumentation-fetch automated instrumentation are nested under the manually created data-fetch span.

I haven't found an elegant way to achieve this result using the official API. According to the documentation and the messages in this issue, spans should be nested under a parent using either the tracer.startActiveSpan function:

await tracer.startActiveSpan("MyName", async (span) => {
    try {
      const fooResult = await foo(); // this or some inner function my create child spans
      span.setAttribute("fooResult", fooResult);
      const barResult = await bar(); // this or some inner function my create child spans
      span.setAttribute("barResult", barResult);
      // ...
    } catch (e) {
      span.recordException(e);
    } finally {
      span.end();
    }
});

or using the context.with function:

tracer.startActiveSpan("root", (rootSpan) => {
    const s1 = tracer.startSpan("s1");
    const ctx1 = api.trace.setSpan(api.ROOT_CONTEXT, s1);

    api.context.with(ctx1, () => {
        tracer.startSpan("s1.1").end();
    });

    s1.end();
    rootSpan.end();
});

Since my goal is to nest spans that are created by @opentelemetry/instrumentation-fetch, I don't think that I can use either tracer.startActiveSpan or context.with.

Another specificity of my current issue is that the manual spans are created by listening to events. For example, the data-fetch span is created when a listener receives a data-fetching-started event:

let span: Span;

eventBus.addListener(DataFetchStartedEvent, () => {
    span = tracer.startSpan("data-fetch");
});

eventBus.addListener(DataReadyEvent, () => {
    span?.end();
});

I tried to hack my way around this with a solution similar to the one described in this reply by providing a applyCustomAttributesOnSpan function to @opentelemetry/instrumentation-fetch:

new HoneycombWebSDK({
     instrumentations: [getWebAutoInstrumentations({
          "@opentelemetry/instrumentation-fetch": {
               applyCustomAttributesOnSpan: applyCustomAttributesOnFetchSpanFunction
          }
    })]
});

The main difference is that instead of adding a custom attribute like behavior.trace_id, I am overriding the trace.trace_id and trace.parent_id attributes.

It kind of works, this is how I have been able to capture the following screenshot:

image

BUT, then I found out that it break distributed tracing.... The reason is that the applyCustomAttributesOnSpan function is executed AFTER the request has been executed. Therefore, the request traceparent header is using the original trace.trace_id and trace.parent_id attributes instead of the overrides I am providing.

As a result, the trace of the server are not bound to any existing span:

image

Then, I tried to define my own ContextManager to manually override the active context, something similar to the solution described in this reply.

Doesn't work either, maybe I could get it working but there are MANY edge cases to handle AND it affect ALL the spans. I ended up with traces like the following:

image

I would prefer a solution that is scoped to spans created by @opentelemetry/instrumentation-fetch.

Describe the solution you'd like

Ideally, a requestHook would be added to @opentelemetry/instrumentation-fetch. Something similar to the requestHook of instrumentation-http.

This hook would allow an application to provide custom attributes BEFORE the request is executed. For my specific case, it would allow me to provide the trace.trace_id and trace.parent_id attributes before the traceparent header is added to the request.

Describe alternatives you've considered

I tried the 3 solutions explained in the first section of this issue:

  1. Using tracer.startActiveSpan and context.with APIs
  2. Using a global variable to share the "active span" as described here
  3. Defining a custom ContextManager as described here

Additional context

N/A

@JamieDanielson
Copy link
Member

Hi @patricklafrance . I am having trouble picturing exactly what you are trying to achieve that is not possible today. The fetch example app in the repo includes the usage of a ZoneContextManager which is needed for managing context, and has an example of nesting an automatic span from fetch instrumentation. This sounds like what you are describing and maybe helps get what you are looking for.

If I am misunderstanding your question though, could you please provide a small reproducer, including what you expect to happen and what actually happens?

@patricklafrance
Copy link
Author

patricklafrance commented Nov 8, 2024

Hey @JamieDanielson!

Thanks for the reply. Indeed it looks similar. I am not familiar with zone.js though.

One key difference with the example you provided is that in my case the fetch request is not triggered in the span scope. I am not sure if it matters?

Otherwise, yeah maybe it could work. That being said, the ZoneContextManager code is not trivial to adapt to other use cases. It requires deep knowledge about how OpenTelemetry context works.

I would argue that using a global variable to share the "active span" with a requestHook is more trivial to implement and maintain.

@patricklafrance
Copy link
Author

I can provide more specific information.

For the following trace:

image

The custom data-fetch span is created here in a library function when a specific event is dispatched by a React hook.

Then, a web application using our library would execute fetch requests using React Query once the data-fetch span is created. In the same repository, we have a sample application using the referenced library that showcase those requests.

Once those requests are done, another event is dispatched, and catch by our library function, to end the data-fetch span.

We expect the fetch requests to be nested under our data-fetch span but there's no direct nesting / scoping between the creation of the span and those fetch requests.

image

I am not sure if those additional information help or are confusing hehe. Let me know.

@patricklafrance
Copy link
Author

One last thing, correct me if I am wrong.. but I think using a solution similar to the ZoneContextManager would nest ANY spans created by automated instrumentation right? Instead of only nesting fetch span like HTTP GET?

Resulting in a trace similar to this one with nested documentLoad and documentFetch spans 👇🏻

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants