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

[Console] Add SignalMap to map signal value to its name #50663

Merged
merged 1 commit into from Jun 20, 2023

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Jun 14, 2023

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

I'm migrating some code to use new SF features, but I have a very little regression.

Before :

    private function setUpSignals(): void
    {
        pcntl_signal(\SIGTERM, function () {
            $this->stop('SIGTERM');
        });
        pcntl_signal(\SIGINT, function () {
            $this->stop('SIGINT');
        });

        pcntl_async_signals(true);
    }

Now:

    public function handleSignal(int $signal): void
    {
        // I need the following line
        $signal = SignalMap::getSignalName($signal);
        $this->stop("Signal {$signal} received.");
    }

As you can see, now in my log I have an int (2, 15), and not anymore SIGTERM, SIGINT.

@lyrixx lyrixx changed the title Add SignalMap to map signal value to its name [Console] Add SignalMap to map signal value to its name Jun 14, 2023
@lyrixx lyrixx added the Console label Jun 14, 2023
@fabpot
Copy link
Member

fabpot commented Jun 20, 2023

Thank you @lyrixx.

@fabpot fabpot merged commit 16014de into symfony:6.4 Jun 20, 2023
5 of 9 checks passed
@lyrixx lyrixx deleted the console-signal-helper branch June 20, 2023 11:24
nicolas-grekas added a commit that referenced this pull request Oct 17, 2023
…le] Enable profiling commands (HeahDude)

This PR was merged into the 6.4 branch.

Discussion
----------

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

| 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:
* #10374
* #14809 (see #14809 (comment))
* #20502

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

<details><summary>Search view</summary>
<img width="1221" alt="Screenshot 2022-08-28 at 11 29 25 PM" src="https://user-images.githubusercontent.com/10107633/187095381-851f6be5-cf8c-4fec-aa7b-9f9f80bf8404.png">
</details>
<details><summary>Command panel</summary>
<img width="1210" alt="Screenshot 2022-08-28 at 11 30 54 PM" src="https://user-images.githubusercontent.com/10107633/187095971-de8f9b85-eeb4-48cf-aff7-fdac0c6f9264.png">
<img width="974" alt="Screenshot 2022-08-28 at 11 31 08 PM" src="https://user-images.githubusercontent.com/10107633/187095980-337f4373-ebe5-4de5-bfb4-3715be868274.png">
<img width="962" alt="Screenshot 2022-08-28 at 11 31 21 PM" src="https://user-images.githubusercontent.com/10107633/187096022-ab18f70a-704a-4c75-81a6-43ca5b66eb9a.png">
<img width="964" alt="Screenshot 2022-08-28 at 11 31 34 PM" src="https://user-images.githubusercontent.com/10107633/187096037-cc45805e-ba65-447f-bca6-2d2ea38239b8.png">

If the command is signal-able the following panel will be available:
<img width="961" alt="Screenshot 2022-08-28 at 11 31 46 PM" src="https://user-images.githubusercontent.com/10107633/187096084-2f6a39be-a780-411b-9000-b9ae3407e82b.png">

If sub commands are run using `$this->getApplication()->run()` sub profiles will be shown as for requests:
<img width="696" alt="Screenshot 2022-08-28 at 11 31 56 PM" src="https://user-images.githubusercontent.com/10107633/187096105-bb7e4a84-42bc-47ed-9f58-527a771c48cc.png">
</details>

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

<details><summary>Performance panel</summary>
<img width="977" alt="Screenshot 2022-08-28 at 11 32 23 PM" src="https://user-images.githubusercontent.com/10107633/187096138-3ff3f347-61c7-4ade-8c73-b48d5b504c04.png">
<img width="969" alt="Screenshot 2022-08-28 at 11 32 32 PM" src="https://user-images.githubusercontent.com/10107633/187096168-35be4773-4941-4e5e-8dd4-f6cc009e5d48.png">
</details>
<details>
<summary>Failing command</summary>
The exception panel is shown by default as for requests:
<img width="1217" alt="Screenshot 2022-08-28 at 11 33 42 PM" src="https://user-images.githubusercontent.com/10107633/187096210-7b206c72-c2e4-4eb3-9978-916cd3dd6cd6.png">
</details>

<details>
<summary>Sub command</summary>
<img width="1217" alt="Screenshot 2022-08-28 at 11 33 19 PM" src="https://user-images.githubusercontent.com/10107633/187096188-a090fb91-b7b8-4f98-a1d7-99b3605bf48b.png">
</details>

<details>
<summary>Profile token when verbose</summary>
(clickable links with compatible terminals)
<img width="534" alt="Screenshot 2022-08-28 at 11 26 51 PM" src="https://user-images.githubusercontent.com/10107633/187096349-8f7619b2-feb4-427c-a315-f4a844536316.png">
</details>

<details>
<summary>Command interrupted by signal</summary>
<img width="1246" alt="Screenshot 2022-10-22 at 4 16 37 PM" src="https://user-images.githubusercontent.com/10107633/197344164-50d72a25-a6e7-4e77-ad87-2d5f54b29b93.png">
</details>

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 symfony/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 #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
------

* [x] I've left some todos inside the code for reviewers to share they thought before I try going further
* [x] 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~

Commits
-------

82914ba [Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request Oct 17, 2023
…le] Enable profiling commands (HeahDude)

This PR was merged into the 6.4 branch.

Discussion
----------

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

| 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:
* #10374
* #14809 (see symfony/symfony#14809 (comment))
* #20502

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

<details><summary>Search view</summary>
<img width="1221" alt="Screenshot 2022-08-28 at 11 29 25 PM" src="https://user-images.githubusercontent.com/10107633/187095381-851f6be5-cf8c-4fec-aa7b-9f9f80bf8404.png">
</details>
<details><summary>Command panel</summary>
<img width="1210" alt="Screenshot 2022-08-28 at 11 30 54 PM" src="https://user-images.githubusercontent.com/10107633/187095971-de8f9b85-eeb4-48cf-aff7-fdac0c6f9264.png">
<img width="974" alt="Screenshot 2022-08-28 at 11 31 08 PM" src="https://user-images.githubusercontent.com/10107633/187095980-337f4373-ebe5-4de5-bfb4-3715be868274.png">
<img width="962" alt="Screenshot 2022-08-28 at 11 31 21 PM" src="https://user-images.githubusercontent.com/10107633/187096022-ab18f70a-704a-4c75-81a6-43ca5b66eb9a.png">
<img width="964" alt="Screenshot 2022-08-28 at 11 31 34 PM" src="https://user-images.githubusercontent.com/10107633/187096037-cc45805e-ba65-447f-bca6-2d2ea38239b8.png">

If the command is signal-able the following panel will be available:
<img width="961" alt="Screenshot 2022-08-28 at 11 31 46 PM" src="https://user-images.githubusercontent.com/10107633/187096084-2f6a39be-a780-411b-9000-b9ae3407e82b.png">

If sub commands are run using `$this->getApplication()->run()` sub profiles will be shown as for requests:
<img width="696" alt="Screenshot 2022-08-28 at 11 31 56 PM" src="https://user-images.githubusercontent.com/10107633/187096105-bb7e4a84-42bc-47ed-9f58-527a771c48cc.png">
</details>

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

<details><summary>Performance panel</summary>
<img width="977" alt="Screenshot 2022-08-28 at 11 32 23 PM" src="https://user-images.githubusercontent.com/10107633/187096138-3ff3f347-61c7-4ade-8c73-b48d5b504c04.png">
<img width="969" alt="Screenshot 2022-08-28 at 11 32 32 PM" src="https://user-images.githubusercontent.com/10107633/187096168-35be4773-4941-4e5e-8dd4-f6cc009e5d48.png">
</details>
<details>
<summary>Failing command</summary>
The exception panel is shown by default as for requests:
<img width="1217" alt="Screenshot 2022-08-28 at 11 33 42 PM" src="https://user-images.githubusercontent.com/10107633/187096210-7b206c72-c2e4-4eb3-9978-916cd3dd6cd6.png">
</details>

<details>
<summary>Sub command</summary>
<img width="1217" alt="Screenshot 2022-08-28 at 11 33 19 PM" src="https://user-images.githubusercontent.com/10107633/187096188-a090fb91-b7b8-4f98-a1d7-99b3605bf48b.png">
</details>

<details>
<summary>Profile token when verbose</summary>
(clickable links with compatible terminals)
<img width="534" alt="Screenshot 2022-08-28 at 11 26 51 PM" src="https://user-images.githubusercontent.com/10107633/187096349-8f7619b2-feb4-427c-a315-f4a844536316.png">
</details>

<details>
<summary>Command interrupted by signal</summary>
<img width="1246" alt="Screenshot 2022-10-22 at 4 16 37 PM" src="https://user-images.githubusercontent.com/10107633/197344164-50d72a25-a6e7-4e77-ad87-2d5f54b29b93.png">
</details>

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 symfony/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 symfony/symfony#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
------

* [x] I've left some todos inside the code for reviewers to share they thought before I try going further
* [x] 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~

Commits
-------

82914bab0d [Console][FrameworkBundle][HttpKernel][WebProfilerBundle] Enable profiling commands
This was referenced Oct 21, 2023
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.

None yet

6 participants