Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[FrameworkBundle] Add configuration for profiler log channels#23247

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
maff wants to merge1 commit intosymfony:3.4frommaff:profiler-log-channels

Conversation

@maff
Copy link
Contributor

@maffmaff commentedJun 21, 2017
edited
Loading

QA
Branch?3.4 or master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets
LicenseMIT
Doc PR

In larger projects, the amount of logs stored in the profiler can grow quickly and the web profiler takes quite some time to render the logs screen when having hundreds of event and db log entries. This PR adds a configuration entry to exclude/include certain log channels from the profiler. The format follows thechannels configuration entry from the monolog bundle:

framework:    profiler:        log_channels: ["!event", "!doctrine"]# more verboseframework:    profiler:        log_channels:            channels: ["!event", "!doctrine"]# or profile only certain channelsframework:    profiler:        log_channels:            channels: ["event", "doctrine"]

I'll be happy to add a doc PR if this is accepted.

welcoMattic and emodric reacted with thumbs up emoji
@maffmaffforce-pushed theprofiler-log-channels branch from4a10c2c tofc4e108CompareJune 21, 2017 11:01
@xabbuhxabbuh added this to the3.4 milestoneJun 21, 2017
@nicolas-grekas
Copy link
Member

Adding configuration is always adding complexity for the end user. If we can do otherwise (including doing nothing), i think that might be better. I all depends on the current "brokenness" status.


privatefunctionisFiltered(array$record)
{
if ($this->channelsExclusive && !in_array($record['channel'],$this->channels)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

if ($this->channelsExclusive && !in_array($record['channel'],$this->channels,true)) {returnfalse;         }returnin_array($record['channel'],$this->channels,true);     }

}
}

if (!count($channels)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

if (!$channels) {}

@fabpot
Copy link
Member

I agree with@nicolas-grekas. What about truncating the logs when there are too important instead?

fabpot added a commit that referenced this pull requestSep 27, 2017
This PR was squashed before being merged into the 3.3 branch (closes#24244).Discussion----------TwigBundle exception/deprecation tweaks| Q             | A| ------------- | ---| Branch?       | 3.3| Bug fix?      | yes| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes/no| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->| License       | MIT| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->- 1st commit) if you view a exception in the profiler, there is no logger available. Making the tab useless, disabled state is now triggered at zero log messages. There's a specialized panel here.- 2nd commit) when an exception occurs this highlights deprecations in the log table outside the profiler with a warning status. This follows the same signal colors in the profiler.- 3rd commit) hide the default inactive tabs from CSS to avoid scrollbar flickering.- 4th commit) favors document.DOMContentLoaded over window.load, we dont want to wait for images to be loadedFurther out-of-scope improvements could be;- From#24191; i think the logs table should show a direct `View file` link for every error/deprecation/red or yellow line in here. Traversing with `Show context` is tedious.  - links to file.php for your trigger_error() calls  - links to config.yml for trigger_error() calls by SF- From#24151; having the same tooling on both sides is nice- Events/Translations logs is noise, we have specialized panels for those. To further reduce the overall page size container logs can be moved away too, linked from Configuration and/or Logs. Also see#23247Commits-------1c595fc [TwigBundle][WebProfilerBundle] Switch to DOMContentLoaded eventea4b096 [WebProfilerBundle] Hide inactive tabs from CSS0c10f97 [TwigBundle] Make deprecations scream in logs03cd9e5 [TwigBundle] Hide logs if unavailable, i.e. webprofiler
@nicolas-grekas
Copy link
Member

Moving to 4.1. Rebase on master might be needed, where PHP 7.1 features can be used btw.

@nicolas-grekasnicolas-grekas modified the milestones:3.4,4.1Oct 8, 2017
@maff
Copy link
ContributorAuthor

maff commentedOct 9, 2017

Sorry, I missed this conversation somehow. Regarding "brokenness": it really depends on the project, but we've seen issues with large profiler data and profiler not being able to load when lots of events and/or doctrine queries are logged during a request. At that time the profiler still popped up the alert window which really made it a problem, now with the toolbar just showing an error it's way better.
For me it's also fine to not include this change as long as I can override this on a project level when really needed (e.g. by decorating the DebugProcessor).

Regarding truncating: could work as well, but I don't really have an idea how to decide what and when to truncate.

ostrolucky pushed a commit to ostrolucky/symfony that referenced this pull requestMar 25, 2018
This PR was squashed before being merged into the 3.3 branch (closessymfony#24244).Discussion----------TwigBundle exception/deprecation tweaks| Q             | A| ------------- | ---| Branch?       | 3.3| Bug fix?      | yes| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes/no| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->| License       | MIT| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->- 1st commit) if you view a exception in the profiler, there is no logger available. Making the tab useless, disabled state is now triggered at zero log messages. There's a specialized panel here.- 2nd commit) when an exception occurs this highlights deprecations in the log table outside the profiler with a warning status. This follows the same signal colors in the profiler.- 3rd commit) hide the default inactive tabs from CSS to avoid scrollbar flickering.- 4th commit) favors document.DOMContentLoaded over window.load, we dont want to wait for images to be loadedFurther out-of-scope improvements could be;- Fromsymfony#24191; i think the logs table should show a direct `View file` link for every error/deprecation/red or yellow line in here. Traversing with `Show context` is tedious.  - links to file.php for your trigger_error() calls  - links to config.yml for trigger_error() calls by SF- Fromsymfony#24151; having the same tooling on both sides is nice- Events/Translations logs is noise, we have specialized panels for those. To further reduce the overall page size container logs can be moved away too, linked from Configuration and/or Logs. Also seesymfony#23247Commits-------1c595fc [TwigBundle][WebProfilerBundle] Switch to DOMContentLoaded eventea4b096 [WebProfilerBundle] Hide inactive tabs from CSS0c10f97 [TwigBundle] Make deprecations scream in logs03cd9e5 [TwigBundle] Hide logs if unavailable, i.e. webprofiler
@nicolas-grekasnicolas-grekas modified the milestones:4.1,nextApr 20, 2018
fabpot added a commit that referenced this pull requestOct 10, 2018
This PR was submitted for the 3.4 branch but it was merged into the 4.2-dev branch instead (closes#24263).Discussion----------Filter logs by level| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->| License       | MIT| Doc PR        | symfony/symfony-docs#... <!--highly recommended for new features-->Proposal to filter logs by level. This PR competes with#23247 (but also see#23038) which propose to filter by channel.<details><summary>Before</summary>![image](https://user-images.githubusercontent.com/1047696/30607022-00536bbe-9d74-11e7-84dd-6427d328f50b.png)</details><details><summary>After</summary>![image](https://user-images.githubusercontent.com/1047696/31036405-6346da12-a56c-11e7-8747-b1ae89c549f2.png)</details>From#23247 (comment)> Adding configuration is always adding complexity for the end user. If we can do otherwise (including doing nothing), i think that might be better. I all depends on the current "brokenness" status.This avoids that. Also single click; noise gone.Commits-------8f88753 Filter logs by level
@stof
Copy link
Member

stof commentedApr 7, 2019

Decorating thedebug.log_processor service is already possible if the project actually needs to filter logs to avoid storing them in the profiler:

class FilteredDebugProcessorimplements DebugLoggerInterface{private$innerProcessor;publicfunction__construct(DebugLogProcessor$processor)    {$this->innerProcessor =$processor;    }publicfunction__invoke(array$record)    {if (/* condition to exclude*/) {return$record;        }return$this->innerProcessor->__invoke($record);    }/**     * {@inheritdoc}     */publicfunctiongetLogs(Request$request =null)    {return$this->innerProcessor->getLogs($request);    }/**     * {@inheritdoc}     */publicfunctioncountErrors(Request$request =null)    {return$this->innerProcessor->countErrors($request);    }/**     * {@inheritdoc}     */publicfunctionclear()    {$this->innerProcessor->clear();    }}
services:filtered_debug_processor:class:FilteredDebugProcessorarguments:filtered_debug_processor.innerdecorates:debug.log_processor

@stof
Copy link
Member

stof commentedApr 7, 2019

I'm closing this PR, as I don't think that this additional complexity is needed in the core (and@fabpot and@nicolas-grekas already said the same a long time ago).

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

1 more reviewer

@stloydstloydstloyd requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

7 participants

@maff@nicolas-grekas@fabpot@stof@stloyd@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp