Skip to content

Commit 2c84e5f

Browse files
committed
refactor: initial review changes
1 parent cb4de92 commit 2c84e5f

File tree

6 files changed

+149
-44
lines changed

6 files changed

+149
-44
lines changed

Diff for: docs/core/tracer.md

+20-10
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ You can trace other Class methods using the `captureMethod` decorator or any arb
256256
}
257257
258258
const handlerClass = new Lambda();
259-
export const handler = myFunction.handler.bind(handlerClass); // (1)
259+
export const handler = handlerClass.handler.bind(handlerClass); // (1)
260260
```
261261

262262
1. Binding your handler method allows your handler to access `this`.
@@ -412,38 +412,48 @@ Use **`POWERTOOLS_TRACER_CAPTURE_RESPONSE=false`** environment variable to instr
412412
2. You might manipulate **streaming objects that can be read only once**; this prevents subsequent calls from being empty
413413
3. You might return **more than 64K** of data _e.g., `message too long` error_
414414

415-
### Disabling response capture for targeted methods and handlers
416-
417-
Use the `captureResponse: false` option in both `tracer.captureLambdaHandler()` and `tracer.captureMethod()` decorators, or use the same option in the middy `captureLambdaHander` middleware to instruct Tracer **not** to serialize function responses as metadata.
415+
Alternatively, use the `captureResponse: false` option in both `tracer.captureLambdaHandler()` and `tracer.captureMethod()` decorators, or use the same option in the Middy `captureLambdaHander` middleware to instruct Tracer **not** to serialize function responses as metadata.
418416

419417
=== "method.ts"
420418

421-
```typescript hl_lines="5"
419+
```typescript hl_lines="6"
422420
import { Tracer } from '@aws-lambda-powertools/tracer';
423421

424422
const tracer = new Tracer({ serviceName: 'serverlessAirline' });
425-
class MyThing {
426-
@tracer.captureMethod({ captureResponse: false })
427-
myMethod(): string {
423+
424+
class Lambda implements LambdaInterface {
425+
@tracer.captureMethod({ captureResult: false })
426+
public getChargeId(): string {
428427
/* ... */
429428
return 'foo bar';
430429
}
430+
431+
public async handler(_event: any, _context: any): Promise<void> {
432+
/* ... */
433+
}
431434
}
435+
436+
const handlerClass = new Lambda();
437+
export const handler = handlerClass.handler.bind(handlerClass);
432438
```
433439

434440
=== "handler.ts"
435441

436-
```typescript hl_lines="6"
442+
```typescript hl_lines="7"
437443
import { Tracer } from '@aws-lambda-powertools/tracer';
438444
import { LambdaInterface } from '@aws-lambda-powertools/commons';
439445

440446
const tracer = new Tracer({ serviceName: 'serverlessAirline' });
441-
class MyHandler implements LambdaInterface {
447+
448+
class Lambda implements LambdaInterface {
442449
@tracer.captureLambdaHandler({ captureResponse: false })
443450
async handler(_event: any, _context: any): Promise<void> {
444451
/* ... */
445452
}
446453
}
454+
455+
const handlerClass = new Lambda();
456+
export const handler = handlerClass.handler.bind(handlerClass);
447457
```
448458

449459
=== "middy.ts"

Diff for: packages/tracer/src/Tracer.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Handler } from 'aws-lambda';
22
import { AsyncHandler, SyncHandler, Utility } from '@aws-lambda-powertools/commons';
33
import { TracerInterface } from '.';
44
import { ConfigServiceInterface, EnvironmentVariablesService } from './config';
5-
import { HandlerMethodDecorator, TracerOptions, TracerCaptureMethodOptions, TracerCaptureLambdaHandlerOptions, MethodDecorator } from './types';
5+
import { HandlerMethodDecorator, TracerOptions, HandlerOptions, MethodDecorator } from './types';
66
import { ProviderService, ProviderServiceInterface } from './provider';
77
import { Segment, Subsegment } from 'aws-xray-sdk-core';
88

@@ -339,7 +339,7 @@ class Tracer extends Utility implements TracerInterface {
339339
*
340340
* @decorator Class
341341
*/
342-
public captureLambdaHandler(options?: TracerCaptureLambdaHandlerOptions): HandlerMethodDecorator {
342+
public captureLambdaHandler(options?: HandlerOptions): HandlerMethodDecorator {
343343
return (_target, _propertyKey, descriptor) => {
344344
/**
345345
* The descriptor.value is the method this decorator decorates, it cannot be undefined.
@@ -419,7 +419,7 @@ class Tracer extends Utility implements TracerInterface {
419419
*
420420
* @decorator Class
421421
*/
422-
public captureMethod(options?: TracerCaptureMethodOptions): MethodDecorator {
422+
public captureMethod(options?: HandlerOptions): MethodDecorator {
423423
return (_target, _propertyKey, descriptor) => {
424424
// The descriptor.value is the method this decorator decorates, it cannot be undefined.
425425
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
@@ -441,9 +441,9 @@ class Tracer extends Utility implements TracerInterface {
441441
if (options?.captureResponse ?? true) {
442442
tracerRef.addResponseAsMetadata(result, originalMethod.name);
443443
}
444-
445444
} catch (error) {
446445
tracerRef.addErrorAsMetadata(error as Error);
446+
447447
throw error;
448448
} finally {
449449
subsegment?.close();

Diff for: packages/tracer/src/middleware/middy.ts

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
import type middy from '@middy/core';
22
import type { Tracer } from '../Tracer';
33
import type { Segment, Subsegment } from 'aws-xray-sdk-core';
4-
5-
export type CaptureLambdaHandlerOptions = {
6-
captureResponse?: boolean
7-
};
4+
import { HandlerOptions } from '../types';
85

96
/**
107
* A middy middleware automating capture of metadata and annotations on segments or subsegments for a Lambda Handler.
@@ -30,7 +27,7 @@ export type CaptureLambdaHandlerOptions = {
3027
* @param target - The Tracer instance to use for tracing
3128
* @returns middleware object - The middy middleware object
3229
*/
33-
const captureLambdaHandler = (target: Tracer, options?: CaptureLambdaHandlerOptions): middy.MiddlewareObj => {
30+
const captureLambdaHandler = (target: Tracer, options?: HandlerOptions): middy.MiddlewareObj => {
3431
let lambdaSegment: Subsegment | Segment;
3532

3633
const open = (): void => {

Diff for: packages/tracer/src/types/Tracer.ts

+7-25
Original file line numberDiff line numberDiff line change
@@ -27,40 +27,23 @@ type TracerOptions = {
2727
};
2828

2929
/**
30-
* Options for the captureMethod decorator to be used when decorating a method.
30+
* Options for handler decorators and middleware.
3131
*
3232
* Usage:
3333
* @example
3434
* ```typescript
3535
* const tracer = new Tracer();
3636
*
37-
* class MyThing {
38-
* @tracer.captureMethod({ captureResponse: false })
39-
* myMethod(): string {
40-
* return 'foo bar';
41-
* }
42-
* }
43-
* ```
44-
*/
45-
type TracerCaptureMethodOptions = {
46-
captureResponse?: boolean
47-
};
48-
49-
/**
50-
* Options for the captureLambdaHandler decorator to be used when decorating a method.
51-
*
52-
* Usage:
53-
* @example
54-
* ```typescript
55-
* const tracer = new Tracer();
56-
*
57-
* class MyThing implements LambdaInterface {
37+
* class Lambda implements LambdaInterface {
5838
* @tracer.captureLambdaHandler({ captureResponse: false })
5939
* async handler(_event: any, _context: any): Promise<void> {}
6040
* }
41+
*
42+
* const handlerClass = new Lambda();
43+
* export const handler = handlerClass.handler.bind(handlerClass);
6144
* ```
6245
*/
63-
type TracerCaptureLambdaHandlerOptions = {
46+
type HandlerOptions = {
6447
captureResponse?: boolean
6548
};
6649

@@ -76,8 +59,7 @@ type MethodDecorator = (target: any, propertyKey: string | symbol, descriptor: T
7659

7760
export {
7861
TracerOptions,
79-
TracerCaptureLambdaHandlerOptions,
80-
TracerCaptureMethodOptions,
62+
HandlerOptions,
8163
HandlerMethodDecorator,
8264
MethodDecorator
8365
};

Diff for: packages/tracer/tests/unit/Tracer.test.ts

+85
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,47 @@ describe('Class: Tracer', () => {
678678

679679
});
680680

681+
test('when used as decorator while captureResponse is set to true, it captures the response as metadata', async () => {
682+
683+
// Prepare
684+
const tracer: Tracer = new Tracer();
685+
const newSubsegment: Segment | Subsegment | undefined = new Subsegment('## index.handler');
686+
jest.spyOn(tracer.provider, 'getSegment')
687+
.mockImplementation(() => newSubsegment);
688+
setContextMissingStrategy(() => null);
689+
const captureAsyncFuncSpy = jest.spyOn(tracer.provider, 'captureAsyncFunc');
690+
class Lambda implements LambdaInterface {
691+
692+
@tracer.captureLambdaHandler({ captureResponse: true })
693+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
694+
// @ts-ignore
695+
public handler<TEvent, TResult>(_event: TEvent, _context: Context, _callback: Callback<TResult>): void | Promise<TResult> {
696+
return new Promise((resolve, _reject) => resolve({
697+
foo: 'bar'
698+
} as unknown as TResult));
699+
}
700+
701+
}
702+
703+
// Act
704+
await new Lambda().handler(event, context, () => console.log('Lambda invoked!'));
705+
706+
// Assess
707+
expect(captureAsyncFuncSpy).toHaveBeenCalledTimes(1);
708+
expect(captureAsyncFuncSpy).toHaveBeenCalledWith('## index.handler', expect.anything());
709+
expect(newSubsegment).toEqual(expect.objectContaining({
710+
name: '## index.handler',
711+
metadata: {
712+
'hello-world': {
713+
'index.handler response': {
714+
foo: 'bar',
715+
},
716+
},
717+
}
718+
}));
719+
720+
});
721+
681722
test('when used as decorator and with standard config, it captures the response as metadata', async () => {
682723

683724
// Prepare
@@ -1040,6 +1081,50 @@ describe('Class: Tracer', () => {
10401081

10411082
});
10421083

1084+
test('when used as decorator and with captureResponse set to true, it does captures the response as metadata', async () => {
1085+
1086+
// Prepare
1087+
const tracer: Tracer = new Tracer();
1088+
const newSubsegment: Segment | Subsegment | undefined = new Subsegment('### dummyMethod');
1089+
jest.spyOn(newSubsegment, 'flush').mockImplementation(() => null);
1090+
jest.spyOn(tracer.provider, 'getSegment')
1091+
.mockImplementation(() => newSubsegment);
1092+
setContextMissingStrategy(() => null);
1093+
const captureAsyncFuncSpy = jest.spyOn(tracer.provider, 'captureAsyncFunc');
1094+
class Lambda implements LambdaInterface {
1095+
1096+
@tracer.captureMethod()
1097+
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
1098+
// @ts-ignore
1099+
public async dummyMethod(some: string): Promise<string> {
1100+
return new Promise((resolve, _reject) => setTimeout(() => resolve(some), 3000));
1101+
}
1102+
1103+
public async handler<TEvent, TResult>(_event: TEvent, _context: Context, _callback: Callback<TResult>): Promise<TResult> {
1104+
const result = await this.dummyMethod('foo bar');
1105+
1106+
return new Promise((resolve, _reject) => resolve(result as unknown as TResult));
1107+
}
1108+
1109+
}
1110+
1111+
// Act
1112+
await new Lambda().handler(event, context, () => console.log('Lambda invoked!'));
1113+
1114+
// Assess
1115+
expect(captureAsyncFuncSpy).toHaveBeenCalledTimes(1);
1116+
expect(captureAsyncFuncSpy).toHaveBeenCalledWith('### dummyMethod', expect.anything());
1117+
expect(newSubsegment).toEqual(expect.objectContaining({
1118+
name: '### dummyMethod',
1119+
metadata: {
1120+
'hello-world': {
1121+
'dummyMethod response': 'foo bar',
1122+
},
1123+
}
1124+
}));
1125+
1126+
});
1127+
10431128
test('when used as decorator and with standard config, it captures the exception correctly', async () => {
10441129

10451130
// Prepare

Diff for: packages/tracer/tests/unit/middy.test.ts

+31
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,37 @@ describe('Middy middleware', () => {
131131

132132
});
133133

134+
test('when used while captureResponse set to true, it captures the response as metadata', async () => {
135+
136+
// Prepare
137+
const tracer: Tracer = new Tracer();
138+
const newSubsegment: Segment | Subsegment | undefined = new Subsegment('## index.handler');
139+
const setSegmentSpy = jest.spyOn(tracer.provider, 'setSegment').mockImplementation();
140+
jest.spyOn(tracer.provider, 'getSegment').mockImplementation(() => newSubsegment);
141+
setContextMissingStrategy(() => null);
142+
const lambdaHandler: Handler = async (_event: unknown, _context: Context) => ({
143+
foo: 'bar'
144+
});
145+
const handler = middy(lambdaHandler).use(captureLambdaHandler(tracer, { captureResponse: true }));
146+
147+
// Act
148+
await handler({}, context, () => console.log('Lambda invoked!'));
149+
150+
// Assess
151+
expect(setSegmentSpy).toHaveBeenCalledTimes(2);
152+
expect(newSubsegment).toEqual(expect.objectContaining({
153+
name: '## index.handler',
154+
metadata: {
155+
'hello-world': {
156+
'index.handler response': {
157+
foo: 'bar',
158+
},
159+
},
160+
}
161+
}));
162+
163+
});
164+
134165
test('when used with standard config, it captures the response as metadata', async () => {
135166

136167
// Prepare

0 commit comments

Comments
 (0)