justbetter / magento2-sentry

Magento 2 module to log to Sentry
MIT License
165 stars 70 forks source link

Add support to filter out events #89

Closed peterjaap closed 7 months ago

peterjaap commented 2 years ago

Sometimes we encounter a bug but it takes a while for us to fix it (due to whatever reason).

We'd like to temporarily filter this event from turning up in Sentry. We could do this by ignoring the event in Sentry, but incoming events will still count towards our quota. Some of these events actually eat up almost all of our quota for a given month, so we want to filter them on the Magento end instead of in Sentry.

Sentry offers a basic way to filter incoming events but that isn't very granular, thereby sometimes accidentally filtering other events too.

We could pretty easily filter this inside PHP, see Filtering for PHP.

The way we do this now is creating a Composer patch that adds a bit of logic to vendor/justbetter/magento2-sentry/Plugin/GlobalExceptionCatcher.php. Like this on line 55 (right before $this->sentryInteraction->initialize($config););

        $config['before_send'] = function (\Sentry\Event $event, ?\Sentry\EventHint $hint): ?\Sentry\Event {
            // Ignore the event when the message includes a certain string
            if ($hint !== null && stripos($hint->exception->getMessage(), 'Illegal string offset \'attribute\'') !== false) {
                return null;
            }
            // Or ignore the event when it is in a specific file on a specific line
            if (
                $hint !== null
                && stripos($hint->exception->getFile(), 'vendor/magento/module-eav/Model/Entity/Collection/AbstractCollection.php') !== false
                && $hint->exception->getLine() === 373
            ) {
                return null;
            }
            // Or ignore the event if the original exception is an instance of MyException
            if ($hint !== null && $hint->exception instanceof MyException) {
                return null;
            }

            return $event;
        };

I'd like to introduce a user-friendlier way to achieve this. My suggestion would be to add an event like sentry_config that we could hook into to add something to the $config array (which then should be encapsulated in an object).

indykoning commented 2 years ago

I fully agree with the ability to filter out the errors in PHP and some of these are already in the Sentry module as well.

Currently we have errorexception_reporting allowing you to filter out Errors extending ErrorException by it's severity, it works the same way as error_reporting

And ignore_exceptions which you can use to exclude specific error classes like your last example.

While you could add a plugin on the initialize function and do it that way i believe events will make it more extendable. I'm also debating wether we should just fire a sentry_before_send event in the sentry module so it will be possible for multiple modules to filter without having to worry about overriding someone else's code.

peterjaap commented 2 years ago

Yes sentry_before_send would be great.

peterjaap commented 2 years ago

Example extension;

app/code/Elgentos/SentryBeforeSendHooks/etc/events.xml

<?xml version="1.0" ?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:Event/etc/events.xsd">
    <event name="sentry_before_init">
        <observer disabled="true" name="elgentos_sentrybeforeinithooks_observer_sentry_beforeinit_sentry_before_init" instance="Elgentos\SentryBeforeInitHooks\Observer\Sentry\BeforeInit"/>
    </event>
</config>

app/code/Elgentos/SentryBeforeInitHooks/Observer/Sentry/BeforeInit.php

<?php

declare(strict_types=1);

namespace Elgentos\SentryBeforeInitHooks\Observer\Sentry;

class BeforeInit implements \Magento\Framework\Event\ObserverInterface
{

    /**
     * Execute observer
     *
     * @param \Magento\Framework\Event\Observer $observer
     * @return void
     */
    public function execute(
        \Magento\Framework\Event\Observer $observer
    ) {
        $observer->getEvent()->getConfig()->setBeforeSend(function (\Sentry\Event $event, ?\Sentry\EventHint $hint): ?\Sentry\Event {
            $exceptionMessageToFilter = 'Illegal string offset \'attribute\'';
            if ($hint !== null && $hint->exception !== null && stripos($hint->exception->getMessage(), $exceptionMessageToFilter) !== false) {
                return null;
            }
            return $event;
        });
    }
}
peterjaap commented 2 years ago

We can apply a similar strategy for frontend events. We now override JustBetter_Sentry/templates/script/sentry.phtml in our theme and add this;

<script>
    Sentry.init({
        dsn: '<?= $block->escapeUrl(trim($block->getDSN())) ?>',
        release: '<?= $block->escapeHtml(trim($block->getVersion())) ?>',
        environment: '<?= $block->escapeHtml(trim($block->getEnvironment())) ?>',
        beforeSend: function(event) {
            // do some filter on the event here
            return event;
        }
    });
</script>

But there should be a cleaner way to approach this. Since this is client-side code, we could also create a field for this in the HTML, similar to the "Miscellaneous HTML" field in Magento.

convenient commented 1 year ago

@peterjaap Does ignoreErrors from the https://docs.sentry.io/clients/javascript/config/ satisfy some of this requirement?

ignoreErrors Very often, you will come across specific errors that are a result of something other than your application, or errors that you’re completely not interested in. ignoreErrors is a list of these messages to be filtered out before being sent to Sentry as either regular expressions or strings. When using strings, they’ll partially match the messages, so if you need to achieve an exact match, use RegExp patterns instead.

peterjaap commented 1 year ago

@convenient yes, but that's only for the Javascript part.

maxacarvalho commented 1 year ago

Hi there.

I saw that the PR to fire the event sentry_before_init was released, so that event is available to use.
I attempted to use it and filter out some of the exceptions that I don't want to reach Sentry but I don't think it's working. I'm most likely doing something wrong. I would appreciate some help if you have time for it.

Here's what I did:

app/code/MyVendor/SentryFilters/registration.php

<?php declare(strict_types=1);

use \Magento\Framework\Component\ComponentRegistrar;

ComponentRegistrar::register(ComponentRegistrar::MODULE, 'MyVendor_SentryFilters', __DIR__);

app/code/MyVendor/SentryFilters/etc/module.xml

<?xml version="1.0"?>
<config
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd"
>
    <module name="MyVendor_SentryFilters" setup_version="1.0.0">
        <sequence>
            <module name="JustBetter_Sentry"/>
        </sequence>
    </module>
</config>

app/code/MyVendor/SentryFilters/etc/events.xml

<?xml version="1.0" ?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:noNamespaceSchemaLocation="urn:magento:framework:Event/etc/events.xsd">
    <event name="sentry_before_init">
        <observer
            name="MyVendor_SentryFilters_Observer_Sentry_BeforeInit_sentry_before_init"
            instance="MyVendor\SentryFilters\Observer\Sentry\BeforeInit"
        />
    </event>
</config>

app/code/MyVendor/SentryFilters/Observer/Sentry/BeforeInit.php

<?php declare(strict_types=1);

namespace MyVendor\SentryFilters\Observer\Sentry;

class BeforeInit implements \Magento\Framework\Event\ObserverInterface
{
    private const FILTERED_EXCEPTION_MESSAGES = [
        'Environment emulation nesting is not allowed.',
        'Could not find a cart with ID',
        'ReCaptcha validation failed, please try again',
        'No such entity.',
        'The account sign-in was incorrect or your account is disabled temporarily. Please wait and try again later.',
    ];

    /**
     * Execute observer
     *
     * @param \Magento\Framework\Event\Observer $observer
     * @return void
     */
    public function execute(
        \Magento\Framework\Event\Observer $observer
    ) {
        $observer->getEvent()->getConfig()->setBeforeSend(function (\Sentry\Event $event, ?\Sentry\EventHint $hint): ?\Sentry\Event {
            foreach (self::FILTERED_EXCEPTION_MESSAGES as $exceptionMessageToFilter) {
                if ($hint !== null && $hint->exception !== null && stripos($hint->exception->getMessage(), $exceptionMessageToFilter) !== false) {
                    return null;
                }
            }

            return $event;
        });
    }
}
peterjaap commented 1 year ago

@maxacarvalho at first glance that looks fine to me. Did you xdebug to see the event actually gets fired and check what's in its payload?

Maybe you can try removing the getEvent() portion, there's no need for that afaik.

peterjaap commented 1 year ago

Here's the code that works for us;

# elgentos/magento2-sentry-before-send-hooks
Adds before_send hooks to Sentry module to filter out events on the Magento side

## Specifications

 - Observer
        - sentry_before_init > Elgentos\SentryBeforeInitHooks\Observer\Sentry\BeforeInit
<?xml version="1.0" ?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
        <module name="Elgentos_SentryBeforeInitHooks"/>
</config>
<?php

/**
 * Copyright ©  All rights reserved.
 * See COPYING.txt for license details.
 */

use Magento\Framework\Component\ComponentRegistrar;

ComponentRegistrar::register(ComponentRegistrar::MODULE, 'Elgentos_SentryBeforeInitHooks', __DIR__);
{
    "name": "elgentos/magento2-sentry-before-send-hooks",
    "description": "Adds before_send hooks to Sentry module to filter out events on the Magento side",
    "type": "magento2-module",
    "license": "GPL-3.0",
    "minimum-stability": "dev",
    "require": {},
    "autoload": {
        "files": [
            "registration.php"
        ],
        "psr-4": {
            "Elgentos\\SentryBeforeInitHooks\\": ""
        }
    }
}
<?php

declare(strict_types=1);

namespace Elgentos\SentryBeforeInitHooks\Observer\Sentry;

use Magento\Framework\Event\Observer;
use Magento\Framework\Event\ObserverInterface;
use Sentry\Event;
use Sentry\EventHint;

class BeforeInit implements ObserverInterface
{
    public function execute(
        Observer $observer
    ): void {
        $observer->getEvent()
            ->getData('config')
            ->setBeforeSend(
                function (Event $event, ?EventHint $hint): ?Event {
                    // Filter for https://sentry.io/organizations/elgentos/issues/3193092421/
                    $exceptionMessageToFilter = 'Illegal string offset \'attribute\'';

                    if (
                        $hint !== null &&
                        $hint->exception !== null &&
                        stripos($hint->exception->getMessage(), $exceptionMessageToFilter) !== false
                    ) {
                        return null;
                    }

                    return $event;
                }
            );
    }
}
indykoning commented 1 year ago

It looks like it should work indeed, is the observer being executed? Also a heads up for an upcoming release. I'm looking at merging this https://github.com/justbetter/magento2-sentry/pull/117

It'll allow multiple observers in different modules to be able to filter and change events before being sent. Shouldn't be breaking as setBeforeSend will take precedence, but it is good to know 🙂

indykoning commented 1 year ago

I've merged it in, it is in release 3.4.0 right now. With this installed you will be able to use observers to filter out events instead https://github.com/justbetter/magento2-sentry#change--filter-events

peterjaap commented 1 year ago

@indykoning nice!