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

Upgrade Sentry to sentry-php 4 #90

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

Firesphere
Copy link

  • Looking at also including SQL tracing

@phptek
Copy link
Owner

phptek commented Dec 12, 2023

Thanks for this. The last time I bumped the SDK version, Sentry had refactored their API which took me ages to refactor for this module.

How much testing have you done with this change in place? FYI there are x2 ways for the module to send a payload to sentry each of which uses a slightly different part of the Sentry API:

  1. Throwing an exception
  2. Using the error(), warning(), info() (etc) methods (See the usage docs)

@Firesphere
Copy link
Author

The SDK from sentry (sentry/sdk) is just a shim around the actual sentry reporting.
I've started it, but due to circumstances haven't been able to fully test it yet.
I should have some time now to wrap it up and do a full test :)

@phptek phptek mentioned this pull request Apr 22, 2024
@lekoala
Copy link
Contributor

lekoala commented Apr 22, 2024

here is a quick snippet from one of my projects using the sdk v4

  • ability to set a debug log in case the http client fails (eg: ssl certificate errors)
  • with v4, curl is always used. without a proper curl.cainfo, it's not going to work, therefore setting http_ssl_verify_peer to false might be interesting (it's less secure, but should not happen on a properly configured server)
  • the before send hook is interesting where you can filter the stack trace and/or add a figerprint which can be interesting to group similar errors
      Sentry\init([
            'dsn' => $sentryDSN,
            'environment' => Director::get_environment_type(),
            'attach_stacktrace' => true,
            'logger' => Director::isDev() ? new \Sentry\Logger\DebugFileLogger(filePath: Director::baseFolder() . '/sentry.log') : null,
            'http_ssl_verify_peer' => ini_get('curl.cainfo') ? true : false, // without this, it fails to send request on local
            'before_send' => function (\Sentry\Event $event, ?\Sentry\EventHint $hint): \Sentry\Event {
                if ($hint !== null && $hint->exception !== null && str_contains($hint->exception->getMessage(), 'database unavailable')) {
                    $event->setFingerprint(['database-unavailable']);
                }

                // Filter stacktrace
                $st = $event->getStacktrace();
                if ($st) {
                    $removed = 0;
                    foreach ($st->getFrames() as $idx => $frame) {
                        if (str_starts_with($frame->getFunctionName() ?? "", 'Sentry')) {
                            $st->removeFrame($idx - $removed);
                            $removed++;
                        }
                    }
                    $event->setStacktrace($st);
                }

                return $event;
            },
        ]);

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

Successfully merging this pull request may close these issues.

None yet

3 participants