useNewRelic: Proper setup?

[zawadzkip]

Hi all,im trying to add the useNewRelic plugin to my app right now and I'm running into problem. Unsure if it is a bug or a setup problem.
"@envelop/core": "2.3.3", "@envelop/newrelic": "4.0.2",

"newrelic": "8.14.1",

(node:55931) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'agent' of undefined at /Users/zawadzki/Documents/graphql-gateway/node_modules/@envelop/newrelic/index.js:65:18 at processTicksAndRejections (internal/process/task_queues.js:95:5)

const { envelop, useSchema, useTiming } = require('@envelop/core')
const { useNewRelic } = require('@envelop/newrelic')

function graphqlHandler(schema) {
  // initialize graphql plugins
  const envelopPlugins = [
    useTiming({
      skipIntrospection: true
    }),
    useSchema(schema),
    useNewRelic()
  ]

  const getEnveloped = envelop({
    plugins: envelopPlugins
  })

  // return route handler
  return async function graphql(req, res) {
    const request = {
      body: req.body,
      headers: req.headers,
      method: req.method,
      query: req.query
    }
    // Passing request into the getEnveloped function ensures that the incoming request data will be on the context for the executor
    const { parse, validate, contextFactory, execute, schema } = getEnveloped({ request })
    if (shouldRenderGraphiQL(request)) {
      res.type('text/html')
      res.send(renderGraphiQL())
      return
    }
    const { operationName, query, variables } = getGraphQLParameters(request)
    const result = await processRequest({
      operationName,
      query,
      variables,
      request,
      schema,
      parse,
      validate,
      execute,
      contextFactory
    })

    sendResult(result, res.raw)
  }
}

module.exports = graphqlHandler

[zawadzkip]

Figured it out :)

I am deploying this to a non-prod environment that we don't have the agent enabled in based on some preconfigured new relic rules, so the agent code above ends up failing.

I am updating the code so the plugin only gets loaded when that agent is enabled.

[zawadzkip]

@Urigo after many days I have finally gotten around to making a small update 😂

https://github.com/zawadzkip/envelop/pull/1/files

I see that the standard contribution process requires an issue to be created for the change prior to the MR being made. Im not sure where to place this as an issue because its not really a "bug" or a major "feature". I'm happy to make an MR from my fork into your repo once I hear back

[Urigo]

there is no need, you can just go ahead and submit your change!

[santino]

Hang on,
is this really something to handle in the plugin?

The plugin expects that you are able to talk to new relic, possibly this is something that can be mentioned more explicitly in the docs.

If this is not the case, shouldn't that be managed in the application?
You could have a condition to enable/disable the plugin, like

plugins: [
  useSchema(schema),
  useTiming(),
  ...(process.env.NODE_ENV === 'production' ? [useNewRelic(config.newrelic)] : []),
  // more plugins here
],

I mean it probably doesn't hurt to throw the error from the plugin, but I believe it's always best to not fire up plugins, especially if this involves instances from external libraries (in this case NewRelic) if you don't need them, depending on your environment.

[zawadzkip]

@santino I understand your point of view. So more specifically, what happened here when i discovered this was that the new relic plugin was installed but the agent itself wasn't enabled. (We disable agents in our infrastructure on certain environments through an environment variable)

So it was a bit confusing trying to debug the issue when I could see the package being loaded, but the agent was being disabled. I understand my error message isn't very clear about that. If I changed it to
throw new Error("Unable to find New Relic agent, please confirm the agent is installed and enabled") would that be better?

[santino]

Just commented on the PR, so we keep all the code feedback in a single place 😉