WithSpan et WithScope decorators

[jeengbe]

Problem Statement

It is a lot more convenient to attach decorators to class methods than to wrap their contents in a callback and bump up nesting by one or two.

class ThingDoer {
  doThing() {
    Sentry.withScope((scope) => {
      Sentry.startSpan(
        {
          name: 'ThingDoer.doThing',
        },
        () => {
          scope.setExtra('extra', 'extra');

          console.log('doing thing');
        },
      );
    });
  }
}

vs.

class ThingDoer {
  @Sentry.WithScope()
  @Sentry.WithSpan()
  doThing() {
    const scope = Sentry.getCurrentScope();
    scope.setExtra('extra', 'extra');

    console.log('doing thing');
  }

---

Also, should Sentry.startSpan() not be named Sentry.withSpan()?

Solution Brainstorm

Rudimentary span implementation:

export function WithSpan(
  spanConfig?: SentryTypes.StartSpanOptions,
): MethodDecorator {
  // @ts-expect-error -- Not sure what the error is
  return function <T extends (...args: readonly unknown[]) => Promise<unknown>>(
    target: object,
    propertyKey: string | symbol,
    descriptor: TypedPropertyDescriptor<T>,
  ) {
    const originalMethod = descriptor.value;

    // istanbul ignore next
    if (!originalMethod) return;

    const config: SentryTypes.StartSpanOptions = spanConfig ?? {
      name: target.constructor.name + '.' + propertyKey.toString(),
      op: 'method',
    };

    // @ts-expect-error -- Not sure what the error is
    descriptor.value = function (...args: unknown[]): Promise<unknown> {
      return Sentry.startSpan(config, () => originalMethod.apply(this, args));
    };
  };
}

[Lms24]

Hey @jeengbe thx for the suggestions!

It is a lot more convenient to attach decorators to class methods

So technically yes, this would be a nice feature. However, we didn't add decorators yet because decorators are not used widely and there are multiple different experimental stages (TS experimentalDecorators vs JS decorators; see TC39 proposal). Also Node doesn't support them without transpilation. This makes it hard to add a feature to the SDK that is supposed to support a variety of older ES/Node versions. The exception to the rule is our Angular SDK
where we can already use decorators as the Angular framework supports them well (with transpilation under the hood but in this case it's safe).

If decorators work fine in your setup, I suggest to write them yourself. Might even be a nice idea to publish them. Maybe in a distant future, we can add such decorators to the SDK.

Also, should Sentry.startSpan() not be named Sentry.withSpan()?

This is unfortunately a bit of a rabbit hole for us but we decided against withSpan to more closely match OpenTelemetry APIs. We're not fully there (startSpan vs. startActiveSpan) but that's the direction we want to move towards. This was discussed and decided in getsentry/rfcs#101 and getsentry/rfcs#113.

I'm going to close this issue for the time being because I don't think we can actionably incorporate these suggestions at the moment. Feel free to ping me if you think this should be reopened. Thanks!

[jeengbe]

(TS experimentalDecorators vs JS decorators; see TC39 proposal)

Interesting to see the proposal differ from TS' implementation. I was not aware.
I disagree with the other arguments about runtimes not supporting decorators, as, from the lib's perspective, decorators are merely functions. A library may provide it regardless of whether the user is capable of consuming that, is my stance on that.

Regardless, a missing standard is a counter-argument strong enough.