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

Feat performance #145

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
20 changes: 19 additions & 1 deletion Helper/Data.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class Data extends AbstractHelper
'js_sdk_version',
'tracing_enabled',
'tracing_sample_rate',
'performance_tracking_enabled',
'performance_tracking_excluded_areas',
'profiles_sample_rate',
'ignore_js_errors',
];

Expand Down Expand Up @@ -93,6 +96,11 @@ public function getTracingSampleRate(): float
return (float) $this->config['tracing_sample_rate'] ?? 0.2;
}

public function getPhpProfileSampleRate(): float
{
return (float) ($this->config['profiles_sample_rate'] ?? 0);
}

/**
* @return array|null
*/
Expand Down Expand Up @@ -272,10 +280,20 @@ public function isPhpTrackingEnabled(): bool
return $this->scopeConfig->isSetFlag(static::XML_PATH_SRS.'enable_php_tracking');
}

public function isPerformanceTrackingEnabled(): bool
{
return $this->isTracingEnabled() && $this->config['performance_tracking_enabled'] ?? false;
}

public function getPerformanceTrackingExcludedAreas(): array
{
return $this->config['performance_tracking_excluded_areas'] ?? ['adminhtml', 'crontab'];
}

/**
* @return bool
*/
public function useScriptTag(): bool
public function useScriptTag()
{
return $this->scopeConfig->isSetFlag(static::XML_PATH_SRS.'enable_script_tag');
}
Expand Down
33 changes: 33 additions & 0 deletions Model/PerformanceTracingDto.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php

declare(strict_types=1);

namespace JustBetter\Sentry\Model;

use Sentry\State\Scope;
use Sentry\Tracing\Span;

class PerformanceTracingDto
{
public function __construct(
private Scope $scope,
private ?Span $parentSpan = null,
private ?Span $span = null
) {
}

public function getScope(): Scope
{
return $this->scope;
}

public function getParentSpan(): ?Span
{
return $this->parentSpan;
}

public function getSpan(): ?Span
{
return $this->span;
}
}
22 changes: 19 additions & 3 deletions Model/SentryInteraction.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,36 @@

// phpcs:disable Magento2.Functions.DiscouragedFunction

use JustBetter\Sentry\Helper\Data;
use Throwable;

use function Sentry\captureException;
use function Sentry\init;

class SentryInteraction
{
public function initialize($config)
public function __construct(
private Data $sentryHelper
) {
}

public function initialize($config): void
{
init($config);
}

public function captureException(\Throwable $ex)
public function captureException(Throwable $ex): void
{
if (!$this->sentryHelper->shouldCaptureException($ex)) {
return;
}

ob_start();
captureException($ex);

try {
captureException($ex);
} catch (Throwable) {
}
ob_end_clean();
}
}
149 changes: 149 additions & 0 deletions Model/SentryPerformance.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,149 @@
<?php

declare(strict_types=1);

namespace JustBetter\Sentry\Model;

// phpcs:disable Magento2.Functions.DiscouragedFunction

use JustBetter\Sentry\Helper\Data;
use Laminas\Http\Response;
use Magento\Framework\App\Http;
use Magento\Framework\App\Request\Http as HttpRequest;
use Magento\Framework\App\ResponseInterface;
use Magento\Framework\App\State;
use Magento\Framework\AppInterface;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\ObjectManagerInterface;
use Sentry\SentrySdk;
use Sentry\Tracing\SpanContext;
use Sentry\Tracing\Transaction;
use Sentry\Tracing\TransactionContext;
use Sentry\Tracing\TransactionSource;
use Throwable;

use function Sentry\startTransaction;

class SentryPerformance
{
private ?Transaction $transaction = null;

public function __construct(
private HttpRequest $request,
private ObjectManagerInterface $objectManager,
private Data $helper
) {
}

public function startTransaction(AppInterface $app): void
{
if (!$app instanceof Http) {
// actually, we only support profiling of http requests.
return;
}

$requestStartTime = $this->request->getServer('REQUEST_TIME_FLOAT', microtime(true));

$context = TransactionContext::fromHeaders(
$this->request->getHeader('sentry-trace') ?: '',
$this->request->getHeader('baggage') ?: ''
);

$requestPath = '/'.ltrim($this->request->getRequestUri(), '/');

$context->setName($requestPath);
$context->setSource(TransactionSource::url());
$context->setStartTimestamp($requestStartTime);

$context->setData([
'url' => $requestPath,
'method' => strtoupper($this->request->getMethod()),
]);

// Start the transaction
$transaction = startTransaction($context);
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure. Starting a transaction here won't count towards your Sentry usage and limits right?
Since we'll be executing it every request

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no. it only counts if the report is sent to the gateway (at the latest moment)

But i did not tested it explicitly, because we do not have any limits.


// If this transaction is not sampled, don't set it either and stop doing work from this point on
if (!$transaction->getSampled()) {
return;
}

$this->transaction = $transaction;
SentrySdk::getCurrentHub()->setSpan($transaction);
}

public function finishTransaction(ResponseInterface|int $statusCode): void
Copy link
Member

Choose a reason for hiding this comment

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

Should we maybe wrap this in a try/catch as well like captureException so issues when collecting performance metrics can't interfere with rendering the page to a customer

{
if ($this->transaction === null) {
return;
}

try {
$state = $this->objectManager->get(State::class);
$areaCode = $state->getAreaCode();
} catch (LocalizedException) {
// we wont track transaction without an area
return;
}

if (in_array($areaCode, $this->helper->getPerformanceTrackingExcludedAreas())) {
return;
}

if ($statusCode instanceof Response) {
$statusCode = (int) $statusCode->getStatusCode();
}

if (is_numeric($statusCode)) {
$this->transaction->setHttpStatus($statusCode);
}

if (in_array($state->getAreaCode(), ['frontend', 'webapi_rest', 'adminhtml'])) {
$this->transaction->setOp('http');

$this->transaction->setData(array_merge(
$this->transaction->getData(),
$this->request->__debugInfo(),
[
'module' => $this->request->getModuleName(),
'action' => $this->request->getFullActionName(),
]
));
} elseif ($state->getAreaCode() === 'graphql') {
$this->transaction->setOp('graphql');
} else {
$this->transaction->setOp($state->getAreaCode());
}
Comment on lines +116 to +120
Copy link
Member

Choose a reason for hiding this comment

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

Was there anything special that needed to happen here? If not we can remove the elseif

Suggested change
} elseif ($state->getAreaCode() === 'graphql') {
$this->transaction->setOp('graphql');
} else {
$this->transaction->setOp($state->getAreaCode());
}
} else {
$this->transaction->setOp($state->getAreaCode());
}


try {
// Finish the transaction, this submits the transaction and it's span to Sentry
$this->transaction->finish();
} catch (Throwable) {
}

$this->transaction = null;
}

public static function traceStart(SpanContext $context): PerformanceTracingDto
{
$scope = SentrySdk::getCurrentHub()->pushScope();
$span = null;

$parentSpan = $scope->getSpan();
if ($parentSpan !== null && $parentSpan->getSampled()) {
$span = $parentSpan->startChild($context);
$scope->setSpan($span);
}

return new PerformanceTracingDto($scope, $parentSpan, $span);
}

public static function traceEnd(PerformanceTracingDto $context): void
{
if ($context->getSpan()) {
$context->getSpan()->finish();
$context->getScope()->setSpan($context->getParentSpan());
}
SentrySdk::getCurrentHub()->popScope();
}
}
47 changes: 24 additions & 23 deletions Plugin/GlobalExceptionCatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,25 @@

// phpcs:disable Magento2.CodeAnalysis.EmptyBlock

use JustBetter\Sentry\Helper\Data as SenteryHelper;
use JustBetter\Sentry\Helper\Data as SentryHelper;
use JustBetter\Sentry\Model\ReleaseIdentifier;
use JustBetter\Sentry\Model\SentryInteraction;
use JustBetter\Sentry\Model\SentryPerformance;
use Magento\Framework\AppInterface;
use Magento\Framework\DataObject;
use Magento\Framework\DataObjectFactory;
use Magento\Framework\Event\ManagerInterface as EventManagerInterface;
use Throwable;

class GlobalExceptionCatcher
{
/**
* ExceptionCatcher constructor.
*
* @param SenteryHelper $sentryHelper
* @param ReleaseIdentifier $releaseIdentifier
* @param SentryInteraction $sentryInteraction
* @param EventManagerInterface $eventManager
* @param DataObjectFactory $dataObjectFactory
*/
public function __construct(
protected SenteryHelper $sentryHelper,
private SentryHelper $sentryHelper,
private ReleaseIdentifier $releaseIdentifier,
private SentryInteraction $sentryInteraction,
private EventManagerInterface $eventManager,
private DataObjectFactory $dataObjectFactory
private DataObjectFactory $dataObjectFactory,
private SentryPerformance $sentryPerformance
) {
}

Expand All @@ -37,6 +32,7 @@ public function aroundLaunch(AppInterface $subject, callable $proceed)
return $proceed();
}

/** @var DataObject $config */
$config = $this->dataObjectFactory->create();

$config->setDsn($this->sentryHelper->getDSN());
Expand All @@ -59,24 +55,29 @@ public function aroundLaunch(AppInterface $subject, callable $proceed)
return $data->getEvent();
});

if ($this->sentryHelper->isPerformanceTrackingEnabled()) {
$config->setTracesSampleRate($this->sentryHelper->getTracingSampleRate());
}

if ($rate = $this->sentryHelper->getPhpProfileSampleRate()) {
$config->setData('profiles_sample_rate', $rate);
}

$this->eventManager->dispatch('sentry_before_init', [
'config' => $config,
]);

$this->sentryInteraction->initialize($config->getData());
$this->sentryPerformance->startTransaction($subject);

try {
return $proceed();
} catch (\Throwable $ex) {
try {
if ($this->sentryHelper->shouldCaptureException($ex)) {
$this->sentryInteraction->captureException($ex);
}
} catch (\Throwable $bigProblem) {
// do nothing if sentry fails
}

throw $ex;
return $response = $proceed();
} catch (Throwable $exception) {
$this->sentryInteraction->captureException($exception);

throw $exception;
} finally {
$this->sentryPerformance->finishTransaction($response ?? 500);
}
}
}
35 changes: 35 additions & 0 deletions Plugin/Profiling/DbQueryLoggerPlugin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php

declare(strict_types=1);

namespace JustBetter\Sentry\Plugin\Profiling;

use JustBetter\Sentry\Model\PerformanceTracingDto;
use JustBetter\Sentry\Model\SentryPerformance;
use Magento\Framework\DB\LoggerInterface;
use Sentry\Tracing\SpanContext;

class DbQueryLoggerPlugin
{
private ?PerformanceTracingDto $tracingDto = null;

public function beforeStartTimer(LoggerInterface $subject): void
{
$this->tracingDto = SentryPerformance::traceStart(
SpanContext::make()
->setOp('db.sql.query')
->setStartTimestamp(microtime(true))
);
}

public function beforeLogStats(LoggerInterface $subject, $type, $sql, $bind = [], $result = null): void
{
if ($this->tracingDto === null) {
return;
}

$this->tracingDto->getSpan()?->setDescription($sql);
SentryPerformance::traceEnd($this->tracingDto);
$this->tracingDto = null;
}
}
Loading
Loading