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

Fix(metrics): decorated class methods cannot access this #1059

Merged
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
20 changes: 16 additions & 4 deletions docs/core/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,20 @@ You can add default dimensions to your metrics by passing them as parameters in
const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' });
const DEFAULT_DIMENSIONS = { 'environment': 'prod', 'foo': 'bar' };

export class MyFunction implements LambdaInterface {
export class Lambda implements LambdaInterface {
// Decorate your handler class method
@metrics.logMetrics({ defaultDimensions: DEFAULT_DIMENSIONS })
public async handler(_event: any, _context: any): Promise<void> {
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
}
}

const handlerClass = new Lambda();
export const handler = handlerClass.handler.bind(handlerClass); // (1)
```

1. Binding your handler method allows your handler to access `this` within the class methods.

If you'd like to remove them at some point, you can use the `clearDefaultDimensions` method.

### Flushing metrics
Expand Down Expand Up @@ -362,15 +367,20 @@ The `logMetrics` decorator of the metrics utility can be used when your Lambda h

const metrics = new Metrics({ namespace: 'serverlessAirline', serviceName: 'orders' });

export class MyFunction implements LambdaInterface {
class Lambda implements LambdaInterface {

@metrics.logMetrics()
public async handler(_event: any, _context: any): Promise<void> {
metrics.addMetric('successfulBooking', MetricUnits.Count, 1);
}
}

const handlerClass = new Lambda();
export const handler = handlerClass.handler.bind(handlerClass); // (1)
```

1. Binding your handler method allows your handler to access `this` within the class methods.

=== "Example CloudWatch Logs excerpt"

```json hl_lines="2 7 10 15 22"
Expand Down Expand Up @@ -629,6 +639,8 @@ CloudWatch EMF uses the same dimensions across all your metrics. Use `singleMetr
}
}

export const myFunction = new Lambda();
export const handler = myFunction.handler;
const handlerClass = new Lambda();
export const handler = handlerClass.handler.bind(handlerClass); // (1)
```

1. Binding your handler method allows your handler to access `this` within the class methods.
26 changes: 15 additions & 11 deletions packages/metrics/src/Metrics.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Callback, Context } from 'aws-lambda';
import { Callback, Context, Handler } from 'aws-lambda';
import { Utility } from '@aws-lambda-powertools/commons';
import { MetricsInterface } from '.';
import { ConfigServiceInterface, EnvironmentVariablesService } from './config';
Expand Down Expand Up @@ -58,8 +58,8 @@ const DEFAULT_NAMESPACE = 'default_namespace';
* }
* }
*
* export const handlerClass = new MyFunctionWithDecorator();
* export const handler = handlerClass.handler;
* const handlerClass = new MyFunctionWithDecorator();
* export const handler = handlerClass.handler.bind(handlerClass);
* ```
*
* ### Standard function
Expand Down Expand Up @@ -221,8 +221,8 @@ class Metrics extends Utility implements MetricsInterface {
* }
* }
*
* export const handlerClass = new MyFunctionWithDecorator();
* export const handler = handlerClass.handler;
* const handlerClass = new MyFunctionWithDecorator();
* export const handler = handlerClass.handler.bind(handlerClass);
* ```
*
* @decorator Class
Expand All @@ -236,24 +236,28 @@ class Metrics extends Utility implements MetricsInterface {
this.setDefaultDimensions(defaultDimensions);
}

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 = ( async (event: unknown, context: Context, callback: Callback): Promise<unknown> => {
this.functionName = context.functionName;
if (captureColdStartMetric) this.captureColdStartMetric();
// eslint-disable-next-line @typescript-eslint/no-this-alias
const metricsRef = this;
// Use a function() {} instead of an () => {} arrow function so that we can
// access `myClass` as `this` in a decorated `myClass.myMethod()`.
descriptor.value = ( async function(this: Handler, event: unknown, context: Context, callback: Callback): Promise<unknown> {
metricsRef.functionName = context.functionName;
if (captureColdStartMetric) metricsRef.captureColdStartMetric();

let result: unknown;
try {
result = await originalMethod.apply(target, [ event, context, callback ]);
result = await originalMethod.apply(this, [ event, context, callback ]);
} catch (error) {
throw error;
} finally {
this.publishStoredMetrics();
metricsRef.publishStoredMetrics();
}

return result;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Metrics, MetricUnits } from '../../src';
import { Context } from 'aws-lambda';
import { LambdaInterface } from '../../examples/utils/lambda/LambdaInterface';
import { LambdaInterface } from '@aws-lambda-powertools/commons';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this use the published version instead of local version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, given that we are inside the NPM workspace & it uses the only lock file that is present at the root, then no, it should get the local one (see screenshot below):

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image


const namespace = process.env.EXPECTED_NAMESPACE ?? 'CdkExample';
const serviceName = process.env.EXPECTED_SERVICE_NAME ?? 'MyFunctionWithStandardHandler';
Expand All @@ -19,21 +19,28 @@ const metrics = new Metrics({ namespace: namespace, serviceName: serviceName });
class Lambda implements LambdaInterface {

@metrics.logMetrics({ captureColdStartMetric: true, defaultDimensions: JSON.parse(defaultDimensions), throwOnEmptyMetrics: true })
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
public async handler(_event: unknown, _context: Context): Promise<void> {
metrics.addMetric(metricName, metricUnit, parseInt(metricValue));
metrics.addDimension(
Object.entries(JSON.parse(extraDimension))[0][0],
Object.entries(JSON.parse(extraDimension))[0][1] as string,
);


this.dummyMethod();
}

private dummyMethod(): void {
const metricWithItsOwnDimensions = metrics.singleMetric();
metricWithItsOwnDimensions.addDimension(
Object.entries(JSON.parse(singleMetricDimension))[0][0],
Object.entries(JSON.parse(singleMetricDimension))[0][1] as string,
);

metricWithItsOwnDimensions.addMetric(singleMetricName, singleMetricUnit, parseInt(singleMetricValue));
}
}

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

expect(console.log).toBeCalledTimes(1);
});

test('Using decorator should preserve `this` in decorated class', async () => {
// Prepare
const metrics = new Metrics({ namespace: 'test' });

// Act
class LambdaFunction implements LambdaInterface {
@metrics.logMetrics()
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
public handler<TEvent, TResult>(
_event: TEvent,
_context: Context,
_callback: Callback<TResult>,
): void | Promise<TResult> {
this.dummyMethod();
}

private dummyMethod(): void {
metrics.addMetric('test_name', MetricUnits.Seconds, 1);
}
}
await new LambdaFunction().handler(dummyEvent, dummyContext.helloworldContext, () => console.log('Lambda invoked!'));
const loggedData = JSON.parse(consoleSpy.mock.calls[0][0]);

// Assess
expect(console.log).toBeCalledTimes(1);
dreamorosi marked this conversation as resolved.
Show resolved Hide resolved
expect(loggedData._aws.CloudWatchMetrics[0].Metrics.length).toEqual(1);
expect(loggedData._aws.CloudWatchMetrics[0].Metrics[0].Name).toEqual('test_name');
expect(loggedData._aws.CloudWatchMetrics[0].Metrics[0].Unit).toEqual('Seconds');
});
});

describe('Feature: Custom Config Service', () => {
Expand Down