Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(logger): remove logEvent from required settings #1821

Merged
merged 4 commits into from
Dec 19, 2023
Merged

docs(logger): remove logEvent from required settings #1821

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8f61fe1
docs: removed required settings and added section for envs
am29d Dec 19, 2023
3867705
fix typo
am29d Dec 19, 2023
60d3c5d
remove env section in logs, adjust woring and link to api and index page
am29d Dec 19, 2023
efb5b71
clarified wording on logEvent
am29d Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions docs/core/logger.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,18 @@ The `Logger` utility must always be instantiated outside the Lambda handler. By
```

### Utility settings

The library requires two settings. You can set them as environment variables, or pass them in the constructor.
The library has three optional settings, which can be set via environment variables or passed in the constructor.

These settings will be used across all logs emitted:

| 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 incoming event** | Whether to log or not the incoming event when using the decorator or middleware | `POWERTOOLS_LOGGER_LOG_EVENT` | `false` | `true`, `false` | `false` | `logEvent` |
| **Debug log sampling** | 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.5` | `sampleRateValue` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than removing the sampleRateValue from 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.

Copy link
Contributor Author

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?

Copy link
Contributor

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 logEvent being 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.

| **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` |

See all enivronment variables in the [Environment variables](../index.md/#environment-variables) section.
Check API docs to learn more about [Logger constructor options](https://docs.powertools.aws.dev/lambda/typescript/latest/api/types/_aws_lambda_powertools_logger.types.ConstructorOptions.html){target="_blank"}.

#### Example using AWS Serverless Application Model (SAM)

Expand Down Expand Up @@ -153,9 +154,9 @@ In each case, the printed log will look like this:
}
```

#### Log incoming event
### 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`.
When debugging in non-production environments, you can instruct Logger to log the incoming event with the middleware/decorator parameter `logEvent`.

???+ warning
This is disabled by default to prevent sensitive info being logged
Expand All @@ -174,6 +175,8 @@ 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.

Use `POWERTOOLS_LOGGER_LOG_EVENT` environment variable to enable or disable (`true`/`false`) this feature.

### 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:
Expand Down Expand Up @@ -389,7 +392,6 @@ 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.


## Advanced

### Log levels
Expand Down