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

Fix laravel transaction naming (issue #86) - Don't use getName() if it's a auto generated route name #174

Merged

Conversation

stockalexander
Copy link
Contributor

Route::getName() returns a random generated string (generated::*) if a route isn't named (Named Routes) and routes are cached via php artisan route:cache since laravel 7.x.

This causes the problem mentioned in #86. To fix the problem, I added a check if the returned string begins with generated::, to skip it and use Route::getAction() instead.

@CLAassistant
Copy link

CLAassistant commented Jun 7, 2021

CLA assistant check
All committers have signed the CLA.

@nr-opensource-bot
Copy link

Can one of the admins verify this patch?

@stockalexander stockalexander changed the title Laravel: Don't use Route::getName() for transaction naming, if it's a randomly generated string Fix laravel transaction naming (issue #86) - Don't use getName() if it's a auto generated route name Jun 15, 2021
Fahmy-Mohammed
Fahmy-Mohammed previously approved these changes Jul 30, 2021
Copy link
Contributor

@Fahmy-Mohammed Fahmy-Mohammed left a comment

Choose a reason for hiding this comment

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

Thanks for adding this fix @stockalexander. I've tested this locally and it works well! This should be included in the upcoming PHP agent release (version 9.18).

@Fahmy-Mohammed
Copy link
Contributor

@stockalexander before we can merge this, would you be able to change this to the dev branch instead of main? That is where all of the work included in the upcoming release is contained.

@stockalexander
Copy link
Contributor Author

@Fahmy-Mohammed: Nice! I rebased my branch to the dev branch (+ force pushed it) and updated the PR. Can you merge it now or should I change something else?

@Fahmy-Mohammed
Copy link
Contributor

@stockalexander There are some internal Jenkins builds that are running currently, so I'll merge this once those tests pass. Awesome work identifying and adding the fix. We really appreciate it!

@Fahmy-Mohammed Fahmy-Mohammed merged commit 5cf1b2e into newrelic:dev Jul 30, 2021
Fahmy-Mohammed pushed a commit that referenced this pull request Aug 3, 2021
zsistla added a commit to zsistla/docs-website that referenced this pull request Aug 23, 2021
---
subject: PHP agent
releaseDate: '2021-08-23'
version: 9.18.1.303
downloadLink: 'https://download.newrelic.com/php_agent/archive/9.18.1.303'
---

## New Relic PHP Agent v9.18.1 ##

### New Features ###
* [Added](newrelic/newrelic-php-agent#162) a docker development environment. It's now possible for contributors to both develop and test (unit tests and integration tests) without setting up a specific environment on their own system. Please see our [documentation](https://github.com/newrelic/newrelic-php-agent/blob/main/docs/dev_environment.md) for more information.
* [Route caching in `Laravel 7.x` is now supported!](newrelic/newrelic-php-agent#174). Transaction naming now works with routes cached via `php artisan route:cache`. @stockalexander, thanks for your contribution!
* `Redis::mget` and `Redis::mset` [functions are now supported](newrelic/newrelic-php-agent#156). @b-viguier, thanks for your contribution!

### Bug Fixes ###
* [Fixed](newrelic/newrelic-php-agent#161) instances where a memory leak was occurring with our `curl multi` instrumentation.
* [Fixed](newrelic/newrelic-php-agent#176) an issue where a supportability metric used to track an edge case was causing a segfault.
* [Fixed](newrelic/newrelic-php-agent#87) an issue where PHP versions with an unknown API version were incorrectly handled during Debian package install.
* [Fixed](newrelic/newrelic-php-agent#164) instances where `parent.transportDuration` values are `0` for transactions between two PHP applications instrumented through distributed tracing. @b-viguier, thanks for your contribution!
* [Fixed](newrelic/newrelic-php-agent#158) an issue where the `newrelic.ini` configuration file was incorrectly installed. @b-viguier, thanks for your contribution!

### Support Statement ###
* New Relic recommends that you upgrade the agent regularly and at a minimum every 3 months. As of this release, the oldest supported version is [8.6.0](/docs/release-notes/agent-release-notes/php-release-notes/php-agent-860238/).
zuluecho9 pushed a commit to zsistla/docs-website that referenced this pull request Aug 23, 2021
---
subject: PHP agent
releaseDate: '2021-08-23'
version: 9.18.1.303
downloadLink: 'https://download.newrelic.com/php_agent/archive/9.18.1.303'
---

## New Relic PHP Agent v9.18.1 ##

### New Features ###
* [Added](newrelic/newrelic-php-agent#162) a docker development environment. It's now possible for contributors to both develop and test (unit tests and integration tests) without setting up a specific environment on their own system. Please see our [documentation](https://github.com/newrelic/newrelic-php-agent/blob/main/docs/dev_environment.md) for more information.
* [Route caching in `Laravel 7.x` is now supported!](newrelic/newrelic-php-agent#174). Transaction naming now works with routes cached via `php artisan route:cache`. @stockalexander, thanks for your contribution!
* `Redis::mget` and `Redis::mset` [functions are now supported](newrelic/newrelic-php-agent#156). @b-viguier, thanks for your contribution!

### Bug Fixes ###
* [Fixed](newrelic/newrelic-php-agent#161) instances where a memory leak was occurring with our `curl multi` instrumentation.
* [Fixed](newrelic/newrelic-php-agent#176) an issue where a supportability metric used to track an edge case was causing a segfault.
* [Fixed](newrelic/newrelic-php-agent#87) an issue where PHP versions with an unknown API version were incorrectly handled during Debian package install.
* [Fixed](newrelic/newrelic-php-agent#164) instances where `parent.transportDuration` values are `0` for transactions between two PHP applications instrumented through distributed tracing. @b-viguier, thanks for your contribution!
* [Fixed](newrelic/newrelic-php-agent#158) an issue where the `newrelic.ini` configuration file was incorrectly installed. @b-viguier, thanks for your contribution!

### Support Statement ###
* New Relic recommends that you upgrade the agent regularly and at a minimum every 3 months. As of this release, the oldest supported version is [8.6.0](/docs/release-notes/agent-release-notes/php-release-notes/php-agent-860238/).
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.

None yet

4 participants