Skip to content

deprecated profiler.matcher configuration #24158

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

Merged
merged 2 commits into from
Sep 11, 2017

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Sep 11, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets closes #24077, would also close #21944, #22484
License MIT
Doc PR not yet

The profiler matcher configuration was added at a time where we thought that having the profiler in production could make sense (and so being able to enable it conditionally made sense). That's not the case anymore. Nobody should ever enable it in production.

With that in mind, I propose to deprecate this setting in 3.4 and remove it in 4.0.

@fabpot fabpot force-pushed the profiler-matcher-deprecation branch from c0a6ff6 to 6eff3e5 Compare September 11, 2017 18:07
@fabpot fabpot merged commit 6eff3e5 into symfony:3.4 Sep 11, 2017
fabpot added a commit that referenced this pull request Sep 11, 2017
This PR was squashed before being merged into the 3.4 branch (closes #24158).

Discussion
----------

deprecated profiler.matcher configuration

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | closes #24077, would also close #21944, #22484
| License       | MIT
| Doc PR        | not yet

The profiler matcher configuration was added at a time where we thought that having the profiler in production could make sense (and so being able to enable it conditionally made sense). That's not the case anymore. Nobody should ever enable it in production.

With that in mind, I propose to deprecate this setting in 3.4 and remove it in 4.0.

Commits
-------

6eff3e5 deprecated profiler.matcher configuration
2c62ba8 fixed CS
@fabpot fabpot deleted the profiler-matcher-deprecation branch September 11, 2017 18:18
This was referenced Oct 18, 2017
@wouterj
Copy link
Member

wouterj commented Oct 19, 2017

😢

We have a route that is executed every x seconds. While in dev, this makes the cache directory very big... This option was our solution to prevent the profiler from being executed. Is there any other solution now?

@stof
Copy link
Member

stof commented Oct 19, 2017

you could disable the profiler in your controller explicitly (as done in the WebProfilerBundle or in AsseticBundle)

@atierant
Copy link

Thanks, but when you have lots of Controllers in your app, it might be an arrow in the knee, no ?
You have to inject the profiler everywhere, just to disable it ? Do you know if it is possible to do it with an EventSubscriber which would listen to FilterControllerEvent ?

@wouterj
Copy link
Member

wouterj commented Jan 15, 2020

You can also create an event listener/subscriber on kernel.request (or the like) and disable the profiler in there.

@atierant
Copy link

Alright ! It works !

I'm on 3.4.
As said here : https://symfony.com/doc/4.4/profiler.html#enabling-the-profiler-conditionally
In my config_dev.yml (I don't have any service_dev.yml yet) :

services:
    Symfony\Component\HttpKernel\Profiler\Profiler: '@profiler'

I've done a ProfilerSubscriber :

<?php
declare(strict_types=1);

namespace App\EventSubscriber;

use App\RouterMatcher\CustomMatcher;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Event\GetResponseEvent;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\HttpKernel\Profiler\Profiler;

class ProfilerSubscriber implements EventSubscriberInterface
{
    private $matcher;
    private $profiler;

    public function __construct(CustomMatcher $matcher, ?Profiler $profiler)
    {
        $this->matcher = $matcher;
        $this->profiler = $profiler;
    }

    public static function getSubscribedEvents(): array
    {
        return [
            KernelEvents::REQUEST => 'onKernelRequest',
        ];
    }

    public function onKernelRequest(GetResponseEvent $event): void
    {
        if (null !== $this->profiler) {
            if ($this->matcher->matches($event->getRequest())) {
                $this->profiler->disable();
            }
        }
    }
}

and a CustomMatcher :

<?php

namespace App\RouterMatcher;

use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\RequestMatcherInterface;

class CustomMatcher implements RequestMatcherInterface
{
    public function matches(Request $request): bool
    {
        return in_array($request->get('_route'), ['route_that_i_don_t_want_to_profile']);
    }
}

thanks @wouterj and @stof !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants