Skip to content

feat(logger): Add log level filtering#19

Merged
DanielMuller merged 7 commits intomainfrom
feature/log-level-filtering
Mar 7, 2025
Merged

feat(logger): Add log level filtering#19
DanielMuller merged 7 commits intomainfrom
feature/log-level-filtering

Conversation

@bboure
Copy link
Copy Markdown
Member

@bboure bboure commented Mar 7, 2025

No description provided.

@bboure bboure requested a review from DanielMuller March 7, 2025 11:41
src/index.ts Outdated
const NO_COMPRESS = env.get("SG_LOGGER_NO_COMPRESS").default("false").asBool();
const NO_SKIP = env.get("SG_LOGGER_NO_SKIP").default("false").asBool();
const LOG_TS = env.get("SG_LOGGER_LOG_TS").default("false").asBool();
const LOG_LEVEL = env.get("SG_LOGGER_LOG_LEVEL").default("warn").asEnum<Level>(["debug", "info", "warn", "error"]);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We can discuss what the default should be.
I thought warn would be a sensible good default to avoid noisy logs on prod, by default.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When using Advanced Logging Controls, the log level is set at the Lambda configuration level and available via process.env.AWS_LAMBDA_LOG_LEVEL.
Setting a default LOG_LEVEL to warn might interfere with the value defined in the function definition. To avoid having 2 settings defining the log level, we should set the default to AWS_LAMBDA_LOG_LEVEL if it exists and warn otherwise

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also, if LOG_LEVEL>AWS_LAMBDA_LOG_LEVEL, only logs up-to AWS_LAMBDA_LOG_LEVEL will be used

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Interesting. How is AWS_LAMBDA_LOG_LEVEL affecting console.log?

Powertools also seems to use it and also has a POWERTOOLS_LOG_LEVEL.

See https://docs.powertools.aws.dev/lambda/python/latest/core/logger/#aws-lambda-advanced-logging-controls-alc

I check

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Actually they explain it well here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess we can stick to AWS_LAMBDA_LOG_LEVEL and drop this PR.
I am not sure I see a benefit from custom log level env var.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is still useful in 2 cases:

  • Lambda functions not using ALC (legacy TEXT format)
  • Non Lambda environments (Fargate, ...)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updated

Copy link
Copy Markdown
Collaborator

@DanielMuller DanielMuller left a comment

Choose a reason for hiding this comment

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

Update Readme with the new SG_LOGGER_LOG_LEVEL environment variable.
We should also add a paragraph to showcase the behavior when used together with AWS_LAMBDA_LOG_LEVEL

  • SG_LOGGER_LOG_LEVEL>AWS_LAMBDA_LOG_LEVEL => max log level is AWS_LAMBDA_LOG_LEVEL
  • SG_LOGGER_LOG_LEVEL<AWS_LAMBDA_LOG_LEVEL => max log level is SG_LOGGER_LOG_LEVEL

@DanielMuller DanielMuller merged commit 79724da into main Mar 7, 2025
2 checks passed
@DanielMuller DanielMuller deleted the feature/log-level-filtering branch March 7, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants