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

Deprecated warnings sent to another channel after minor upgrade (v5.4.29 -> v5.4.31) #52672

Closed
bakotiinii opened this issue Nov 21, 2023 · 21 comments

Comments

@bakotiinii
Copy link

Symfony version(s) affected

v5.4.31

Description

I upgrade symfony from v5.4.29 to v5.4.31 and got huge amount of deprecated warning in my monolog logger in php channel.

My research showed me, that previously all deprecated errors were sent to security channel, and after upgrade it started to send to php channel.

Can you please explain, why symfony team decide to change default channel for deprecate error (from security to php) and how is it possible to return it back to security?

Example of deprecate errors:

User Deprecated: Since symfony/security-guard 5.3: The "Symfony\Component\Security\Guard\GuardAuthenticatorHandler" class is deprecated, use the new authenticator system instead.

User Deprecated: Since symfony/security-core 5.3: The "Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface" interface is deprecated, use the new authenticator system instead.

__

Thank you for your time.

p.s. i checked comparison v5.4.29...v5.4.31 and cant see anything related to change of the channel

How to reproduce

Use version of symfony/symfony - v5.4.31
Use some deprecated component from Security Core.

Possible Solution

No response

Additional Context

No response

@javaDeveloperKid
Copy link
Contributor

javaDeveloperKid commented Nov 21, 2023

Please provide your monolog.yaml file (if you use separate files per environment please provide config/monolog.yaml and config/<env>/monolog.yaml)

@stof
Copy link
Member

stof commented Nov 21, 2023

Deprecations were never sent to the security channel. They are sent to the php channel since years, and there is an opt-in way of sending them to a deprecation channel instead.

@bakotiinii
Copy link
Author

bakotiinii commented Nov 22, 2023

Please provide your monolog.yaml file (if you use separate files per environment please provide config/monolog.yaml and config/<env>/monolog.yaml)

services.yml

services:
    gelf.transport:
        class: Shared\LoggerBundle\Transport\Gelf\UdpTransport
            arguments:
                - '%graylog.host%'
                - '%graylog.port%'

    gelf.publisher:
        class: Gelf\Publisher
        arguments:
            - '@gelf.transport'

    monolog.gelf_handler:
        class: Monolog\Handler\GelfHandler
        arguments:
                - '@gelf.publisher'

    monolog.processor.web_processor:
        class: Monolog\Processor\WebProcessor
        tags:
                 - { name: monolog.processor, method: __invoke }
            

config.yml

monolog:

    handlers:

        main:
            type:         fingers_crossed
            action_level: warning
            handler:      errors
            channels:     ["!elastica"]

        errors:
            type:   service
            id:     monolog.gelf_handler
            nested: true

        filter_common:
            type:      filter
            handler:   common
            min_level: DEBUG
            max_level: NOTICE
            channels: ["!event", "!doctrine", "!security", "!request"]
        common:
            type:     service
            id:       monolog.gelf_handler
            nested:   true

And we get deprecated in common with php after v5.4.29 -> v5.4.31, so we suppress then like:

        filter_common:
            type:      filter
            handler:   common
            min_level: DEBUG
            max_level: NOTICE
            channels:  ["!event", "!doctrine", "!security", "!request", "!php"]

@stof
Copy link
Member

stof commented Nov 22, 2023

Have you only updated Symfony packages from 5.4.29 to 5.4.31 or have you also made other package updates ? Maybe the actual change is that there was no deprecations triggered before (and so you were not seeing anything)

@javaDeveloperKid
Copy link
Contributor

javaDeveloperKid commented Nov 22, 2023

Downgrade the component from which you get a deprecation, comment out "!security" and "!php" and see if deprecation is logged. Then try options with only security channel excluded and only php channel excluded.

@oldy777
Copy link

oldy777 commented Nov 22, 2023

I think problem here https://github.com/symfony/symfony/blob/48be4b33ac3f144b803f0e33a0e51c4946eee2ea/src/Symfony/Component/HttpKernel/EventListener/DebugHandlersListener.php#L164...L177
This conditions now don't have any sense. In both cases we will set service('logger')

@stof
Copy link
Member

stof commented Nov 22, 2023

@oldy777 this is not true. We have a compiler pass that injects the appropriate monolog channels (the default config still injects logger only to account for projects not using MonologBundle)

@oldy777
Copy link

oldy777 commented Nov 23, 2023

@stof Maybe I got it wrong, pls correct me
image
Previously, when we didn't have monolog.logger.deprecation we didn't get deprecation logs, because $this->deprecationLogger === null

@nicolas-grekas
Copy link
Member

Can you provide the link to this commit in your screenshot please?

@oldy777
Copy link

oldy777 commented Nov 23, 2023

@nicolas-grekas
It seems this PR #52009

@stof
Copy link
Member

stof commented Nov 23, 2023

@oldy777 and this same PR has the compiler pass in its diff.

@javaDeveloperKid
Copy link
Contributor

Maybe the ifs in this CompilerPass are not fulfilled in the OP's application.

@oldy777
Copy link

oldy777 commented Nov 23, 2023

@stof There is different logic in CompilerPass

 public function process(ContainerBuilder $container)
    {
        if (!$container->hasDefinition('debug.debug_handlers_listener')) {
            return;
        }

        $definition = $container->getDefinition('debug.debug_handlers_listener');
        if ($container->hasDefinition('monolog.logger.php')) {
            $definition->replaceArgument(1, new Reference('monolog.logger.php'));
        }
        if ($container->hasDefinition('monolog.logger.deprecation')) {
            $definition->replaceArgument(6, new Reference('monolog.logger.deprecation'));
        }
    }

If we don't have monolog.logger.deprecation we will use service('logger'), not null

@nicolas-grekas
Copy link
Member

Let's fix this then. Can you please send a PR to ensure we behave as before?

@stof
Copy link
Member

stof commented Nov 23, 2023

AFAICT, it was never intended to skip logging entirely if you don't create a deprecation channel. Logging them nowhere was a regression introduced by the previous PR introducing the opt-in way of using a deprecation channel and the PR you linked fixed that by changing the way this was implemented.

@oldy777
Copy link

oldy777 commented Nov 23, 2023

@stof and now we have huge regression on production servers without any normal possibility to filter out all this logs.
I can't even imagine who need deprecation logs in production.
Moreover, I think this PR broke BC and shouldn't have been posted as a minor fix

@javaDeveloperKid
Copy link
Contributor

javaDeveloperKid commented Nov 23, 2023

and now we have huge regression on production servers without any normal possibility to filter out all this logs.

Do you mean filer out already existing logs or new logs? The former you can ignore by filtering out word Deprecated. The latter you can use (for now, until fixed in the framework) a handler of type filter and set the minimum level to Notice (or whatever higher than Deprecated).

I can't even imagine who need deprecation logs in production.

Actually I would say deprecation logs are the most useful when enabled on production, because there is the real traffic and you can collect the most information about which deprecated code you use.

@stof
Copy link
Member

stof commented Nov 23, 2023

Allowing to opt-in for the usage of a deprecation channel instead of sending them in the same php channel than other PHP errors is precisely meant to allow you to easily configure MonologBundle to have a special handling for the deprecation channel.

@nicolas-grekas
Copy link
Member

Not sure why we need this lengthy discussion now that the issue is identified. Here is the fix: #52707

@stof
Copy link
Member

stof commented Nov 23, 2023

if you want to ignore the deprecation logs entirely in prod, this is the easiest way to do it:

# this goes in the config/packages/monolog.yaml file
monolog:
    channels:
        - deprecation # Create the dedicated channel to log deprecations

# For older Symfony version that don't support this `when@prod` feature, put the config
# in config/packages/prod/monolog.yaml instead
when@prod:
    monolog:
        handlers:
            deprecation_backhole:
                type: null
                channels: [deprecation]
                priority: 1000

@oldy777
Copy link

oldy777 commented Nov 23, 2023

@stof Now I know how to fix it. It took me more than a day to found the problem, but I had less than 30 minutes to fix it on production. 😀
I think this behavior prod: deprecation -> null should be by default

nicolas-grekas added a commit that referenced this issue Nov 23, 2023
… when channel "deprecation" is not defined (nicolas-grekas)

This PR was merged into the 5.4 branch.

Discussion
----------

[HttpKernel] Fix logging deprecations to the "php" channel when channel "deprecation" is not defined

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #52672
| License       | MIT

Commits
-------

4bc6567 [HttpKernel] Fix logging deprecations to the "php" channel when channel "deprecation" is not defined
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants