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

Replace compiled view paths in profiles #747

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/Sentry/Laravel/Profiling/FrameProcessor.php
@@ -0,0 +1,40 @@
<?php

namespace Sentry\Laravel\Profiling;

use Illuminate\Support\Str;
use Sentry\Laravel\Tracing\BacktraceHelper;

class FrameProcessor
{
/**
* @param array $frame
*
* @return array
*/
public function __invoke(array $frame): array
{
// Check if we are dealing with a frame for a cached view path
if (Str::startsWith($frame['filename'], '/storage/framework/views/')) {
$originalViewPath = $this->backtraceHelper()->getOriginalViewPathForCompiledViewPath($frame['abs_path']);
Copy link
Member

Choose a reason for hiding this comment

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

Could we benchmark the perf overhead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we could but not sure how to quantify, since this does a few I/O operations to read the view file. In this WIP I didn't implement memoizing which could reduce the amount of operations needed when a view is seen multiple times. I could run this code a few iterations on my machine and see what the result is, but it's always an increase since before we didn't do anything.

Copy link
Member

Choose a reason for hiding this comment

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

A rough ballpark would suffice.


if ($originalViewPath !== null) {
// For views both the filename and function is the view path so we set them both to the original view path
$frame['filename'] = $frame['function'] = $originalViewPath;
}
}

return $frame;
}

private function backtraceHelper(): BacktraceHelper
{
static $helper = null;

if ($helper === null) {
$helper = app(BacktraceHelper::class);
}

return $helper;
}
}
16 changes: 14 additions & 2 deletions src/Sentry/Laravel/Tracing/BacktraceHelper.php
Expand Up @@ -75,12 +75,24 @@ public function getOriginalViewPathForFrameOfCompiledViewPath(Frame $frame): ?st
return null;
}

return $this->getOriginalViewPathForCompiledViewPath($frame->getAbsoluteFilePath());
}

/**
* Takes a path and if it's a compiled view returns the original view path.
*
* @param string $suspectedViewPath
*
* @return string|null
*/
public function getOriginalViewPathForCompiledViewPath(string $suspectedViewPath): ?string
{
// If for some reason the file does not exists, skip resolving
if (!file_exists($frame->getAbsoluteFilePath())) {
if (!file_exists($suspectedViewPath)) {
return null;
}

$viewFileContents = file_get_contents($frame->getAbsoluteFilePath());
$viewFileContents = file_get_contents($suspectedViewPath);

preg_match('/PATH (?<originalPath>.*?) ENDPATH/', $viewFileContents, $matches);

Expand Down
4 changes: 4 additions & 0 deletions src/Sentry/Laravel/Tracing/ServiceProvider.php
Expand Up @@ -16,8 +16,10 @@
use InvalidArgumentException;
use Laravel\Lumen\Application as Lumen;
use Sentry\Laravel\BaseServiceProvider;
use Sentry\Laravel\Profiling\FrameProcessor;
use Sentry\Laravel\Tracing\Routing\TracingCallableDispatcherTracing;
use Sentry\Laravel\Tracing\Routing\TracingControllerDispatcherTracing;
use Sentry\Profiling\Profile;
use Sentry\Serializer\RepresentationSerializer;

class ServiceProvider extends BaseServiceProvider
Expand Down Expand Up @@ -56,6 +58,8 @@ public function boot(): void
$httpKernel->prependMiddleware(Middleware::class);
}
}

Profile::setFrameProcessor(new FrameProcessor);
}

public function register(): void
Expand Down