[4.1] Log deprecated only when explicitly requested

[Fedik]

Summary of Changes

Try to fix performance issue #30997.
I have added proxy method Log::deprecated(), to use for deprecation log, which doing log only when log_deprecated is On.

However here a little caveats:
First: The patch add Application dependency to Log class.
Second: The method may be called when Application is not initialized, then the method will do nothing, such logs may be missed in log output. This affect logs from bootup process. (Actually this is true for all bootup logs)

Use of trigger_error() for log for deprecated messages.

@wilsonge @HLeithner please review

Testing Instructions

Apply patch.
Enable debug.
In global config enable "Log Deprecated API"
In the debug plugin disable "Log Entries"

Open any page (example /administrator/index.php?option=com_config)
check Debug Bar and note rendering time,

Then in global config disable "Log Deprecated API", refresh the same page,
check Debug Bar and note rendering time,

Actual result BEFORE applying this Pull Request

Debug Bar does not show logs but rendering time still huge.
At administrator/index.php?option=com_config I got around 3s

Expected result AFTER applying this Pull Request

Debug Bar does not show logs and rendering time is smaller.
At administrator/index.php?option=com_config I got around 270ms

Documentation Changes Required

none

[HLeithner]

I would say we should target this for 3.10 (but didn't looked at it now)

[SharkyKZ]

Going forward trigger_error() should be used to trigger deprecation warnings.

[Fedik]

Going forward trigger_error() should be used to trigger deprecation warnings.

I do not know what a performance of this approach.
In past I seen complains about such approach, but did not tested by myself.

[SharkyKZ]

It doesn't automatically solve the issue you're trying to solve. But if Joomla\CMS\Log\Log::add() calls for deprecation logging are being replaced by anything, it should trigger_error(). Any changes you need then should be done either in Joomla\CMS\Exception\ExceptionHandler::handleUserDeprecatedErrors() or whatever it ends up calling.

Also configuration should be read from configuration file. If Joomla\CMS\Factory::getApplication() or application's get() method gets deprecated, current code will result in infinite loop.

[Fedik]

If Joomla\CMS\Factory::getApplication() or application's get() method gets deprecated, current code will result in infinite loop.

nope

well, can be, but that problem of that guy who will make it deprecated 😄

[SharkyKZ]

[Fedik]

Yea I already understood what you meant, the same is true if "Log::add" become deprecated, even with trigger_error.
Just ignore these edge cases ;)

[SharkyKZ]

It's not an edge case. You are adding a broken dependency where there should be no dependencies. As for Log::add() getting deprecated, it's a poor example. You obviously wouldn't make Log:add() calls inside Log::add() itself.

[HLeithner]

I think that's one of the rare cases we could use Factory::getContainer()->get('application');

[SharkyKZ]

🤦‍♂️

[Fedik]

@SharkyKZ I understood what you talked about, but this does not help to fix anything 😉
About extra dependency I have wrote in the initial comment, I did not found a better way for now.

As another idea: can try adopt Symfony ErrorHandler somehow (for catch PHP errors/deprecation), but I have no idea how it may work, and whether it will help to avoid performance issue.
With this the option log_deprecated should modify error_reporting() to enable E_USER_DEPRECATED

[Fedik]

Well,forget all,
I think we can switch E_USER_DEPRECATED on Application bootup, and then read it in Log::deprecated

I try to test how it work, and update PR some time later.

[wilsonge]

I don't think this is right. We have a category system specifically to ensure full interopability with other logging systems (also the category system on more complicated sites actually allows a lot of granularity via plugins). If we have performance issues we need to fix them - not hide them away. Has anyone actually tried figuring out what our performance issues are?

[Fedik]

Has anyone actually tried figuring out what our performance issues are?

call Log::add() 3000 times, even if log_deprecated set to Off

[Fedik]

but If you asking why Log::add() become so slow,
there a couple potential reasons: from use of regexp to load debug_backtrace, both is required log features :)

[Fedik]

I have changed much stuff, now it better I hope 😄

Also since now, must be used @trigger_error('blabla message', E_USER_DEPRECATED) for deprectated stuff. As @SharkyKZ suggested.

@HLeithner @wilsonge check check

upd: it cannot be ported to 3.10, unless this #30317 patch will be ported also

[Fedik]

do we have a chance to have it in, before next beta? :)

[JonasAtZwetschke]

I have tested this item ✅ successfully on 3ccf3aa

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31143.}

[ghost]

I have tested this item ✅ successfully on 3ccf3aa

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31143.}

[richard67]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/31143.}

[rdeutz]

I am moving this to 4.1, we are in beta and this is a new feature. Thank you all who have been working on this and bringing it to the state it is now. But if we want to get 4.0 out of the door we need to make a cut.

Side note: this decision has be discussed in the production department and the maintainers team. It's not just mine even if I support the decision.

[Fedik]

conflict fixed :)

[bembelimen]

Thanks