Repository navigation
WithSpan et WithScope decorators #11353
Description
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));
};
};
}
Activity
Lms24 commentedon Apr 2, 2024
Hey @jeengbe thx for the suggestions!
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
experimentalDecoratorsvs 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 SDKwhere 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.
This is unfortunately a bit of a rabbit hole for us but we decided against
withSpanto 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 commentedon Apr 2, 2024
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.