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

Remove extracting route name or controller for transaction names #583

Merged
merged 3 commits into from Oct 11, 2022

Conversation

stayallive
Copy link
Collaborator

@stayallive stayallive commented Oct 7, 2022

After some discussion it might be best if we only show the uri (parameterized URL) as a transaction name instead of defaulting to the developer provided route name or controller class FQN.

This is slated for 3.x and technically not a breaking change but it will change almost all transaction names for most users.


For illustration purposes this is how a Laravel app currently looks when using mixed route names and controllers:

image

@cleptric cleptric mentioned this pull request Oct 11, 2022
19 tasks
@cleptric cleptric self-assigned this Oct 11, 2022
@cleptric cleptric merged commit 78817c5 into tracing-updates Oct 11, 2022
@cleptric cleptric deleted the remove-route-name-controller-extraction branch October 11, 2022 12:11
cleptric added a commit that referenced this pull request Oct 12, 2022
@denisdulici
Copy link

After this change, different and unrelated actions/events will be grouped under the same transaction name. For example, the index (GET) action and store (POST) action:

https://laravel.com/docs/9.x/controllers#actions-handled-by-resource-controller

I guess this will cause a lot of confusion.

@stayallive
Copy link
Collaborator Author

They are not, they are still grouped by their HTTP method.

image

@denisdulici
Copy link

Hmm, you're right. While they're not as cool/clear as before, they're not mixed:

Screenshot_1095

Thanks for the prompt response 🙏

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

3 participants