Skip to content

Conversation

@HazAT
Copy link
Member

@HazAT HazAT commented Jun 23, 2020

Installation

Read
https://docs.sentry.io/performance-monitoring/performance/
to understand the concept behind Tracing/Performance in Sentry.

Make sure you have following in you composer.json

"require": {
        "sentry/sentry-laravel": "dev-performance/beta"
},
"minimum-stability": "dev",
"prefer-stable": true,

After that run composer update.

Make sure the installation of sentry/sentry, sentry/sdk and sentry/sentry-laravel in the output looks something like this:

 - Installing sentry/sentry (3.x-dev dd6ef34)
 - Installing sentry/sdk (3.x-dev f6b434e)
 - Installing sentry/sentry-laravel (dev-performance/beta 745bd72)

The important part is that both sentry/sentry and sentry/sdk are 3.x where sentry/sentry-laravel is dev-performance/beta.

After that add 'traces_sample_rate' => 1.0, to the file in config/sentry.php.
Note that the value should be larger than 0.0 and smaller or equal than 1.0 (to send everything).
More info can be found here: https://docs.sentry.io/performance-monitoring/getting-started/

Connecting your frontend

To setup tracing for your Vue frontend, follow: https://docs.sentry.io/performance-monitoring/getting-started/?platform=vue

An additional step you should take is, add this to your blade template rendering the <head> tag {!! Sentry\Laravel\Integration::sentryTracingMeta() !!}
This adds a meta tag similar to <meta name="sentry-trace" content="ae02a316231d490c870b14f27aba8d29-c4a72ffd39444c84-1"/> to your header and tells the frontend code to pick up the trace.

Known Issues

  • Right now we are not using an agent to send those transactions meaning Laravel sends the Transactions after finishing serving the request but the process is only terminated once the Transaction is sent. It shouldn't impact your users a lot but something to keep in mind.
  • If you are using on-premise you need to make sure to use the latest release of Sentry https://github.com/getsentry/sentry/releases/tag/20.6.0 or master. Transactions hit a new endpoint and can only be received there.

Papertrail

  • Record sql queries as spans
  • Record rendered views as spans
  • Get route name like /users/{id} instead of /users/1
  • Start Transaction from incoming headers
  • Collect query span regardless of sentry.breadcrumbs.sql_queries option?

This works together with getsentry/sentry-php#1031

Resulting in:

image

Record sql queries as spans
@dmyers
Copy link

dmyers commented Jun 23, 2020

I was just reading about this today on Sentry’s docs and was hoping PHP and Laravel would be added as the docs just have Python and JavaScript. This would be fantastic!

Would the terminable option be useful for this? https://laravel.com/docs/7.x/middleware#terminable-middleware

@HazAT
Copy link
Member Author

HazAT commented Jun 24, 2020

@dmyers Yes PHP/Laravel will be supported next :)

That's a good point, maybe we should finish the transaction in terminate.
If you have more input or idea how and what to measure feel free to comment / help :)
(Updated the screenshot)

I was also trying to measure/instrument the 217ms gap but I am not sure what's causing this. It's not the middleware, I guess the ServiceProviders but not sure how to measure them.

@stayallive stayallive changed the title feat: Add Tracing Middleware feat: Add Tracing Jun 24, 2020
@markstaun
Copy link

Just tried this out. Seems to be working when I'm browsing our app, however, I'm getting the following error within PHPUnit tests. (50% of all tests are failing)

164) Tests\Unit\Liquid\RequestTest::has_page_type_404
Error: Call to a member function startChild() on null

/Users/marknoergaard/code/codefort-one/vendor/sentry/sentry-laravel/src/Sentry/Laravel/TracingViewEngineDecorator.php:44
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/View/View.php:139
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/View/View.php:122
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/View/View.php:91
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Http/Response.php:62
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Http/Response.php:34
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Routing/ResponseFactory.php:55
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Routing/ResponseFactory.php:85
/Users/marknoergaard/code/codefort-one/app/Exceptions/Handler.php:145
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Foundation/Exceptions/Handler.php:313
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Foundation/Exceptions/Handler.php:210
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php:415
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php:113
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php:507
/Users/marknoergaard/code/codefort-one/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php:291
/Users/marknoergaard/code/codefort-one/tests/Unit/Liquid/RequestTest.php:143

I would also I would love to have an option to silence any error a just report them if the tracer is failing. Even if it's failing in the terminate middleware this would result in the complete request failing if using Laravel Vapor, as the terminate is called before the response is returned.

@HazAT
Copy link
Member Author

HazAT commented Jul 17, 2020

@markstaun Thanks for the feedback, we'll fix this.

@markstaun
Copy link

Sounds great, really looking forward to deploying this to production!

@Josh-G
Copy link

Josh-G commented Jul 21, 2020

if you aren't seeing view.render spans; we had to add sentry/sentry-laravel to our dont-discover array as other service providers were resolving view.engine.resolver prior to the Tracing service provider being registered. To resolve, we manually corrected the service provider order in our config.

alternatively, Sentry could check if the view engine resolver has already been resolved in register ($this->app->resolved('view.engine.resolver'))

other than that, it works quite well for us, providing insight into our slowest rendering views 👍

@stayallive
Copy link
Collaborator

@Josh-G good one thanks for letting us know, could you show me a package that resolves the view engine so I can test with working around that? Having a hard time coming up with one.

@markstaun I believe the issue in testing should be fixed!

@Josh-G
Copy link

Josh-G commented Jul 21, 2020

could you show me a package that resolves the view engine

@stayallive One of the two service providers in hyn/multi-tenant ends up resolving the view engine (I didn't go as far as to determine exactly which provider, just the package that was conflicting).

@stayallive
Copy link
Collaborator

@Josh-G, thanks, I'll figure it out from here, I'll let you know once it's fixed 👍

@stayallive
Copy link
Collaborator

Actually @Josh-G, I think I already got it, let me know how this works for you!

@Josh-G
Copy link

Josh-G commented Jul 22, 2020

@stayallive Perfect, thanks! Have tested and no longer requires a specific service provider order :) works out of the box

@archon810
Copy link

Anything special that is needed to get this to work for WordPress?

@stayallive
Copy link
Collaborator

stayallive commented Aug 6, 2020

This code is specific to Laravel and will not work for any other platform.

The PHP SDK is currently in the works to get all tracing related code integrated (getsentry/sentry-php#1031) and once that is stable other integrations could start implementing it.

To instrument WordPress the WordPress integration will need to write a similar kind of code hooking into parts of WordPress to get some meaningful tracing information.

@archon810
Copy link

@stayallive Should I start a ticket for that or is official WordPress PHP Tracing/Performance support not expected?

@stayallive
Copy link
Collaborator

@archon810 you are ofcourse free to open a ticket to track the interest but the WordPress integration is technically not an official integration while PHP, Laravel (and Symfony) are so it might take a bit longer.

@markstaun
Copy link

Just tested this branch again with our codebase, but some of our tests are failing.

1) Tests\Feature\Admin\AdminLoginTest::testSystemAdminCantAccessShops
Sentry\Exception\EventCreationException: Unable to instantiate an event

/Users/user/code/laravel/vendor/sentry/sentry/src/EventFactory.php:114
/Users/user/code/laravel/vendor/sentry/sentry/src/Client.php:184
/Users/user/code/laravel/vendor/sentry/sentry/src/Client.php:112
/Users/user/code/laravel/vendor/sentry/sentry/src/Client.php:104
/Users/user/code/laravel/vendor/sentry/sentry/src/State/Hub.php:134
/Users/user/code/laravel/app/Exceptions/Handler.php:78
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php:403
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php:111
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php:508
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php:292
/Users/user/code/laravel/tests/Feature/Admin/AdminLoginTest.php:37

And here is the actual exception that is catched in the EventFactory.php:114.

1) Tests\Feature\Admin\AdminLoginTest::testSystemAdminCantAccessShops
ReflectionException: Method Illuminate\Pipeline\Pipeline::__call() does not exist

/Users/user/code/laravel/vendor/sentry/sentry/src/FrameBuilder.php:178
/Users/user/code/laravel/vendor/sentry/sentry/src/FrameBuilder.php:87
/Users/user/code/laravel/vendor/sentry/sentry/src/StacktraceBuilder.php:66
/Users/user/code/laravel/vendor/sentry/sentry/src/StacktraceBuilder.php:46
/Users/user/code/laravel/vendor/sentry/sentry/src/EventFactory.php:147
/Users/user/code/laravel/vendor/sentry/sentry/src/EventFactory.php:102
/Users/user/code/laravel/vendor/sentry/sentry/src/Client.php:184
/Users/user/code/laravel/vendor/sentry/sentry/src/Client.php:112
/Users/user/code/laravel/vendor/sentry/sentry/src/Client.php:104
/Users/user/code/laravel/vendor/sentry/sentry/src/State/Hub.php:134
/Users/user/code/laravel/app/Exceptions/Handler.php:78
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php:403
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php:111
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php:508
/Users/user/code/laravel/vendor/laravel/framework/src/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php:292
/Users/user/code/laravel/tests/Feature/Admin/AdminLoginTest.php:37

@HazAT
Copy link
Member Author

HazAT commented Aug 28, 2020

@markstaun I fixed this in the latest commit in PHP repo getsentry/sentry-php@67f15ce

@markstaun
Copy link

Thanks, I can confirm that it's working and all our tests are now passing 👌

@HazAT HazAT changed the base branch from master to 2.x September 3, 2020 08:19
@HazAT HazAT merged commit 69b666a into 2.x Sep 3, 2020
@HazAT HazAT deleted the performance/beta branch September 3, 2020 08:20
HazAT added a commit that referenced this pull request Sep 28, 2020
* feat: Add Tracing (#358)

* feat: Add Tracing Middleware

Record sql queries as spans

* feat: Auto prepend middleware

Instrument view renders

* ref: Change code comments

* ref: Use terminate to send transaction

* Rename view engine decorator

* Improve transaction context name/description

* Prevent crashes when using missing defines

* Do not remove all leading `/` keep 1

* Add fallback autoload + bootstrap span

* Set the transaction name from the event handler

* Cleanup query span

* Prevent errors on Laravel 5.3 and below

* CS

* feat: Use correct route and add data

* ref: Add name

* fix: Route naming

* feat: Start transaction in serviceProvider

Move all renders as a child

* ref: Rename to view.render

* ref: Move back to starting transaction in middleware

Use fromTraceparent function to start a trace

* ref: Small refactor

* feat: Update composer.json

* ref: Add traces_sample_rate

* Refactor active span retrieval

* Guard against the active span not being set

* Docblock updates

* Correctly return rendered view without span

* Move tracing related code to the Tracing namespace

* Improve the route name detection order

* Do not use route names ending with a `.`

* Fix not wrapping the view enines when resolver is already resolved

* feat: Rework code to use transaction on the hub

Co-authored-by: Alex Bouma <[email protected]>

* meta: Bump version 2.0.0-beta.0

* meta: Change version 2.0.0-beta1

* meta: Define beta version 3.0.0-beta1 for php sdk

* meta: Changelog

* Update CHANGELOG.md

Co-authored-by: Alex Bouma <[email protected]>

* feat: Add publish command (#379)

* feat: Add publish command

* fix: Typos

* feat: Counterpart to PHP polish apm pr (#384)

* meta: Prepare 2.x

* fix: Travis

* ref: Fix tests

* fix: tests

* ref: Master

Co-authored-by: Alex Bouma <[email protected]>
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.

7 participants