Skip to content

Conversation

FeBe95
Copy link

@FeBe95 FeBe95 commented Sep 4, 2025

Description

Deletes the ->removeTag('command') call in the CommandFinished event handler.

Reasoning

Many of our Sentry events show -- for the command tag, even though they are coming from a command execution. The command is only present when ->captureMessage() or ->captureException() was used, or when the error is caused by PHP itself (e.g. Undefined array key "foo").

When manually throwing errors with throw new ... in the code, the CommandFinished event seems to be still fired at the end, and thus the command tag is deleted.

I see no reason why the command tag shouldn't remain in the payload that is sent to Sentry.

@cleptric cleptric requested a review from stayallive September 5, 2025 11:21
@stayallive
Copy link
Collaborator

stayallive commented Sep 5, 2025

You can also run commands from inside requests I believe which fire the same events, there we don't want to keep the tag. I see what you mean though and we should fix this but simply always leaving the tag might not be the solution here.

@FeBe95
Copy link
Author

FeBe95 commented Sep 5, 2025

I had a look into the available console events in Laravel and it seems like we could utilize the ArtisanStarting event:

Events

Artisan dispatches three events when running commands: Illuminate\Console\Events\ArtisanStarting, Illuminate\Console\Events\CommandStarting, and Illuminate\Console\Events\CommandFinished. The ArtisanStarting event is dispatched immediately when Artisan starts running. Next, the CommandStarting event is dispatched immediately before a command runs. Finally, the CommandFinished event is dispatched once a command finishes executing.

My idea is as follows:

  1. ArtisanStarting: Keep track that this event was fired (e.g. by using Laravel's Context class)
  2. CommandStarting: Set the command tag (only if the tag is still empty, so we don't override the tag when a command runs another command)
  3. CommandFinished: Only remove the command tag, if ArtisanStarting wasn't called before

Or we choose a completely different approach, that doesn't use the ArtisanStarting event:

  1. CommandStarting: Set the command tag (only if the tag is still empty, so we don't override the tag when a command runs another command)
  2. CommandFinished: Only remove the command tag, if app()->runningInConsole() is true.

What do you think, @stayallive?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants