Question: Why does the API require the developer to manually close spans?

[tomwwright]

Context 🧠

I'm working to convert some TypeScript code from being instrumented with Datadog dd-trace to OpenTelemetry. I'm finding the tracing API not very friendly or straightforward because of the need to manually end() spans. I read through the discussion at open-telemetry/opentelemetry-js#1923 which highlighted that implementation in other languages (such as Ruby) don't have this issue.

With dd-trace, once things are initialised then I can easily manually instrument as follows:

tracer.trace(
    "event",
    (span) => {
      // do stuff
    }
  );

As part of that API I get this useful behaviour based on the execution of the wrapped callback:

The span will automatically be finished when one of these conditions is met:

• The function returns a promise, in which case the span will finish when the promise is resolved or rejected.
• The function takes a callback as its second parameter, in which case the span will finish when that callback is called.
• The function doesn't accept a callback and doesn't return a promise, in which case the span will finish at the end of the function execution.

Question ❓

OpenTelemetry is providing me with tracer.startActiveSpan but I don't understand why the responsibility is left with me to end the resulting span. For most cases, isn't it reasonable to assume that once the callback context ends I want the span to end? And if it throws to mark the span with the error? Otherwise why the callback-style API?

I'm finding myself writing a trace wrapper to make the API workable and apply boilerplate such as error handling to close spans as errors etc. and I don't know why I need to?

// something like follows
(tracer as any).trace = (
  name: string,
  fn: (span: opentelemetry.Span) => unknown
) => {
  return tracer.startActiveSpan(name, async (span) => {
    try {
      await Promise.resolve(fn(span));
      span.setStatus({
        code: opentelemetry.SpanStatusCode.OK,
      });
    } catch (e) {
      const err = e as Error;
      span.recordException(err);
      span.setStatus({
        code: opentelemetry.SpanStatusCode.ERROR,
        message: err.message,
      });
      throw err;
    } finally {
      span.end();
    }
  });
};

[tomwwright]

Code I ended up with to bake some expected behaviour into tracing calls

import * as opentelemetry from "@opentelemetry/api";

type TraceFunction<T = any> = (
  name: string,
  fn: (span: opentelemetry.Span) => T
) => Promise<T>;

type Tracer = opentelemetry.Tracer & {
  trace: TraceFunction;
};

export const tracer = opentelemetry.trace.getTracer("app") as Tracer;

/**
 * Utility function for wrapping OpenTelemetry tracer.startActiveSpan with
 * some sensible default behaviors to automatically end spans:
 * - when function completes successfully, status of span also set
 * - when function throws (exception recorded against span)
 * - if function returns Promise, promise is resolved
 * @param name Span name, passed through to tracer.startActiveSpan
 * @param fn Span callback function, passed through to tracer.startActiveSpan
 * @returns Return value that bubbles back out of of tracer.startActiveSpan, wrapped as a Promise
 */
export const makeTrace =
  (tracer: opentelemetry.Tracer) =>
  (name: string, fn: (span: opentelemetry.Span) => any) => {
    return tracer.startActiveSpan(name, async (span) => {
      try {
        const result = await Promise.resolve(fn(span));
        span.setStatus({
          code: opentelemetry.SpanStatusCode.OK,
        });
        return result;
      } catch (e) {
        const err = e as Error;
        span.recordException(err);
        span.setStatus({
          code: opentelemetry.SpanStatusCode.ERROR,
          message: err.message,
        });
        throw err;
      } finally {
        span.end();
      }
    });
  };

tracer.trace = makeTrace(tracer);

[vmarchaud]

To be honest this is question (is the api easily usable for end user use-cases) is poping up time and time again for different method around the tracing API. There is nothing wrong to make a wrapper to match your use-case (i did the same at my company).
I think the main reason it didn't get changed to be simpler is that we just initially matched the spec and tried to stick with it as much as possible, the goal being to not drift too much with other implementations.
However with time passing more users are using the API and making feedback about how it could be improved which is obvisouly something we want here.
I don't think anyone is against implement a similar util but i think it was preferable to wait for more usage before choosing a correct set of utils that cover all use cases. However I'm wondering if that should be asked at the spec level instead than here

[tomwwright]

Thanks for the reply @vmarchaud -- it's valuable to confirm I didn't miss something! Happy for this one to be closed unless there is value in using it to host further discussion on the topic

[dyladan]

I think there is value. I would prefer to see these APIs get implemented. I was originally the one that delayed these convenience APIs because there was some talk of having them specified for all languages. I didn't want to implement something like withSpan, then have the spec add a method with a similar name and different semantics and ours be drifted from theirs.

Now I think its pretty clear these convenience APIs are not going to be added to spec and we should be free to implement them as we choose. You can see #163 for an example of that, and I would welcome a mechanism to easily wrap an operation in a span like withSpan.

I think we can close this issue since the question is addressed and we can create new issues for proposed convenience APIs

[Flarna]

I still think that we should be careful in which utils we add in @opentelemetry/api and which utils can be in some other package.

@opentelemetry/api has it's own release cycle, development workflow and stability guarantees. Moving helper in other packages doesn't result in this "burden".

Besides that it allows to add more and even opinionated helpers as users can then choose to use API directly or choose one of the addon packages available in the wild.

This doesn't imply that we should not add simple helper here.