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.

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

A specific log file #51

Merged
merged 2 commits into from
Aug 13, 2024
Merged

A specific log file #51

merged 2 commits into from
Aug 13, 2024

Conversation

WaPoNe
Copy link
Contributor

@WaPoNe WaPoNe commented Jul 29, 2024

A specific log file.

@convenient
Copy link
Contributor

Hello @WaPoNe

Thanks for the PR, I hope you don't mind me asking.

I'm not fully sure of the benefit here, when the magento 2 application tried to move to "log levels" with specific logs for debug/system/critical to cut down on the old magento1 case of there being 20 log files written to every day. To me that ISO level system makes much more sense.

It doesn't really make much difference (in my mind) either way you're monitoring and reviewing your logs

  • Cloud log management system - it deals with aggregation and filtering so not much change there either
  • CLI - you have already tools like grep for monitoring/filtering logs. There's also other CLI tools which will handle the aggregation and filtering in a more advanced way, i cant think of any of the names right now because I don't use them and mostly use cloud.

Using a different file to separate logs, seems like it's trying to solve a problem that should be handled elsewhere, to better effect?

@WaPoNe
Copy link
Contributor Author

WaPoNe commented Aug 2, 2024

Hello @convenient,
I think it's just neater.

If the module writes logs, I simply have to check the specific log file instead of having to read and filter through more generic files like system.log or exception.log

Copy link
Contributor

@convenient convenient left a comment

Choose a reason for hiding this comment

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

Okay thanks. if this is something you need its probably something other people need so this is grand. I'm happy to proceed with this.

Can you please have a look at it though, i'm 99% certain that we don't need to define the CustomLogger class and this is (all?) possible via di.xml


use Magento\Framework\Logger\Monolog;

class CustomLogger extends Monolog
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe its possible to inject the logger as a virtual type? So that we don't have the extra class?

Copy link
Contributor

Choose a reason for hiding this comment

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

@convenient
Copy link
Contributor

convenient commented Aug 3, 2024

EDIT: Never mind, i spotted the issue here c1c6c66 the log handler level needed changed

Hello @WaPoNe

I have pulled this in and captured it in a test in #55

Most of this is looking great 👌 I can see the logs being written to the custom file just fine. I have one test at the end which is being flagged as off though

It seems that the log is being written to the custom file, as well as the system.log. I am just checking if that is the intended situation here?

Verifying that the log is not present in system.log
[2024-08-03T07:05:02.636242+00:00] main.INFO: gene encryption manager - legacy decryption {"trace_id":"29dbea682f3e24653ad5233c871848a2","trace":"|#0 vendor/magento/framework/Interception/Interceptor.php(146): Gene\\EncryptionKeyManager\\Plugin\\LogDecrypts->afterDecrypt()|#1 vendor/magento/framework/Interception/Interceptor.php(153): Magento\\Framework\\Encryption\\Encryptor\\Interceptor->Magento\\Framework\\Interception\\{closure}()|#2 generated/code/Magento/Framework/Encryption/Encryptor/Interceptor.php(23): Magento\\Framework\\Encryption\\Encryptor\\Interceptor->___callPlugins()|#3 phar://vendor/n98/magerun2-dist/n98-magerun2/src/N98/Magento/Command/Developer/DecryptCommand.php(63): Magento\\Framework\\Encryption\\Encryptor\\Interceptor->decrypt()|#4 phar://vendor/n98/magerun2-dist/n98-magerun2/vendor/symfony/console/Command/Command.php(298): N98\\Magento\\Command\\Developer\\decryptCommand->execute()|#5 phar://vendor/n98/magerun2-dist/n98-magerun2/src/N98/Magento/Command/AbstractMagentoCommand.php(249): Symfony\\Component\\Console\\Command\\Command->run()|#6 phar://vendor/n98/magerun2-dist/n98-magerun2/vendor/symfony/console/Application.php(1058): N98\\Magento\\Command\\AbstractMagentoCommand->run()|#7 phar://vendor/n98/magerun2-dist/n98-magerun2/vendor/symfony/console/Application.php(301): Symfony\\Component\\Console\\Application->doRunCommand()|#8 phar://vendor/n98/magerun2-dist/n98-magerun2/src/N98/Magento/Application.php(256): Symfony\\Component\\Console\\Application->doRun()|#9 phar://vendor/n98/magerun2-dist/n98-magerun2/vendor/symfony/console/Application.php(171): N98\\Magento\\Application->doRun()|#10 phar://vendor/n98/magerun2-dist/n98-magerun2/src/N98/Magento/Application.php(364): Symfony\\Component\\Console\\Application->run()|#11 vendor/n98/magerun2-dist/n98-magerun2(8): N98\\Magento\\Application->run()|#12 vendor/bin/n98-magerun2(120): include('...')|#13 {main}"} []
Error on line 288

@convenient convenient merged commit e04dd29 into genecommerce:master Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants