digiaonline / lumen-newrelic

New Relic instrumentation for the Lumen framework
MIT License
28 stars 21 forks source link

Transactions not showing up in New Relic #26

Closed BigDaneLane closed 3 years ago

BigDaneLane commented 3 years ago

What did you do?

Upgrade to 3.1.0

What did you expect to happen?

My transactions to show up in New Relic

What actually happened?

My transactions did not show up.

What versions of relevant language, framework and this module are you using?

nordsoftware/lumen-newrelic: "3.1.0" laravel/lumen-framework: "8.1" php: "7.3"

In NewRelicMiddleware.php on line 46 newRelic->setAppName is called:

$this->newRelic->setAppName(
    config('newRelic.application_name'),
    config('newRelic.license')
);

Which calls this function:

public function setAppName( $name, $license = "", $xmit = false )
{
     return $this->call( 'newrelic_set_appname', array( $name, $license, $xmit ) );
}

The docblock states:

* The xmit flag is new in version 3.1 of the agent. Usually, when you change an application name, the agent simply
* discards the current transaction and does not send any of the accumulated metrics to the daemon. However, if you
* want to record the metric and transaction data up to the point at which you called this function, you can specify
* a value of true for this argument to make the agent send the transaction to the daemon. This has a very slight
* performance impact as it takes a few milliseconds for the agent to dump its data. By default this parameter is
* false.

So when the NewRelicMiddleware is loaded (on the way out) the transaction up to that point is dropped. So the transactionName is not showing up in new relic.

When I change the $xmit parameter to 'true' the transaction does show up in New Relic. If I completely remove the '$this->newRelic->setAppName()'-function the transaction is also showing up.

$this->newRelic->setAppName(
   config('newRelic.application_name'),
   config('newRelic.license'),
   true
);

On my server New Relic is already set up before creating the lumen app. So when calling the setAppName() it drops the transaction up to that point.

I've spend way too much time on this and hope by creating this issue it prevents other people from wasting time. I will stay on v3.0.0 which works for me

Jalle19 commented 3 years ago

@miff2000 any chance you could fix this?

miff2000 commented 3 years ago

Yep, happy to! @BigDaneLane thanks for raising the issue 👍

Jalle19 commented 3 years ago

I haven't set application name this way personally so I'm not sure what the best fix is. Setting the third parameter to true seems to be an easy solution, and potentially the most correct one.

BigDaneLane commented 3 years ago

I was also thinking about the best way to fix it. For people like me who have New Relic config already set up setting the third parameter true might be a performance impact like the docblock states. But there is no way to check if New Relic is already set up, and if this is the desired setting. Maybe add a variable to the config/newRelic.php file to check if the appName needs to be customised?

<?php
return [
~~~~
    'custom_settings' => env('NEW_RELIC_CUSTOM_SETTINGS', false),
~~~~

And in the NewRelicMiddleware:

    public function handle(Request $request, Closure $next)
    {
        // We must let the response get handled before naming the transaction, otherwise the necessary route i
        // information won't be available in the request object.
        $response = $next($request);

        $this->newRelic->nameTransaction($this->getTransactionName($request));
        if(config('newRelic.custom_settings')){
            $this->newRelic->setAppName(
                config('newRelic.application_name'),
                config('newRelic.license'),
                true
            );
        }

        return $response;
    }

Btw, thanks for the great work guys!

miff2000 commented 3 years ago

I would agree, it should be a setting that is overridden from defaults if there is a performance impact, and a blurb in the docs for it. This functionality was only just added so the impact will be minimal.

I can take care of that - you've done all the hard work for me anyway @BigDaneLane :)

Jalle19 commented 3 years ago

https://packagist.org/packages/nordsoftware/lumen-newrelic#3.1.1