[Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands #47416

HeahDude · 2022-08-28T23:05:48Z

Q	A
Branch?	6.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Fix #45241
License	MIT
Doc PR	~

TLDR;

I've shown a POC of this feature at the Symfony Live Paris last April to some of the core team members (ping @nicolas-grekas, @stof, @lyrixx, @chalasr, @GromNaN).
I propose here a new work from scratch addressing the comments I already got and based on Javier's profiler redesign (#47148).
Reviews should better be done by commits.

Summary

This PR aims to leverage the profiler and its collectors by using a VirtualRequestStack to aggregate data on virtual requests.
Such requests are obfuscated by default to avoid side effects.
It can feel like a hack... or a pragmatic way to get, without much complexity, tons of useful feedback on what's going on during console execution, from basic info about the command, time/memory metrics, to every existing features already available in HTTP context: events, message dispatching, http requests, emails, serialization, validation, cache, database queries... and so on, all that just out of the box!

Previous work

There were some work to extract the Profiler logic in a dedicated component, that proved to require a lot of complexity and BC breaks in the API:

Screenshots

For now I've focused only on the functional parts.

Search view

Command panel

If the command is signal-able the following panel will be available:

If sub commands are run using $this->getApplication()->run() sub profiles will be shown as for requests:

The server tab is the same as in the request panel.

Performance panel

Failing command

The exception panel is shown by default as for requests:

Sub command

Profile token when verbose

(clickable links with compatible terminals)

Command interrupted by signal

Opt-in profiling

Use the new global option --profile (in debug only) to profile a command.

Future scopes

When I've discussed the limitation of profiling long running processes such as messenger:consume with @GromNaN (one of the reasons why I've added an excludes option), he told that it would be nice it we could find a way to profile consumers as well.
So I've added ~~an abstract VirtualRequest~~ a _virtual_type request attribute and a virtualType property to profiles, that will allow to create a MessengerWorkerRequest and a new type of profile with ease in a follow-up PR if the current implementation is accepted.
We could add some dedicated casters for input and output in the VarDumper component (/cc @nicolas-grekas)
It could be interesting to decorate and collect traces from some helpers (i.e. when running processes)
~~Add a global option in debug to enable/disable the profiler on the fly when running commands (e.g. a negatable --profile flag)~~
[update] implemented in current scope in replacement of semantic config.
Extract profiling to a new component.

Limitations

No sub profiles are created when using $this->getApplication()->find(...)->run() because events (needed by the profiler to hook into) are dispatched from Application::run(), not from Command::run().
[update] The docs has been updated in [Console] Improve console events doc symfony-docs#18741.
~~No profiles are created when killing the command process (i.e. using ctrl-C, should we add a handler to some signals to force saving profiles?~~
[update] I've added support for this.
Signals as int may not be as useful as they could in the profiler pages, does it worth trying to add a label to some (knowing that some signals can have different constants (labels) with the same int value)?
[update] done thanks to [Console] Add SignalMap to map signal value to its name #50663.
~~Long running processes should be excluded via configuration to avoid memory leaks~~
[update] profiling is now opt-in using the --profile option.
Profiling messenger:consume does not work since the kernel is reset after handling a message.

TODO

I've left some todos inside the code for reviewers to share they thought before I try going further
Add a few tests
Get help for the UI (new top nav, ~~svg for the command panel~~) /cc @javiereguiluz
~~PR on symfony/recipes to add the new framework.profiler.cli node~~

carsonbot · 2022-08-29T19:07:02Z

Hey!

I think @Seb33300 has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

…ms (HeahDude) This PR was merged into the 5.4 branch. Discussion ---------- [WebProfilerBundle] Fix profile search bar link query params | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | ~ | License | MIT | Doc PR | ~ Spotted while working on #47416. The second argument of `controller()` is `attributes`. Commits ------- c09e7a6 [WebProfilerBundle] Fix profile search bar link query params

HeahDude · 2022-09-13T16:13:06Z

Rebased, this PR is now reviewable.

94noni

Looking forward to playing with it locally
This will be very pleasant for cli based app etc :)

src/Symfony/Component/HttpKernel/Debug/VirtualRequest.php

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/layout.html.twig

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/command.html.twig

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

HeahDude · 2022-10-22T12:28:06Z

I've pushed a new implementation to use a _virtual request attribute instead of a virtual request type.

chalasr

Thanks for this. Here are some comments. I'm not sure about DebugRequestStack also, this is probably not going to be used elsewhere so I'm leaning toward naming at around the console command use case.

src/Symfony/Component/HttpKernel/Profiler/FileProfilerStorage.php

src/Symfony/Bundle/FrameworkBundle/Debug/CliRequest.php

src/Symfony/Bundle/FrameworkBundle/Console/Application.php

HeahDude · 2022-10-22T14:21:03Z

I've added support for profiling commands interrupted by signals.
I've updated the PR description and added a screenshot as well.

94noni · 2022-10-26T07:39:14Z

@HeahDude did not find time to test on a personal project it but just a raw comment:

given you screenshot in PR desc, is it possible to have the command with its arguments/options being copy on click?
like by default the url of the request giving the current profile we are redirected to the url, here we mimic the logic but with the console, thus having the cli il the clipboard

ℹ️ also I do not know if it is feasible, if the "server" is symfony cli, copy something like:

symfony console app-delete-user -v

instead of

bin/console app-delete-user -v

Jeroeny · 2022-11-22T14:47:45Z

This would be very useful. Would a similar abstraction become possible for profiling messages processed by the messenger?

94noni · 2022-11-22T15:25:04Z

This would be very useful. Would a similar abstraction become possible for profiling messages processed by the messenger?

@Jeroeny as per PR desc Future scopes, I think yes

maxhelias · 2022-11-27T16:42:26Z

ℹ️ also I do not know if it is feasible, if the "server" is symfony cli, copy something like:

symfony console app-delete-user -v

instead of

bin/console app-delete-user -v

I think now you can thanks to symfony-cli/symfony-cli#231

94noni

hi @HeahDude

I find and took some time this morning to test this locally, it is really nice :)
here are some comments mainly in UX/UI usage

also with #47416 (comment) I wrote before

src/Symfony/Bundle/FrameworkBundle/EventListener/ConsoleProfilerListener.php

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/header.html.twig

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/command.html.twig

javiereguiluz · 2023-01-13T12:36:24Z

This is an amazing contribution @HeahDude. I hope this is reviewed and merged soon 🙏

I have some ideas related to the UI of this feature. If you agree, I'd prefer to merge this PR and then I can open a new PR with the proposed UI changes to discuss about them.

Thanks!

adrienbrault · 2023-03-24T10:49:08Z

Hey, first party CLI profiler support sounds awesome!

I have been using a hacky solution to get CLI/Messenger profiles: https://github.com/sourceability/instrumentation/blob/1.0.1/src/Profiler/SymfonyProfiler.php

Which is really really useful with https://github.com/sourceability/console-toolbar-bundle that renders the profiler toolbar within the terminal.
There is a video demo in Sylius/Sylius#12711

src/Symfony/Component/HttpKernel/Profiler/ProfilerStorageInterface.php

src/Symfony/Bundle/FrameworkBundle/Debug/TraceableEventDispatcher.php

nicolas-grekas · 2023-10-17T12:37:42Z

LGTM after a few changes I just pushed, but I agree with @stof it'd be great to get rid of this strange event-dispatcher + listener logic to start/stop sections. Doable in this PR?

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/command.html.twig

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/header.html.twig

src/Symfony/Component/HttpKernel/Profiler/Profiler.php

…iling commands

nicolas-grekas

🚀

nicolas-grekas · 2023-10-17T13:26:07Z

Thank you @HeahDude.

HeahDude · 2023-10-17T13:30:32Z

1 and a half year after showing a POC of this feature at the Symfony Live, and 188 comments later, so happy to have this merged.

Thanks to all the reviewers for their challenging reviews 🎉 🥰

…Dude) This PR was merged into the 7.0 branch. Discussion ---------- [HttpKernel] Remove deprecation layer for Profiler | Q | A | ------------- | --- | Branch? | 7.0 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | ~ | License | MIT Follows #47416. Commits ------- 65970b2 [HttpKernel] Remove deprecation layer for Profiler

…ault) This PR was merged into the 6.4 branch. Discussion ---------- [WebProfilerBundle] Fix generated application link | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no  | Deprecations? | no  | Tickets | Fix #...  | License | MIT I played a little bit with the new Profiling command #47416 (I love it !), and found that `type=command` is missing in one URL ![image](https://github.com/symfony/symfony/assets/9253091/c0ba0a81-e906-4223-b91d-7b91ceac2309) Commits ------- 477d849 [WebProfilerBundle] Fix generated application link

…javiereguiluz) This PR was squashed before being merged into the 6.4 branch. Discussion ---------- [WebProfilerBundle] UI tweaks for the command profiler | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT This PR proposes some minor UI tweaks for the amazing feature contributed by `@HeahDude` in #47416. ## First change I propose to not display the `HTTP / Commands` toggle in the header of all pages. I don't think this should be an option with quick permanent access from all profiler pages. My proposal is to move the toggle to the search sidebar: ![search-before-after](https://github.com/symfony/symfony/assets/73419/417cc200-5f56-41a8-b177-8124e6bed2f3) And here in action: ![http-commands-toggle](https://github.com/symfony/symfony/assets/73419/ced6dae1-2de9-4e42-ad5b-48e752513ec3) ## Second change In my opinion, the current header of command profiles looks too similar to HTTP profiles: ### Before / Light ![command-profile-before-light](https://github.com/symfony/symfony/assets/73419/b623e582-7a67-414c-9d5f-51d1ceee83d7) ### Before / Dark ![command-profile-before-dark](https://github.com/symfony/symfony/assets/73419/884fa8db-d915-439b-8fe0-12e5d7a49a41) ----- I propose to differentiate them a bit more and use the well-known "fake terminals" used on Symfony website and docs: ### After / Light ![command-profile-after-light](https://github.com/symfony/symfony/assets/73419/670cdb13-35fc-42ad-9b42-7d83bb75c1d0) ### After / Dark ![command-profile-after-dark](https://github.com/symfony/symfony/assets/73419/6899926d-0831-4241-8132-75e2a180fa23) ----- The "fake terminals" look different depending on your OS. See this image (from top to bottom: macOS, Windows, Linux) ![terminal-os](https://github.com/symfony/symfony/assets/73419/04bea418-63df-45f2-9fa8-06afbfa8048f) Commits ------- 041480f [WebProfilerBundle] UI tweaks for the command profiler

GromNaN · 2024-03-28T22:11:14Z

src/Symfony/Component/Console/Command/TraceableCommand.php

+    {
+        $event = $this->stopwatch->start($this->getName().'.execute', 'command');
+
+        $exitCode = $this->command->execute($input, $output);


I noticed that this call does not work if the method is defined in a trait.
https://3v4l.org/AFJnQ
This is the case for DoctrineFixturesBundle with the CommandCompatibility trait

I don't think it's worth opening an issue.

HeahDude requested review from wouterj and chalasr as code owners August 28, 2022 23:05

carsonbot added Status: Needs Review Deprecation Feature FrameworkBundle HttpKernel WebProfilerBundle labels Aug 28, 2022

carsonbot added this to the 6.2 milestone Aug 28, 2022

carsonbot changed the title ~~[HttpKernel][FrameworkBundle][WebProfilerBundle] Enable profiling commands~~ [FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands Aug 28, 2022

HeahDude force-pushed the feat/profile-cli branch from 23d46cb to e87cf77 Compare August 28, 2022 23:19

HeahDude mentioned this pull request Aug 28, 2022

[WebProfilerBundle] Fix profile search bar link query params #47417

Merged

HeahDude force-pushed the feat/profile-cli branch from e87cf77 to 70574d8 Compare September 13, 2022 16:07

94noni reviewed Sep 27, 2022

View reviewed changes

HeahDude force-pushed the feat/profile-cli branch from 70574d8 to 96a5cdf Compare October 22, 2022 12:21

chalasr reviewed Oct 22, 2022

View reviewed changes

src/Symfony/Component/HttpKernel/Profiler/FileProfilerStorage.php Outdated Show resolved Hide resolved

src/Symfony/Bundle/FrameworkBundle/Debug/CliRequest.php Outdated Show resolved Hide resolved

src/Symfony/Bundle/FrameworkBundle/Console/Application.php Show resolved Hide resolved

nicolas-grekas modified the milestones: 6.2, 6.3 Nov 5, 2022

94noni reviewed Dec 10, 2022

View reviewed changes

HeahDude force-pushed the feat/profile-cli branch from 2f5ea2e to 649e4ef Compare October 17, 2023 12:16

nicolas-grekas force-pushed the feat/profile-cli branch from 649e4ef to b404c26 Compare October 17, 2023 12:28

nicolas-grekas reviewed Oct 17, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Profiler/ProfilerStorageInterface.php Outdated Show resolved Hide resolved

stof reviewed Oct 17, 2023

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Debug/TraceableEventDispatcher.php Outdated Show resolved Hide resolved

nicolas-grekas force-pushed the feat/profile-cli branch from b404c26 to 343a318 Compare October 17, 2023 12:42

HeahDude force-pushed the feat/profile-cli branch from 343a318 to a2081df Compare October 17, 2023 12:48

stof reviewed Oct 17, 2023

View reviewed changes

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/command.html.twig Show resolved Hide resolved

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/header.html.twig Show resolved Hide resolved

HeahDude force-pushed the feat/profile-cli branch 2 times, most recently from aeb3266 to ece480c Compare October 17, 2023 13:02

nicolas-grekas reviewed Oct 17, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Profiler/Profiler.php Outdated Show resolved Hide resolved

nicolas-grekas reviewed Oct 17, 2023

View reviewed changes

src/Symfony/Component/HttpKernel/Profiler/Profiler.php Outdated Show resolved Hide resolved

[Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable prof…

82914ba

…iling commands

HeahDude force-pushed the feat/profile-cli branch from ece480c to 82914ba Compare October 17, 2023 13:22

nicolas-grekas approved these changes Oct 17, 2023

View reviewed changes

nicolas-grekas merged commit 814bcac into symfony:6.4 Oct 17, 2023
2 of 9 checks passed

HeahDude deleted the feat/profile-cli branch October 17, 2023 13:28

HeahDude mentioned this pull request Oct 17, 2023

[HttpKernel] Remove deprecation layer for Profiler #52114

Merged

javiereguiluz mentioned this pull request Oct 18, 2023

[WebProfilerBundle] UI tweaks for the command profiler #52138

Merged

soyuka mentioned this pull request Oct 18, 2023

chore: symfony dev updates api-platform/core#5895

Merged

alamirault mentioned this pull request Oct 18, 2023

[WebProfilerBundle] Fix generated application link #52152

Merged

This was referenced Oct 21, 2023

Release v6.4.0-BETA1 #52220

Merged

Release v7.0.0-BETA1 #52224

Merged

94noni mentioned this pull request Jan 26, 2024

Messenger consume command with profile option is missing Performance statistics #53637

Closed

GromNaN reviewed Mar 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands #47416

[Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands #47416

HeahDude commented Aug 28, 2022 •

edited

carsonbot commented Aug 29, 2022

HeahDude commented Sep 13, 2022

94noni left a comment

HeahDude commented Oct 22, 2022

chalasr left a comment

HeahDude commented Oct 22, 2022

94noni commented Oct 26, 2022

Jeroeny commented Nov 22, 2022

94noni commented Nov 22, 2022

maxhelias commented Nov 27, 2022

94noni left a comment

javiereguiluz commented Jan 13, 2023

adrienbrault commented Mar 24, 2023

nicolas-grekas commented Oct 17, 2023

nicolas-grekas left a comment

nicolas-grekas commented Oct 17, 2023

HeahDude commented Oct 17, 2023

GromNaN Mar 28, 2024

[Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands #47416

[Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands #47416

Conversation

HeahDude commented Aug 28, 2022 • edited

Summary

Previous work

Screenshots

Opt-in profiling

Future scopes

Limitations

TODO

carsonbot commented Aug 29, 2022

HeahDude commented Sep 13, 2022

94noni left a comment

Choose a reason for hiding this comment

HeahDude commented Oct 22, 2022

chalasr left a comment

Choose a reason for hiding this comment

HeahDude commented Oct 22, 2022

94noni commented Oct 26, 2022

Jeroeny commented Nov 22, 2022

94noni commented Nov 22, 2022

maxhelias commented Nov 27, 2022

94noni left a comment

Choose a reason for hiding this comment

javiereguiluz commented Jan 13, 2023

adrienbrault commented Mar 24, 2023

nicolas-grekas commented Oct 17, 2023

nicolas-grekas left a comment

Choose a reason for hiding this comment

nicolas-grekas commented Oct 17, 2023

HeahDude commented Oct 17, 2023

GromNaN Mar 28, 2024

Choose a reason for hiding this comment

HeahDude commented Aug 28, 2022 •

edited