search

binaryseed / new_relic_absinthe

Absinthe Instrumentation for the New Relic Elixir Agent
https://hex.pm/packages/new_relic_absinthe
Apache License 2.0
19 stars 13 forks source link

Middleware does not play well with NewRelic.Phoenix.Transaction #4

Closed barthez closed 5 years ago

barthez commented 5 years ago

Hello @binaryseed,

I noticed that transaction name isn't set properly in the middleware when I use NewRelic.Phoenix.Transaction.

I noticed that middleware sets framework_name transaction attribute, but it is being overwritten in NewRelic.Phoenix.Transtion.Plug.before_send.

I think there are 2 possible solutions:

  1. Disallow overwriting framework_name attribute
  2. Set custom_name in Absinthe middleware (which has bigger precedence over framework_name in transform_name_attrs.

I can implement it, just tell me which solution you find better.

binaryseed commented 5 years ago

This is tricky since both frameworks are involved in the request - the question is what do you want the transaction named after - the Phoenix based name or the Absinthe based name...

custom_name is reserved for end users (via the NewRelic.set_transaction_name())

barthez commented 5 years ago

I believe that fist framework (deepest in the call stack) should be the one to sign off the transaction (give it its name).

Maybe it is possible to make framework_name attribute writable only once per transaction?

binaryseed commented 5 years ago

There's no way to prevent it from being written more than once, but I could store it as a list and grab just the first one.

binaryseed commented 5 years ago

~I think it might be ideal to simply set the Phoenix name earlier in the stack and leave the behavior as is~

Actually that won't work, we don't know the controller yet in the call, only in before_send

binaryseed commented 5 years ago

Upon reflection, it turns out that the first framework to set a name isn't necessarily the "deepest in the call stack", it actually just reflects where the instrumentation was inserted, not something semantic.

Nor can we really assume that the first is the "correct" one to choose - the first will actually be Plug instrumentation here, which wouldn't provide what you want either.

For now, your best be is to set a name yourself with NewRelic.set_transaction_name