Skip to content

Commit 85c67b6

Browse files
authored
Steal #864 for lambda-sqs-worker (#871)
* Steal #864 for lambda-sqs-worker * Add changeset
1 parent 034fe46 commit 85c67b6

File tree

8 files changed

+107
-49
lines changed

8 files changed

+107
-49
lines changed

.changeset/olive-worms-run.md

+9-1
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,17 @@ Before:
1010

1111
```typescript
1212
import createLogger from '@seek/logger';
13-
import Koa from 'koa';
13+
import Koa, { Context } from 'koa';
1414
import { RequestLogging } from 'seek-koala';
15+
1516
const rootLogger = createLogger();
17+
1618
const contextLogger = (ctx: Context) =>
1719
rootLogger.child(RequestLogging.contextFields(ctx));
20+
1821
const app = new Koa().use((ctx) => {
1922
rootLogger.info('Has no context');
23+
2024
contextLogger(ctx).info('Has context');
2125
});
2226
```
@@ -27,10 +31,14 @@ After:
2731
import createLogger from '@seek/logger';
2832
import Koa from 'koa';
2933
import { RequestLogging } from 'seek-koala';
34+
3035
const { createContextMiddleware, mixin } =
3136
RequestLogging.createContextStorage();
37+
3238
const contextMiddleware = createContextMiddleware();
39+
3340
const logger = createLogger({ mixin });
41+
3442
const app = new Koa().use(contextMiddleware).use((ctx) => {
3543
logger.info('Has context');
3644
});

.changeset/pink-flowers-vanish.md

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
---
2+
'skuba': patch
3+
---
4+
5+
template/lambda-sqs-worker: Use [AsyncLocalStorage](https://nodejs.org/docs/latest-v16.x/api/async_context.html#asynchronous-context-tracking) to track logger context
6+
7+
We now employ this Node.js API to thread logging context through the handler of your Lambda function. This enables use of a singleton `logger` instance instead of manually propagating Lambda context and juggling `rootLogger`s and `contextLogger`s, and is equivalent to #864.
8+
9+
Before:
10+
11+
```typescript
12+
import createLogger from '@seek/logger';
13+
import { Context } from 'aws-lambda';
14+
15+
const rootLogger = createLogger();
16+
17+
const contextLogger = ({ awsRequestId }: Context) =>
18+
rootLogger.child({ awsRequestId });
19+
20+
const handler = async (_event: unknown, ctx: Context) => {
21+
rootLogger.info('Has no context');
22+
23+
contextLogger(ctx).info('Has context');
24+
};
25+
```
26+
27+
After:
28+
29+
```typescript
30+
import { AsyncLocalStorage } from 'async_hooks';
31+
32+
import createLogger from '@seek/logger';
33+
import { Context } from 'aws-lambda';
34+
35+
const loggerContext = new AsyncLocalStorage<{ awsRequestId: string }>();
36+
37+
const logger = createLogger({
38+
mixin: () => loggerContext.getStore() ?? {},
39+
});
40+
41+
const handler = (_event: unknown, { awsRequestId }: Context) =>
42+
loggerContext.run({ awsRequestId }, async () => {
43+
logger.info('Has context');
44+
});
45+
```

template/lambda-sqs-worker/src/app.test.ts

+9-9
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { metricsClient } from 'src/framework/metrics';
22
import { createCtx, createSqsEvent } from 'src/testing/handler';
3-
import { contextLogger } from 'src/testing/logging';
3+
import { logger } from 'src/testing/logging';
44
import { scoringService, sns } from 'src/testing/services';
55
import { chance, mockJobPublishedEvent } from 'src/testing/types';
66

@@ -19,7 +19,7 @@ describe('handler', () => {
1919

2020
const increment = jest.spyOn(metricsClient, 'increment').mockReturnValue();
2121

22-
beforeAll(contextLogger.spy);
22+
beforeAll(logger.spy);
2323
beforeAll(scoringService.spy);
2424
beforeAll(sns.spy);
2525

@@ -31,7 +31,7 @@ describe('handler', () => {
3131
});
3232

3333
afterEach(() => {
34-
contextLogger.clear();
34+
logger.clear();
3535
increment.mockClear();
3636
scoringService.clear();
3737
sns.clear();
@@ -44,9 +44,9 @@ describe('handler', () => {
4444

4545
expect(scoringService.request).toBeCalledTimes(1);
4646

47-
expect(contextLogger.error).not.toBeCalled();
47+
expect(logger.error).not.toBeCalled();
4848

49-
expect(contextLogger.info.mock.calls).toEqual([
49+
expect(logger.info.mock.calls).toEqual([
5050
[{ count: 1 }, 'received jobs'],
5151
[{ snsMessageId: expect.any(String) }, 'scored job'],
5252
['request'],
@@ -75,7 +75,7 @@ describe('handler', () => {
7575

7676
await expect(app.handler(event, ctx)).rejects.toThrow('invoke error');
7777

78-
expect(contextLogger.error).toBeCalledWith({ err }, 'request');
78+
expect(logger.error).toBeCalledWith({ err }, 'request');
7979
});
8080

8181
it('bubbles up SNS error', async () => {
@@ -87,7 +87,7 @@ describe('handler', () => {
8787

8888
await expect(app.handler(event, ctx)).rejects.toThrow('invoke error');
8989

90-
expect(contextLogger.error).toBeCalledWith({ err }, 'request');
90+
expect(logger.error).toBeCalledWith({ err }, 'request');
9191
});
9292

9393
it('throws on zero records', async () => {
@@ -97,7 +97,7 @@ describe('handler', () => {
9797

9898
await expect(app.handler(event, ctx)).rejects.toThrow('invoke error');
9999

100-
expect(contextLogger.error).toBeCalledWith({ err }, 'request');
100+
expect(logger.error).toBeCalledWith({ err }, 'request');
101101
});
102102

103103
it('throws on multiple records', async () => {
@@ -110,6 +110,6 @@ describe('handler', () => {
110110

111111
await expect(app.handler(event, ctx)).rejects.toThrow('invoke error');
112112

113-
expect(contextLogger.error).toBeCalledWith({ err }, 'request');
113+
expect(logger.error).toBeCalledWith({ err }, 'request');
114114
});
115115
});

template/lambda-sqs-worker/src/app.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import 'skuba-dive/register';
33
import { SQSEvent } from 'aws-lambda';
44

55
import { createHandler } from 'src/framework/handler';
6+
import { logger } from 'src/framework/logging';
67
import { metricsClient } from 'src/framework/metrics';
78
import { validateJson } from 'src/framework/validation';
89
import { scoreJobPublishedEvent, scoringService } from 'src/services/jobScorer';
@@ -16,7 +17,7 @@ const smokeTest = async () => {
1617
await Promise.all([scoringService.smokeTest(), sendPipelineEvent({}, true)]);
1718
};
1819

19-
export const handler = createHandler<SQSEvent>(async (event, { logger }) => {
20+
export const handler = createHandler<SQSEvent>(async (event) => {
2021
// Treat an empty object as our smoke test event.
2122
if (!Object.keys(event).length) {
2223
logger.info('received smoke test request');

template/lambda-sqs-worker/src/framework/handler.test.ts

+10-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { createCtx } from 'src/testing/handler';
2-
import { contextLogger } from 'src/testing/logging';
2+
import { logger } from 'src/testing/logging';
33
import { chance } from 'src/testing/types';
44

55
import { createHandler } from './handler';
@@ -8,14 +8,14 @@ describe('createHandler', () => {
88
const ctx = createCtx();
99
const input = chance.paragraph();
1010

11-
beforeAll(contextLogger.spy);
11+
beforeAll(logger.spy);
1212

13-
afterEach(contextLogger.clear);
13+
afterEach(logger.clear);
1414

1515
it('handles happy path', async () => {
1616
const output = chance.paragraph();
1717

18-
const handler = createHandler((event, { logger }) => {
18+
const handler = createHandler((event) => {
1919
expect(event).toBe(input);
2020

2121
logger.info('hello from handler');
@@ -25,9 +25,9 @@ describe('createHandler', () => {
2525

2626
await expect(handler(input, ctx)).resolves.toBe(output);
2727

28-
expect(contextLogger.error).not.toBeCalled();
28+
expect(logger.error).not.toBeCalled();
2929

30-
expect(contextLogger.info.mock.calls).toEqual([
30+
expect(logger.info.mock.calls).toEqual([
3131
['hello from handler'],
3232
['request'],
3333
]);
@@ -40,9 +40,9 @@ describe('createHandler', () => {
4040

4141
await expect(handler(input, ctx)).rejects.toThrow('invoke error');
4242

43-
expect(contextLogger.error.mock.calls).toEqual([[{ err }, 'request']]);
43+
expect(logger.error.mock.calls).toEqual([[{ err }, 'request']]);
4444

45-
expect(contextLogger.info).not.toBeCalled();
45+
expect(logger.info).not.toBeCalled();
4646
});
4747

4848
it('handles sync error', async () => {
@@ -54,8 +54,8 @@ describe('createHandler', () => {
5454

5555
await expect(handler(input, ctx)).rejects.toThrow('invoke error');
5656

57-
expect(contextLogger.error.mock.calls).toEqual([[{ err }, 'request']]);
57+
expect(logger.error.mock.calls).toEqual([[{ err }, 'request']]);
5858

59-
expect(contextLogger.info).not.toBeCalled();
59+
expect(logger.info).not.toBeCalled();
6060
});
6161
});
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,20 @@
1-
import { Logger } from '@seek/logger';
21
import { Context } from 'aws-lambda';
32

4-
import { contextLogger } from 'src/framework/logging';
3+
import { logger, loggerContext } from 'src/framework/logging';
54

6-
export const createHandler = <Event, Output = unknown>(
7-
fn: (event: Event, ctx: { logger: Logger }) => Promise<Output>,
8-
) =>
9-
async function lambdaHandler(event: Event, ctx: Context) {
10-
const logger = contextLogger(ctx);
5+
export const createHandler =
6+
<Event, Output = unknown>(fn: (event: Event) => Promise<Output>) =>
7+
(event: Event, { awsRequestId }: Context) =>
8+
loggerContext.run({ awsRequestId }, async () => {
9+
try {
10+
const output = await fn(event);
1111

12-
try {
13-
const output = await fn(event, { logger });
12+
logger.info('request');
1413

15-
logger.info('request');
14+
return output;
15+
} catch (err) {
16+
logger.error({ err }, 'request');
1617

17-
return output;
18-
} catch (err) {
19-
logger.error({ err }, 'request');
20-
21-
throw new Error('invoke error');
22-
}
23-
};
18+
throw new Error('invoke error');
19+
}
20+
});

template/lambda-sqs-worker/src/framework/logging.ts

+11-6
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,27 @@
1+
import { AsyncLocalStorage } from 'async_hooks';
2+
13
import createLogger from '@seek/logger';
2-
import { Context } from 'aws-lambda';
34

45
import { config } from 'src/config';
56

6-
export const rootLogger = createLogger({
7+
interface LoggerContext {
8+
awsRequestId: string;
9+
}
10+
11+
export const loggerContext = new AsyncLocalStorage<LoggerContext>();
12+
13+
export const logger = createLogger({
714
base: {
815
environment: config.environment,
916
version: config.version,
1017
},
1118

1219
level: config.logLevel,
1320

21+
mixin: () => loggerContext.getStore() ?? {},
22+
1423
name: config.name,
1524

1625
transport:
1726
config.environment === 'local' ? { target: 'pino-pretty' } : undefined,
1827
});
19-
20-
/* istanbul ignore next: @seek/logger interface */
21-
export const contextLogger = ({ awsRequestId }: Context) =>
22-
rootLogger.child({ awsRequestId });
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import * as logging from 'src/framework/logging';
22

3-
export const contextLogger = {
3+
export const logger = {
44
error: jest.fn(),
55
info: jest.fn(),
66

77
clear: () => {
8-
contextLogger.error.mockClear();
9-
contextLogger.info.mockClear();
8+
logger.error.mockClear();
9+
logger.info.mockClear();
1010
},
1111

12-
spy: () =>
13-
jest.spyOn(logging, 'contextLogger').mockReturnValue(contextLogger as any),
12+
spy: () => {
13+
jest.spyOn(logging.logger, 'error').mockImplementation(logger.error);
14+
jest.spyOn(logging.logger, 'info').mockImplementation(logger.info);
15+
},
1416
};

0 commit comments

Comments
 (0)