search

getsentry / sentry-laravel

The official Laravel SDK for Sentry (sentry.io)
https://sentry.io
MIT License
1.26k stars 189 forks source link

Laravel Pennant performance spans & breadcrumbs #655

Open cleptric opened 1 year ago

cleptric commented 1 year ago

Laravel Pennant is a new feature flagging library for Laravel.

There are currently two events dispatched, FeatureRetrieved and FeatureResolved.

We could try contributing new FeatureRetrieve and FeatureResolve events upstream, or use decorators, so we are able to wrap the code or DB query executed into a span, containing the feature identifier and result.

martinbean commented 1 year ago

@cleptric I’m happy to create the code and PR for this if you would like?

cleptric commented 1 year ago

@martinbean Hey, thanks a lot! In order to create meaningful spans & breadcrumbs, we would need new events added to pennant, that fire before a feature is retrieved, see https://github.com/laravel/pennant/issues/41.

martinbean commented 1 year ago

@cleptric Why would Sentry need an event before a feature is retrieved? Would you not want to add a breadcrumb after a feature and its value has been resolved?

cleptric commented 1 year ago

We already trace all DB queries related to retrieving a feature, so the only addition here would be to wrap these queries into a new span and create a breadcrumb before the DB queries are run. This is mostly about better UX in the product.

stayallive commented 1 year ago

In addition to @cleptric their comments, having an event before & after means we know when the framework started and finished resolving the feature flag, meaning we know how long it took and which (if any) database queries or other expensive operations took place while resolving the flag. This gives much insights in the timing. So for just breadcrumbs the "after" is enough but for the performance spans the "before" is a must.