brefphp / laravel-bridge

Package to use Laravel on AWS Lambda with Bref
https://bref.sh/docs/frameworks/laravel.html
MIT License
314 stars 63 forks source link

Setting whether to output the initialization log #143

Closed KentarouTakeda closed 7 months ago

KentarouTakeda commented 7 months ago

Hi!

In this pull request, I propose a configurable option to selectively enable logging for the Laravel application initialization process orchestrated by Bref.

Added log_init option to the config file published with php artisan vendor:publish --tag=bref-config.

Motivation

The intention is to streamline the log output to CloudWatch by allowing selective logging, enhancing the readability and utility of the logs.

Currently, logs such as the following are output.

Creating storage directories: /tmp/storage/bootstrap/cache, /tmp/storage/framework/cache, /tmp/storage/framework/views, /tmp/storage/psysh
Running 'php artisan config:cache' to cache the Laravel configuration
INFO  Configuration cached successfully.  

These logs are useful for understanding how Bref is working within the Lambda runtime and whether the configuration of the Laravel application we have deployed is as intended.

However, this verbosity becomes superfluous when multiple cold starts occur in rapid succession, as the same messages are replicated each time. (For example, when Bref\LaravelBridge\Queue\QueueHandler handles a large number of "Push Notifications" sent simultaneously)

This option aims to provide flexibility in managing log verbosity based on the developers' needs during different stages of deployment and operation.

Thank you.

KentarouTakeda commented 7 months ago

This fix may solve some of the issues in #134.

The issue also mentions changes to the log output destination and format. Although these points are not supported by this Pull Request, it will prevent log output in a format other than Json from interfering with other programs.

/cc @micheleorselli

KentarouTakeda commented 7 months ago

@tillkruss @georgeboot I think I have corrected all the points you have pointed out. Did you have any other problems?

georgeboot commented 7 months ago

I like the feature as is, but personally would flip the logic and make logging opt-out using environment variables. It's now technically a breaking change and only few users will care about it.

tillkruss commented 7 months ago

I like the feature as is, but personally would flip the logic and make logging opt-out using environment variables. It's now technically a breaking change and only few users will care about it.

Agreed.

KentarouTakeda commented 7 months ago

Fixed in https://github.com/brefphp/laravel-bridge/pull/143/commits/cf92542c151678ecffdeb706461a6b60d0abffc4

For details such as the name, I have enabled "Allow edits and access to secrets by maintainers" setting in the Pull Request, so feel free to modify it in any way you like.

KentarouTakeda commented 7 months ago

Thanks!