Support for Apollo Express GraphQL servers where schema is loaded using the schema property

[matt-rosenberg-mantra]

Problem Statement

My team is looking to add support for the known limitation mentioned under the Apollo Server Integration

Known Limitation: This integration does not support Apollo servers where the schema was loaded using the schema property (new ApolloServer({ schema: ... })).

We use Express with GraphQL which requires that we load our schemas via the schema property, and have observed that Sentry.getCurrentHub().getScope().getTransaction() is undefined for all GraphQL endpoints.

While this Sentry article 'Performance Monitoring in GraphQL' is helpful for pure GraphQL servers, we would ideally like to see our GraphQL resolvers (as well as relevant db and http.client spans inside of those GraphQL resolver) attached to the original Express-initiated transactions so that they can be viewed in the context of a single trace.

Solution Brainstorm

We have a workaround at the moment, but would like to know if our use of Sentry.makeMain(Hub) is a safe way to approach this problem, or whether you foresee any unexpected side effects. And also, whether this is something that could be included in future versions of your SDK.

Our workaround has 3 main steps:

1. Express middleware that adds the current hub to the express.Request (as req.sentryHub).
2. Apollo GraphQL context configuration that maps the req.sentryHub field to the GraphQL context.
3. Apollo GraphQL plugin that maps the graphql context's sentryHub as the current hub via makeMain(Hub).

The code looks something like so:

Main

const app = express();
Sentry.init({
  integrations: [
    new Sentry.Integrations.Express({ app }),
    // ... 
  ],
  // ...
});
app.use(Sentry.Handlers.requestHandler() as express.RequestHandler);
app.use(Sentry.Handlers.tracingHandler() as express.RequestHandler);
 // (1) Middleware that adds the current Sentry.Hub to express.Request
app.use((req, res, next) => {
  req.sentryHub = Sentry.getCurrentHub();
  next();
});

// ... other middleware

const server = new ApolloServer({
  schema,
  context: ({ req }: any) => ({ sentryHub: req.sentryHub }), // (2) add current hub to graphql context
  plugins: [ sentryGraphqlPlugin ], // (3) see 'Sentry GraphQL Plugin' below
  // ...
});
server.applyMiddleware({ app });
// ...

Sentry GraphQL Plugin

Based on Performance Monitoring in GraphQL

import { PluginDefinition } from 'apollo-server-core';

export const sentryGraphqlPlugin: PluginDefinition = {
  async requestDidStart({ context, request }) {
    // (3a) Pull the hub from the graphQL context and `makeMain`
    const hub = context.sentryHub;
    if (hub) {
      Sentry.makeMain(hub);
    }

    // (3b) We can now fetch the existing `transaction`
    const transaction = Sentry.getCurrentHub().getScope().getTransaction();

    // (3c) The rest is based on the 'Performance Monitoring in GraphQL' blog post
    if (request.operationName) {
      transaction?.setName(`${transaction.name} ${request.operationName}`);
    }
    return {
      async executionDidStart() {
        return {
          willResolveField({ info }) {
            const span = transaction?.startChild({
              op: 'resolver',
              description: `${info.parentType.name}.${info.fieldName}`,
            });
            return () => {
              span?.finish();
            };
          },
        };
      },
    };
  },
};

[Lms24]

Hi @matt-rosenberg-mantra, thanks for writing in!

So just to confirm:

• Your feature request is for our Apollo Server integration to handle the currently known limitation?
• You already have a workaround in place that solves this for your use case?

We have a workaround at the moment, but would like to know if our use of Sentry.makeMain(Hub) is a safe way to approach this problem

Generally, setting a hub manually as the current hub has implications, for instance that it could lead to scope bleed. This means that for example, tags or spans from other requests that are handled concurrently could potentially leak to the current request and data could be mixed up. This doesn't necessarily have to happen though - it depends a lot on your use case.

Generally, our Node SDK handles scope isolation per incoming request by using AsyncLocalStorage so this doesn't happen as long as you're not setting hubs manually.

I'm not sure I understand yet, why you need to set your hub on the request as opposed to just the transaction. Is your Sentry GraphQL Plugin running in a different process than the express server? Are there other things you need from the hub?

[matt-rosenberg-mantra]

Hi @Lms24, thanks for the reply!

• Your feature request is for our Apollo Server integration to handle the currently known limitation?

In short, yes.

I am assuming that my problem is because of the "known limitation" because we do load schemas in that fashion, but it could be also be a current limitation of the SDK when running Express+ApolloServer apps.

Whatever the specific solution (new feature or guided workaround), my goal is for the SDK to automatically track db and http.client spans as part of the Express transactions:

express tx
|_ express.middleware spans
|_ apollo-graphql-resolver span
  |_ db span
  |_ http.client span

• You already have a workaround in place that solves this for your use case?

Maybe yes? I suspect my workaround has some side effects I'm not yet aware of...

... why you need to set your hub on the request as opposed to just the transaction

By setting the hub on the request then calling makeMain(Hub), I seem to be able to make the ApolloServer request handlers aware of the existing transaction, wherein they automatically track db and http.client spans. My assumption is that the ApolloServer is handling http requests outside of the Express context (maybe using child processes?), and using makeMain(Hub) was really just a guess at how to bind those ApolloServer request handlers to the existing transaction.

If I only pass along the transaction, then I can only add new spans imperatively/programmatically.

[github-actions[bot]]

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!

---

"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

[Lms24]

maybe using child processes?

This would explain why the hub isn't set otherwise. However, I'm still surprised that our instrumentation would work then

By setting the hub on the request then calling makeMain(Hub), I seem to be able to make the ApolloServer request handlers aware of the existing transaction, wherein they automatically track db and http.client spans

Assuming that all other instrumentation is in place, I'm wondering if it would still be enough to pass along the transaction and then call

Sentry.getCurrentHub().getScope().setSpan(yourPassedTransaction)

This sets the transaction as the active one but doesn't mock around with the current hub. Just a thought though.

Your feature request is for our Apollo Server integration to handle the currently known limitation?

In short, yes.

We should look into this at some point in the future but at the moment this isn't high enough on our priority list. I'll backlog the issue for this aspect.

[parlato-vooma]

• Your feature request is for our Apollo Server integration to handle the currently known limitation?

We have the same need - we also use Express + Apollo Server and load our schemas via the schema property. We currently have little visibility into our resolvers' performance. I imagine many people are in need of the same.

@Lms24 Is a solution for this roadmapped at all?

@matt-rosenberg-mantra did your workaround end up working in the end? Any other learnings to share as we think about implementing the same?

[Lms24]

Hey @parlato-vooma, I'd recommend updating to the upcoming v8 SDK release once it's out (first beta is shipping today-ish). Version 8 will use OpenTelemetry instrumentation instead of our custom instrumentation for getting performance data so it's worth trying how things work better with OpenTelemetry data. There might still be problems (not saying this solves everything) but time-wise we can't look a this while working on the major. Once it's more stable and you gave it a try, we're happy to revisit things. Thanks :)

[matt-rosenberg-mantra]

@parlato-vooma Yes, the workaround has been working well to provide us with a complete picture of each request.

Here's an example that includes the db spans:

[matt-rosenberg-mantra]

@parlato-vooma And for learnings 🤔 , to be honest it has been working smoothly and we have not needed to make any significant changes since it was setup.

Hope it helps!

[AbhiPrasad]

We recommend using 8.x of the Sentry Node SDK to handle these setups - please upgrade your SDK! https://docs.sentry.io/platforms/javascript/guides/node/migration/v7-to-v8/

It includes auto-enabled integrations for:

• Express: https://docs.sentry.io/platforms/javascript/guides/express/
• GraphQL: https://docs.sentry.io/platforms/javascript/guides/express/configuration/integrations/graphql/