Napp / xray-laravel

AWS X-Ray tracing for Laravel apps
MIT License
56 stars 34 forks source link

Allow request filtering #44

Closed nicDamours closed 8 months ago

nicDamours commented 10 months ago

Problem

Currently, all the incoming request are recorded. This can be problematic since they might have some request we wish not to be recorded. For example, we have a health check path that is used to determine whether the application is running correctly or not. This url is called once every 10 seconds or so, which creates a lot of noise in AWS X-Ray.

Proposed solution

There should have a way to do some sort of request filtering. Maybe by adding a callback that is used to check if the current request should be recorded or not. This callback could take the incoming request as an argument and return true or false.

Note. I will attempt to fix this problem and submit a pull request for review.

giagara commented 5 months ago

I've checked this PR but i cannot use this filter. If, as the documentation say, the Napp\Xray\XrayServiceProvider is the first of the list, how can i put the filter in the AppServiceProvider? The condition is loaded but never used.

Am I worng?

nicDamours commented 5 months ago

@giagara Since the XrayServiceProvider is already registered, when calling the register function of your AppServiceProvider, the necessary Xray Facade will be accessible.

This facade is used to add a callback that will be checked against the request, before attempting to capture it. We've used it for some times in production and it works fine. For reference, here is our providers config, in the config/app.php file.

<?php
return [
  /* ... */
  'providers' => [
     /*
      * Laravel Framework Service Providers...
      */
       Napp\Xray\XrayServiceProvider::class,
       Illuminate\Auth\AuthServiceProvider::class,
       /* ... */

       /*
        * Application Service Providers...
        */
       App\Providers\AppServiceProvider::class, // <- This is the class that register the filter
       App\Providers\AuthServiceProvider::class,
       App\Providers\EventServiceProvider::class,
       /* ... */
  ]
]

I'm not sure if this will help you. Perhaps you could verify the order of your laravel providers.

giagara commented 5 months ago

thanks @nicDamours for the quick reply. The facade is available and i have no errors with it. My though was about the order but my issue is that i was putting that method in the boot method and not in the register. My fault, sorry.

Thanks again