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

Add support to filter out events #89

Closed
peterjaap opened this issue May 16, 2022 · 12 comments
Closed

Add support to filter out events #89

peterjaap opened this issue May 16, 2022 · 12 comments

Comments

@peterjaap
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor Author

Yes sentry_before_send would be great.

@peterjaap
Copy link
Contributor Author

peterjaap commented May 17, 2022

Example extension;

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

<?xml version="1.0" ?>
<config xmlns:xsi="https://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
Copy link
Contributor Author

peterjaap commented May 17, 2022

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
Copy link

@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
Copy link
Contributor Author

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

@maxacarvalho
Copy link

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="https://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="https://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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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="https://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
Copy link
Member

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
#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
Copy link
Member

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
Copy link
Contributor Author

@indykoning nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants