Feature request: allow enabling log buffering via constructor

[dreamorosi]

Use case

In #3617 we have implemented a mechanism to buffer logs. The next step in the implementation is to make the feature configurable via constructor options.

Solution/User Experience

Customers should be able to configure log buffering via constructor options.

The ConstructorOptions type/object is already pretty crowded (link), so we should add the new settings as a nested object.

In practice, we should add a new optional bufferConfig object to ConstructorOptions and this object should have the following keys all optional:

• enabled: boolean (defaults to true)
• maxBytes - this allows customers to configure the max size of the buffer in bytes (optional defaults to 1024)
• flushOnErrorLog - this allows customers to toggle whether the buffer is flushed when calling logger.error() (optional defaults to true)
• bufferAtVerbosity - this allows customers to configure the threshold at which we'll buffer logs (optional defaults to DEBUG | debug, accepts TRACE, DEBUG, INFO, and WARN plus their corresponding small case versions - you can never buffer ERROR logs)

The configuration logic of the Logger can be found in the setOptions() method. The order in which things are initialized in this method matters, but for this feature I can't think of any dependency so I'd just add it at the end here.

To follow the existing pattern, we should add a new #setLogBuffering() method (feel free to use a better name as long as it starts with the #set prefix). This method:

• is called only if ConstructorOptions['bufferConfig'] is present
• accepts the ConstructorOptions['bufferConfig'] object
• configures the appropriate fields that were added in feat(logger): Add log buffer and flush method #3617 (we might need to move the instantiation of the #buffer here, since the max size is know only at this point)

Then, after this is added, we should also:

• make the flushBuffer() method public
• add the newly public method to the LoggerInterface
• modify the error() method to flush the buffer if log buffering is enabled && flushOnErrorLog is true

Besides the implementation, we should update the unit tests that use the feature to now use both the public config and new public methods as well as add tests for the new config + flush-on-error behavior. Likewise, we should make sure the new types are added and documented properly (see other types in the same file for reference).

Alternative solutions

N/A

Acknowledgment

• This feature request meets Powertools for AWS Lambda (TypeScript) Tenets
• Should this be considered in other Powertools for AWS Lambda languages? i.e. Python, Java, and .NET

Future readers

Please react with 👍 and your use case to help us understand customer demand.

[dreamorosi]

Hey @leandrodamascena, I can't remember if we agreed on a default size for the log buffer in bytes. I checked the RFC but I didn't write it down.

What do you think we should put?

[leandrodamascena]

Hello @dreamorosi! I was testing and I came up with an arbitrary value of 20480 in Python. I saw that the information I save in buffer has on average 200 bytes, so I calculated more or less 100 lines of debug. It seems to be a reasonable number, but since this is still a long way off, what do you think?

Also:

enabled: boolean and required

In Python, I consider that if the Buffer object is passed to the constructor, it will always be true, so I removed this field. I think having the enabled field might be unnecessary, WDYT?

[dreamorosi]

enabled: boolean and required

In Python, I consider that if the Buffer object is passed to the constructor, it will always be true, so I removed this field. I think having the enabled field might be unnecessary, WDYT?

I initially thought the same, but what if customers want to enable the buffering but leave every other setting with default values? They'd have to do this in JS

const logger = new Logger({
  logLevel: 'WARN',
  bufferConfig: {} // buffer is enabled, with default max size, and other default options
});

Imo this is kind of ugly, so I thought of making it explicit by adding enabled But I guess we could also just make it optional so that customers can do this:

const logger = new Logger({
  logLevel: 'WARN',
  bufferConfig: { // this means enabled
    maxBytes: 12345,
  }
});

You're right - I'll update the issue.

For the size, I don't have any strong feelings and will go with the number that you got from the tests.

Thank you for the quick reply!

[ConnorKirk]

I'll work on this next