feat(RequestLogging): Add createContextStorage
#153
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
72636c
reviewed
May 17, 2022
72636c
approved these changes
May 18, 2022
src/requestLogging/requestLogging.ts
Outdated
| /** | ||
| * Returns context fields for the passed Koa context | ||
| */ | ||
| export type ContextFields = (ctx: Koa.Context) => Record<string, unknown>; |
There was a problem hiding this comment.
Would it be worth aligning this closer with RequestLogging.createMiddleware:
Suggested change
| export type ContextFields = (ctx: Koa.Context) => Record<string, unknown>; | |
| export type ContextFields = (ctx: Koa.Context, fields: Fields) => Record<string, unknown>; |
This would make it easier to extend the context without having to manually invoke the baseline contextFields:
createLoggerContextStorage((ctx, fields) => ({
...fields,
additionalField: getAdditionalField(ctx),
}));There was a problem hiding this comment.
Oooo yep that would make sense
samchungy
changed the title
|
fyi I don't have perms in this repo 😆 @72636c |
72636c
changed the title
createContextStorage
|
FYI, this doesn't show as a breaking change on renovate PRs. https://github.com/SEEK-Jobs/cm-rr-service/pull/167 Because BREAKING CHANGE wasn't in a commit message. |
rip thanks for letting us know. Should be relatively okay though since Node 10 is no longer maintained |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
BREAKING CHANGE: The minimum version of Node.js has been bumped up to 12.17
This adds functionality to allow for a root logger instance to retain request based context.
This means you can simply import the logger in any file and the request context will just be there when you call
logger.info,logger.erroror any other method.This means you will no longer need to create a child logger to pass around the codebase in order to retain request context.
This implementation uses the Node.js AsyncLocalStorage instance to track logger context.
There is a performance hit of ~1.75% on Node 16.2.0+ according to this which is negligible