Automatic transaction names

[danwall]

Since Laravel 4.1 and the getCurrentRouteName() confusion in other issues I have also noticed another problem and would like to start a discussion on improving this functionality.

In 4.0, currentRouteName() used to return either the name of the route (e.g. "home") or the HTTP verb followed by the request path (e.g. "get /"). This would work on both controller methods as well as route closures.

In 4.1, this functionality has been changed so that currentRouteName() returns either the route name (e.g. "home") or null. If a route name is not defined, it returns null - this is the case for both controllers and route closures.

Request::path() returns the current requests path, without the HTTP verb.

Route::currentRouteAction() returns the controller method in the format "HomeController@getIndex" which is my preferred method as it does not rely on the route being named. One caveat with this is that it also returns null on route closures.

Perhaps we could have some sort of fallback if currentRouteAction() returns null, we then use Request::path()?

These are just my thoughts, would love to hear feedback from others.

[pleckey]

I still personally like the idea of using the route name when it's available - if you've given the route a meaningful name, why not make use of it?

But I also like the idea of going by controller method.

Perhaps a default of cascading fallbacks if null, like:

1. currentRouteName()
2. currentRouteAction()
3. HTTP verb + path()

... and also add the ability to pass a closure that will allow you to perform your own route naming?

Could add a config parameter that would default to null, or you could set a closure that will get passed Request, Response and App - registerNamedTransactions() in the ServiceProvider would then either implement the default naming method if the config param is null (or more specifically, if it isn't a function), otherwise register with the provided closure.

Just off the top of my head on a foggy New Years - comments welcome. :)

[danwall]

That sounds like an ideal solution to me. Gives us the best of everything really.

Can you actually set a closure as a config value though? I haven't seen this done before.

After taking a quick look, it looks like we can access the HTTP verb via Request::getMethod() which is from the underlying Symfony Request class.

Is there anything else that might be a useful addition when monitoring transactions?

[pleckey]

You can definitely set closures as array values, see my Gist here: https://gist.github.com/pleckey/8211883

The default Illuminate Config Repository class implements ArrayAccess and I don't see anything in there that would prevent the default array functionality from working: https://github.com/laravel/framework/blob/master/src/Illuminate/Config/Repository.php

... as you can see, offsetGet and offsetSet are pretty straightforward, don't seem to do any post-processing on the config values to prevent a closure from being passed.

It also makes no effort to detect closures (and it shouldn't), so we would need to add the is_callable condition to the Service Provider, something like:

$nameProvider = Config::get( 
    'laravel-newrelic::name_provider', 
    function( $request, $response, $app ) 
    { ... default name provider implementation ...}
);

if ( is_callable( $nameProvider ) ) $nameProvider( $request, $response, $app );

One of the other features I've been wanting to add is attaching to the Eloquent load / save / etc. events and track those as custom events in NR, so you would also be able to get model object read / write timings and counts. But that's a bit more work, and should be in a different issue / PR anyways.

[danwall]

I have attempted to implement this with the same coding style as the rest of the package but let me know if there's anything that needs changing.

I like the idea of hooking into Eloquent methods as well, that could be very interesting.