sezzle / sezzle-magento2

Apache License 2.0
5 stars 3 forks source link

Invalid usage of PSR `LoggerInterface`, breaking the log aggregation #28

Open lbajsarowicz opened 3 days ago

lbajsarowicz commented 3 days ago

Your current log implementation incorrectly uses PSR LoggerInterface, causing excessive log message volume, that log aggregation tools can't process correctly.

https://github.com/sezzle/sezzle-magento2/blob/c7825f3bf438eccbf4a084bf823f07d22b10e789/Helper/Data.php#L98-L114

When you take a closer look into the \Psr\Log\LoggerInterface::info, you can notice there's a 2nd argument called $context:

    /**
     * Interesting events.
     *
     * Example: User logs in, SQL logs.
     *
     * @param string|\Stringable $message
     * @param mixed[] $context
     *
     * @return void
     */
    public function info(string|\Stringable $message, array $context = []);

So the 1st argument $message should explain what happened, the second one - technical / unique details. For example:

$this->logger->error('Cannot place order for payment method X', [
    'quote_id' => $quote->getId(),
    'customer_id' => $quote->getCustomerId(),
    ...
]);

As a result, the same logs are aggregated correctly in Sentry / NewRelic / DataDog etc.

arijit-sezzle commented 3 days ago

Thanks for reporting the issue. I get what you meant, but the $context parameter inside the function is optional, so I guess it was never a requirement to pass a value through it and so, we did not do that.

Can you please give us more context or show us an example where you see its logging in excessive volume? Just trying to understand how it is logging in excessive volume.