Skip to content

fix(tracer): use instance as this in capture decorators #1055

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

Merged
merged 7 commits into from
Aug 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions docs/core/tracer.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,11 @@ You can quickly start by importing the `Tracer` class, initialize it outside the
}
export const handlerClass = new Lambda();
export const handler = handlerClass.handler;
export const handler = handlerClass.handler.bind(handlerClass); // (1)
```

1. Binding your handler method allows your handler to access `this`.

=== "Manual"

```typescript hl_lines="6 8-9 12-13 19 22 26 28"
Expand Down Expand Up @@ -253,8 +256,11 @@ You can trace other Class methods using the `captureMethod` decorator or any arb
}
export const myFunction = new Lambda();
export const handler = myFunction.handler;
export const handler = myFunction.handler.bind(myFunction); // (1)
```

1. Binding your handler method allows your handler to access `this`.

=== "Manual"

```typescript hl_lines="6 8-9 15 18 23 25"
Expand Down
55 changes: 33 additions & 22 deletions packages/tracer/src/Tracer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Handler } from 'aws-lambda';
import { Utility } from '@aws-lambda-powertools/commons';
import { AsyncHandler, SyncHandler, Utility } from '@aws-lambda-powertools/commons';
import { TracerInterface } from '.';
import { ConfigServiceInterface, EnvironmentVariablesService } from './config';
import { HandlerMethodDecorator, TracerOptions, MethodDecorator } from './types';
Expand Down Expand Up @@ -67,7 +67,7 @@ import { Segment, Subsegment } from 'aws-xray-sdk-core';
* }
*
* export const handlerClass = new Lambda();
* export const handler = handlerClass.handler;
* export const handler = handlerClass.handler.bind(handlerClass);
* ```
*
* ### Functions usage with manual instrumentation
Expand Down Expand Up @@ -334,33 +334,40 @@ class Tracer extends Utility implements TracerInterface {
* }
*
* export const handlerClass = new Lambda();
* export const handler = handlerClass.handler;
* export const handler = handlerClass.handler.bind(handlerClass);
* ```
*
* @decorator Class
*/
public captureLambdaHandler(): HandlerMethodDecorator {
return (target, _propertyKey, descriptor) => {
return (_target, _propertyKey, descriptor) => {
/**
* The descriptor.value is the method this decorator decorates, it cannot be undefined.
*/
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const originalMethod = descriptor.value!;

descriptor.value = ((event, context, callback) => {
if (!this.isTracingEnabled()) {
return originalMethod.apply(target, [ event, context, callback ]);
// eslint-disable-next-line @typescript-eslint/no-this-alias
const tracerRef = this;
// Use a function() {} instead of an () => {} arrow function so that we can
// access `myClass` as `this` in a decorated `myClass.myMethod()`.
descriptor.value = (function (this: Handler, event, context, callback) {
// eslint-disable-next-line @typescript-eslint/no-this-alias
const handlerRef: Handler = this;

if (!tracerRef.isTracingEnabled()) {
return originalMethod.apply(handlerRef, [ event, context, callback ]);
}

return this.provider.captureAsyncFunc(`## ${process.env._HANDLER}`, async subsegment => {
this.annotateColdStart();
this.addServiceNameAnnotation();
return tracerRef.provider.captureAsyncFunc(`## ${process.env._HANDLER}`, async subsegment => {
tracerRef.annotateColdStart();
tracerRef.addServiceNameAnnotation();
let result: unknown;
try {
result = await originalMethod.apply(target, [ event, context, callback ]);
this.addResponseAsMetadata(result, process.env._HANDLER);
result = await originalMethod.apply(handlerRef, [ event, context, callback ]);
tracerRef.addResponseAsMetadata(result, process.env._HANDLER);
} catch (error) {
this.addErrorAsMetadata(error as Error);
tracerRef.addErrorAsMetadata(error as Error);
throw error;
} finally {
subsegment?.close();
Expand All @@ -369,7 +376,7 @@ class Tracer extends Utility implements TracerInterface {

return result;
});
}) as Handler;
}) as SyncHandler<Handler> | AsyncHandler<Handler>;

return descriptor;
};
Expand Down Expand Up @@ -411,23 +418,27 @@ class Tracer extends Utility implements TracerInterface {
* @decorator Class
*/
public captureMethod(): MethodDecorator {
return (target, _propertyKey, descriptor) => {
return (_target, _propertyKey, descriptor) => {
// The descriptor.value is the method this decorator decorates, it cannot be undefined.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const originalMethod = descriptor.value!;

descriptor.value = (...args: unknown[]) => {
if (!this.isTracingEnabled()) {
return originalMethod.apply(target, [...args]);
// eslint-disable-next-line @typescript-eslint/no-this-alias
const tracerRef = this;
// Use a function() {} instead of an () => {} arrow function so that we can
// access `myClass` as `this` in a decorated `myClass.myMethod()`.
descriptor.value = function (...args: unknown[]) {
if (!tracerRef.isTracingEnabled()) {
return originalMethod.apply(this, [...args]);
}

return this.provider.captureAsyncFunc(`### ${originalMethod.name}`, async subsegment => {
return tracerRef.provider.captureAsyncFunc(`### ${originalMethod.name}`, async subsegment => {
let result;
try {
result = await originalMethod.apply(target, [...args]);
this.addResponseAsMetadata(result, originalMethod.name);
result = await originalMethod.apply(this, [...args]);
tracerRef.addResponseAsMetadata(result, originalMethod.name);
} catch (error) {
this.addErrorAsMetadata(error as Error);
tracerRef.addErrorAsMetadata(error as Error);

throw error;
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ const tracer = new Tracer({ serviceName: serviceName });
const dynamoDBv3 = tracer.captureAWSv3Client(new DynamoDBClient({}));

export class MyFunctionWithDecorator {
private readonly returnValue: string;

public constructor() {
this.returnValue = customResponseValue;
}

@tracer.captureLambdaHandler()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
Expand Down Expand Up @@ -77,9 +83,9 @@ export class MyFunctionWithDecorator {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
public myMethod(): string {
return customResponseValue;
return this.returnValue;
}
}

export const handlerClass = new MyFunctionWithDecorator();
export const handler = handlerClass.handler;
export const handler = handlerClass.handler.bind(handlerClass);
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ const tracer = new Tracer({ serviceName: serviceName });
const dynamoDBv3 = tracer.captureAWSv3Client(new DynamoDBClient({}));

export class MyFunctionWithDecorator {
private readonly returnValue: string;

public constructor() {
this.returnValue = customResponseValue;
}

@tracer.captureLambdaHandler()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
Expand Down Expand Up @@ -72,9 +78,9 @@ export class MyFunctionWithDecorator {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
public myMethod(): string {
return customResponseValue;
return this.returnValue;
}
}

export const handlerClass = new MyFunctionWithDecorator();
export const handler = handlerClass.handler;
export const handler = handlerClass.handler.bind(handlerClass);
69 changes: 69 additions & 0 deletions packages/tracer/tests/unit/Tracer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,37 @@ describe('Class: Tracer', () => {

});

test('when used as decorator and when calling the handler, it has access to member variables', async () => {

// Prepare
const tracer: Tracer = new Tracer();
const newSubsegment: Segment | Subsegment | undefined = new Subsegment('### dummyMethod');
jest.spyOn(tracer.provider, 'getSegment')
.mockImplementation(() => newSubsegment);
setContextMissingStrategy(() => null);

class Lambda implements LambdaInterface {
private readonly memberVariable: string;

public constructor(memberVariable: string) {
this.memberVariable = memberVariable;
}

@tracer.captureLambdaHandler()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
public async handler<TEvent, TResult>(_event: TEvent, _context: Context, _callback: Callback<TResult>): void | Promise<TResult> {
return <TResult>(`memberVariable:${this.memberVariable}` as unknown);
}

}

// Act / Assess
const lambda = new Lambda('someValue');
const handler = lambda.handler.bind(lambda);
expect(await handler({}, context, () => console.log('Lambda invoked!'))).toEqual('memberVariable:someValue');

});
});

describe('Method: captureMethod', () => {
Expand Down Expand Up @@ -1009,6 +1040,44 @@ describe('Class: Tracer', () => {

});

test('when used as decorator and when calling a method in the class, it has access to member variables', async () => {

// Prepare
const tracer: Tracer = new Tracer();
const newSubsegment: Segment | Subsegment | undefined = new Subsegment('### dummyMethod');
jest.spyOn(tracer.provider, 'getSegment')
.mockImplementation(() => newSubsegment);
setContextMissingStrategy(() => null);

class Lambda implements LambdaInterface {
private readonly memberVariable: string;

public constructor(memberVariable: string) {
this.memberVariable = memberVariable;
}

@tracer.captureMethod()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
public async dummyMethod(): Promise<string> {
return `memberVariable:${this.memberVariable}`;
}

@tracer.captureLambdaHandler()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
public async handler<TEvent, TResult>(_event: TEvent, _context: Context, _callback: Callback<TResult>): void | Promise<TResult> {
return <TResult>(await this.dummyMethod() as unknown);
}

}

// Act / Assess
const lambda = new Lambda('someValue');
expect(await lambda.dummyMethod()).toEqual('memberVariable:someValue');

});

});

describe('Method: captureAWS', () => {
Expand Down