docs(logger): remove logEvent from required settings #1821
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,8 +50,6 @@ These settings will be used across all logs emitted: | |
| | ---------------------- | ---------------------------------------------------------------------------------------------------------------- | ------------------------------- | ------------------- | ------------------------------------------------------ | ------------------- | --------------------- | | ||
| | **Service name** | Sets the name of service of which the Lambda function is part of, that will be present across all log statements | `POWERTOOLS_SERVICE_NAME` | `service_undefined` | Any string | `serverlessAirline` | `serviceName` | | ||
| | **Logging level** | Sets how verbose Logger should be, from the most verbose to the least verbose (no logs) | `POWERTOOLS_LOG_LEVEL` | `INFO` | `DEBUG`, `INFO`, `WARN`, `ERROR`, `CRITICAL`, `SILENT` | `ERROR` | `logLevel` | | ||
|
|
||
| #### Example using AWS Serverless Application Model (SAM) | ||
|
|
||
|
|
@@ -153,7 +151,7 @@ In each case, the printed log will look like this: | |
| } | ||
| ``` | ||
|
|
||
| ### Log incoming event | ||
|
|
||
| When debugging in non-production environments, you can instruct Logger to log the incoming event with the middleware/decorator parameter `logEvent` or via `POWERTOOLS_LOGGER_LOG_EVENT` env var set to `true`. | ||
|
|
||
|
|
@@ -174,6 +172,9 @@ When debugging in non-production environments, you can instruct Logger to log th | |
|
|
||
| 1. Binding your handler method allows your handler to access `this` within the class methods. | ||
|
|
||
| Logging incoming events only works with middy or decorator by using `injectLambdaContext`. | ||
|
There was a problem hiding this comment. I think this first sentence is somewhat redundant as it rephrases the one above almost entirely ( I think most of the confusion we have seen reports of is related to the fact that just enabling the env variable is not enough, which is addressed by the next sentence you've added. Maybe we could just rephrase the initial sentence to be more specific rather than adding another one. What do you think? |
||
| Only setting `POWEETOOLS_LOGGER_LOG_EVENT` to `true` will not log the incoming event. | ||
|
There was a problem hiding this comment. There's a typo in the environment variable name |
||
|
|
||
| ### Appending persistent additional log keys and values | ||
|
|
||
| You can append additional persistent keys and values in the logs generated during a Lambda invocation using either mechanism: | ||
|
|
@@ -389,6 +390,18 @@ The error will be logged with default key name `error`, but you can also pass yo | |
| !!! tip "Logging errors and log level" | ||
| You can also log errors using the `warn`, `info`, and `debug` methods. Be aware of the log level though, you might miss those errors when analyzing the log later depending on the log level configuration. | ||
|
|
||
| ### Environment variables | ||
|
There was a problem hiding this comment. We have this table in the main page (here) where we keep track of all env variables, including the ones below. Should we just link that one? I'm not sure, what do you think? There was a problem hiding this comment. Yes, let's keep it one place. |
||
|
|
||
| The following environment variables are available to configure Logger at a global scope: | ||
|
|
||
|
|
||
| | Setting | Description | Environment variable | Default Value | Allowed Values | Example Value | Constructor parameter | | ||
| | ---------------------- |-----------------------------------------------------------------------------------------------------------------| ------------------------------- | ------------------- | ------------------------------------------------------ | ------------------- | --------------------- | | ||
| | **Service name** | Sets the name of service of which the Lambda function is part of, that will be present across all log statements | `POWERTOOLS_SERVICE_NAME` | `service_undefined` | Any string | `serverlessAirline` | `serviceName` | | ||
| | **Logging level** | Sets how verbose Logger should be, from the most verbose to the least verbose (no logs) | `POWERTOOLS_LOG_LEVEL` | `INFO` | `DEBUG`, `INFO`, `WARN`, `ERROR`, `CRITICAL`, `SILENT` | `ERROR` | `logLevel` | | ||
| | **Log event** | Whether to log or not the incoming event when using the decorator or middleware | `POWERTOOLS_LOGGER_LOG_EVENT` | `false` | `true`, `false` | `true` | `logEvent` | | ||
| | **Sample rate** | Probability that a Lambda invocation will print all the log items regardless of the log level setting | `POWERTOOLS_LOGGER_SAMPLE_RATE` | `0` | `0.0` to `1.0` | `0.1` | `sampleRateValue` | | ||
|
|
||
|
|
||
| ## Advanced | ||
|
|
||
|
|
||
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than removing the
sampleRateValuefrom here I would instead change the wording above to remove the "required" mention entirely.My reasoning here is that none of the settings is really required since they all have defaults and they are all typed as such.
So to sum up I'd change the first sentence from what it is now to something along the lines of:
The logger accepts three settings.or something similar.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My goal was to align it with python docs and have a dedicated section for env variables.
I think the confusion comes from the phrase 'or pass them in the constructor' and because we mix environment variables and constructor settings, which overlap with exceptions. So if we keep three ENVs that are also part of the constructor people might assume that the other constructor values are missing from the docs.
We could add additional deep link to API docs to the constructor. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think having a section to the env variables or linking to the existing one in the main page is great.
I was mainly arguing against the "required" wording which is really not true, and at the same time wanting to have a place to get started quickly.
As a user I want to read at the very top of the page how to get started using the library either because I don't know or remember it. Having all the applicable settings surfaced early is useful and we have evidence that customers are doing this (we had multiple reports about
logEventbeing out of place here).I'm in favor of deep linking the API docs and making the wording clearer if needed, but I wouldn't remove the parameter from here.