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

Provide typescript decorator to auto instrument functions #3827

Closed
pksunkara opened this issue May 20, 2023 · 8 comments
Closed

Provide typescript decorator to auto instrument functions #3827

pksunkara opened this issue May 20, 2023 · 8 comments

Comments

@pksunkara
Copy link

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

Right now, for the end-user to trace their functions call, they have to write code to surround the respective function body in an active span, and then end the span at the end of the body.

This is very cumbersome and adds a lot of noise to the code.

Describe the solution you'd like

Provide a typescript decorator that does this automatically for functions. Inspired by the instrument macro from Rust.

It's possible to add support for this using decorators.

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context or screenshots about the feature request here.

@pksunkara
Copy link
Author

@pichlermarc I am planning to work on this by creating a @opentelemetry/decorators package. This improves the dev ergonomics of OTLP.

Do I have the go-ahead?

@clouedoc
Copy link

as an end-user, I would love this!

For reference, here is something that I have done for dd-trace:

import tracer, { Span } from 'dd-trace';

tracer.init(); // initialized in a different file to avoid hoisting.

/**
 * A decorator that enables Datadog tracing on a method.
 * @param method
 */
export function telemetryTraceMethod() {
  return function (
    target: any,
    methodName: string,
    descriptor: PropertyDescriptor,
  ) {
    const className = target.constructor.name; // correct type: Function
    const originalMethod = descriptor.value;

    descriptor.value = function (...args: any[]) {
      return tracer.trace(
        `${className}.${methodName}`,
        { resource: className },
        () => {
          return originalMethod.apply(this, args);
        },
      );
    };
  };
}

/**
 * Get root span from the current active scope
 * @returns
 */
export function getRootSpan(): Span | undefined {
  return (
    (tracer.scope().active()?.context() as any)._trace?.started[0] ?? undefined
  );
}

export { tracer };

@clouedoc
Copy link

clouedoc commented Jun 27, 2023

I have tried to adapt it for OpenTelemetry but couldn't get something that passes tests:

// filename: tracing-decorator.ts
/* eslint-disable @typescript-eslint/ban-types */ // needed for Function type
import opentelemetry from '@opentelemetry/api';

type OpenTelemetryTracer = ReturnType<typeof opentelemetry.trace.getTracer>;

/**
 * A decorator that enables Datadog tracing on a method.
 * @param method
 */
export function telemetryTraceMethod(
  tracer: OpenTelemetryTracer,
): (
  target: Function,
  methodName: string,
  descriptor: PropertyDescriptor,
) => void {
  return function (
    target: Function,
    propertyKey: string,
    descriptor: PropertyDescriptor,
  ) {
    const className: string = target.constructor.name; // correct type: Function
    // eslint-disable-next-line @typescript-eslint/ban-types
    const originalMethod: Function = descriptor.value;

    descriptor.value = function (...args: unknown[]) {
      return tracer.startActiveSpan(
        `${className}.${propertyKey}`,
        {},
        async (span) => {
          try {
            return await originalMethod.apply(this, args);
          } catch (err) {
            span.recordException(err as Error);
            throw err;
          } finally {
            span.end();
          }
        },
      );
    };
  };
// filename: tracing-decorator.spec.ts
// Mock the OpenTelemetry API
jest.mock('@opentelemetry/api', () => ({
  trace: {
    getTracer: jest.fn().mockReturnValue({
      // eslint-disable-next-line @typescript-eslint/naming-convention
      startActiveSpan: jest.fn((_, __, callback) =>
        callback({
          end: jest.fn(),
          recordException: jest.fn(),
        }),
      ),
    }),
  },
}));

import { trace } from '@opentelemetry/api';
import { telemetryTraceMethod } from './tracing-decorator'; // Adjust as per your file structure

const testTracer = trace.getTracer('test');

describe('telemetryTraceMethod', () => {
  it('should decorate a method with telemetry tracing', async () => {
    class TestClass {
      @telemetryTraceMethod(testTracer)
      public async testMethod(arg1: number, arg2: number): Promise<number> {
        return arg1 + arg2;
      }
    }

    const instance = new TestClass();
    const result = await instance.testMethod(1, 2);
    expect(result).toBe(3);

    const tracer = trace.getTracer('test');
    expect(tracer.startActiveSpan).toHaveBeenCalled();
  });

  it('should record an exception if the decorated method throws an error', async () => {
    class TestClass {
      @telemetryTraceMethod(testTracer)
      public async testMethod(): Promise<void> {
        throw new Error('Test error');
      }
    }

    const instance = new TestClass();
    await expect(instance.testMethod()).rejects.toThrow('Test error');

    const span = await jest.mocked(testTracer.startActiveSpan).mock.results[0]
      .value;
    expect(jest.mocked(span.recordException)).toHaveBeenCalled();
  });
});

@pichlermarc
Copy link
Member

@pichlermarc I am planning to work on this by creating a @opentelemetry/decorators package. This improves the dev ergonomics of OTLP.

Do I have the go-ahead?

I can see how this would be used. It would add another dependency though that could see significant use across user code unrelated to setup - everything like that usually lives in @opentelemetry/api, so a decorators module like that would be unique in that sense and has also no precedent that I know of. 🤔

I'll bring this up during the SIG meeting later today and bring the summary of what we discussed back to this issue. 🙂

@pichlermarc
Copy link
Member

@pichlermarc I am planning to work on this by creating a @opentelemetry/decorators package. This improves the dev ergonomics of OTLP.
Do I have the go-ahead?

I can see how this would be used. It would add another dependency though that could see significant use across user code unrelated to setup - everything like that usually lives in @opentelemetry/api, so a decorators module like that would be unique in that sense and has also no precedent that I know of. thinking

I'll bring this up during the SIG meeting later today and bring the summary of what we discussed back to this issue. slightly_smiling_face

Brought it up during the SIG meeting and we discussed using an entry point in @opentelemetry/api to have this kind of functionality and to avoid introducing a new module.

So ideally it would be in @opentelemetry/api/experimental/decorators at first to ensure that people know that there is no stability guarantee for it at first. Once we're happy with the way they work and we're confident that they're stable, we'd move them to @opentelemetry/api/decorators.

So if that works for you you have the go-ahead @pksunkara 🙂

@pksunkara
Copy link
Author

I started work on this, but it looks like typescript doesn't support function decorators.

Ref: https://stackoverflow.com/questions/37518913/typescript-decorator-for-function-not-method-possible

Therefore, I am going to pause working on this because it doesn't suit my needs. I will probably go with supporting something like #3250

@github-actions
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Sep 25, 2023
Copy link

This issue was closed because it has been stale for 14 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 15, 2024
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