Apollo performance tracing not working with nestjs graphql

kaufmo commented 2 years ago

Is there an existing issue for this?

[X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
[X] I have reviewed the documentation https://docs.sentry.io/
[X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Self-hosted/on-premise

Which package are you using?

@sentry/node

SDK Version

7.13.0

Framework Version

@nestjs/graphql: ^10.0.0
@nestjs/apollo: ^10.0.0
apollo-server-core": ^3.3.0
apollo-server-express: ^3.3.0

Link to Sentry event

No response

Steps to Reproduce

Im running a nestjs graphql api which uses apollo-server behind. I want to integrate the Apollo tracing: https://docs.sentry.io/platforms/node/performance/database/opt-in/#apollo-server-integration but I'm always getting an error.

It looks like the problem is that the integration cannot find the resolvers in constructSchema? See here my resolvers are empty https://github.com/getsentry/sentry-javascript/blob/master/packages/tracing/src/integrations/node/apollo.ts#L49

Can somebody help me how can i fix that?

Expected Result

Expected result would be getting tracing into my sentry instance :sweat_smile:

Actual Result

Error on API startup

This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason:
TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at /dist/sentry.ts:58:28
    at Array.map (<anonymous>)
    at ApolloServer.<anonymous> (/dist/sentry.ts:56:51)
    at new ApolloServerBase (xxx/node_modules/apollo-server-core/src/ApolloServer.ts:330:18)
    at new ApolloServer (xxx/node_modules/apollo-server-express/src/ApolloServer.ts:55:1)
    at ApolloDriver.registerExpress (xxx/node_modules/@nestjs/apollo/dist/drivers/apollo-base.driver.js:77:30)
    at ApolloDriver.registerServer (xxx/node_modules/@nestjs/apollo/dist/drivers/apollo.driver.js:38:24)
    at ApolloDriver.start (xxx/node_modules/@nestjs/apollo/dist/drivers/apollo.driver.js:23:20)
    at GraphQLModule.onModuleInit (xxx/node_modules/@nestjs/graphql/dist/graphql.module.js:105:9)

AbhiPrasad commented 2 years ago

Hey, thanks for writing in! This is super strange - @nestjs/apollo should be using apollo-server-core under the hood. Could you try upgrading your apollo-server-core? Perhaps that will help.

kaufmo commented 2 years ago

Hi @AbhiPrasad, I've upgrade both to the newest version but that didn't helped. Do you have another idea?

"apollo-server-core": "^3.10.2",
"apollo-server-express": "^3.10.2",

onurtemizkan commented 2 years ago

@kaufmo, could you create a reproduction for us to debug this further?

kaufmo commented 2 years ago

@onurtemizkan yes for sure i can create a simple repo for that, but we are using a selfhosted sentry instance which i cannot use here. Is it ok when you have to set dsn manually?

onurtemizkan commented 2 years ago

Is it ok when you have to set dsn manually?

That's totally fine @kaufmo, thanks.

kaufmo commented 2 years ago

@onurtemizkan I've created the repo: https://github.com/kaufmo/nestjs-sentry-apollo-example. When you start the api with yarn start:dev you will see the error. When you comment out the apollo integration then everything works fine. Is that ok for you to debug?

This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason:
TypeError: Cannot convert undefined or null to object
    at Function.keys (<anonymous>)
    at /xxx/dev/nestjs-sentry-apollo-example/node_modules/@sentry/tracing/cjs/integrations/node/apollo.js:42:18
    at Array.map (<anonymous>)
    at ApolloServer.<anonymous> (/xxx/dev/nestjs-sentry-apollo-example/node_modules/@sentry/tracing/cjs/integrations/node/apollo.js:41:43)
    at new ApolloServerBase (/xxx/dev/nestjs-sentry-apollo-example/node_modules/apollo-server-core/src/ApolloServer.ts:330:18)
    at new ApolloServer (/xxx/dev/nestjs-sentry-apollo-example/node_modules/apollo-server-express/src/ApolloServer.ts:55:1)
    at ApolloDriver.registerExpress (/xxx/dev/nestjs-sentry-apollo-example/node_modules/@nestjs/apollo/dist/drivers/apollo-base.driver.js:77:30)
    at ApolloDriver.registerServer (/xxx/dev/nestjs-sentry-apollo-example/node_modules/@nestjs/apollo/dist/drivers/apollo.driver.js:38:24)
    at ApolloDriver.start (/xxx/dev/nestjs-sentry-apollo-example/node_modules/@nestjs/apollo/dist/drivers/apollo.driver.js:23:20)
    at GraphQLModule.onModuleInit (/xxx/dev/nestjs-sentry-apollo-example/node_modules/@nestjs/graphql/dist/graphql.module.js:105:9)

onurtemizkan commented 2 years ago

Thanks for the reproduction @kaufmo! I was able to reproduce and debug the issue.

Apollo integration only supports ApolloServer instances constructed with a resolvers array provided.

It seems there are two other ways to create an ApolloServer instance:

Providing a pre-constructed schema (apparently, Nestjs is doing this)
Providing a modules object

So, resolvers array we iterate on is in fact optional, and we should fix this, logging a warning message if resolvers is not available. I'll open a PR to fix it.

I'm also opening another issue about adding support for schema and module.

AbhiPrasad commented 2 years ago

Let's leave this issue open until we fix the NestJS specific problem, thanks for opening a PR about bug fix though @onurtemizkan!

kaufmo commented 2 years ago

@onurtemizkan thank you very much that sounds good. Do you now when the other issue is fixed? It would be really nice when I could use this :)

andreialecu commented 1 year ago

I have a custom implementation of tracing with NestJS Apollo or Mercurius + mongoose here, if anyone wants to use it for inspiration:

https://gist.github.com/andreialecu/40cb13c01b0d88c8163bbec3de59c580

Works great, and properly traces async boundaries using AsyncLocalStorage.

Nickersoft commented 1 year ago

Just to chime in here – I too am using NestJS with a code-first GraphQL schema, and I was skeptical about whether my resolvers were actually getting picked up (Sentry only ever logged GraphQLTransactions, no spans), so I printed out the this.config.resolvers variable and for some reasons the entire object is just the Upload type from the graphql-upload package that I'm using:

{
  Upload: GraphQLScalarType {
    name: 'Upload',
    description: 'The `Upload` scalar type represents a file upload.',
    specifiedByURL: undefined,
    serialize: [Function: serialize],
    parseValue: [Function: parseValue],
    parseLiteral: [Function: parseLiteral],
    extensions: [Object: null prototype] {},
    astNode: undefined,
    extensionASTNodes: []
  }
}

I placed by Sentry init at the top of my main.ts' bootstrap() function and after the NestFactory.create call and even tried putting it in its own module that uses the onApplicationBootstrap interface. On the first two, I still only got the Upload type, and on the last, nothing printed. I'm not sure where Sentry needs to be initialized to pick up on Nest's resolvers, but for now I guess I'll continue using the custom Apollo plugin from the Sentry blog. Was really excited when I saw Sentry has built-in support now for Apollo performance tracing 😞

ThallesP commented 9 months ago

If anyone stumbled into this issue and you're using code first, try the plugin mentioned by @Nickersoft

And also, here's an updated version of the article mentioned:

import { ApolloServerPlugin } from "@apollo/server";
import { Transaction, startTransaction } from "@sentry/node";
// Move this to another file if desirable
const plugin: ApolloServerPlugin<{ transaction: Transaction }> = {
    async requestDidStart(requestContext) {
        if (requestContext.request.operationName) {
            requestContext.contextValue.transaction.setName(
                requestContext.request.operationName,
            );
        }

        return {
            async willSendResponse(requestContext) {
                requestContext.contextValue.transaction.finish();
            },
            async executionDidStart() {
                return {
                    willResolveField(requestContext) {
                        const span = requestContext.contextValue.transaction.startChild({
                            op: "resolver",
                            description: `${requestContext.info.parentType.name}.${requestContext.info.fieldName}`,
                        });
                        return () => {
                            span.finish();
                        };
                    },
                };
            },
        };
    },
};

@Module({
    imports: [
        GraphQLModule.forRoot({
            plugins: [plugin],
            context: () => ({
                transaction: startTransaction({
                    op: "gql",
                    name: "GraphQLTransaction", // default name in case query doesn't contain a name
                }),
            }),
        }),
    ],
})
export class Test {}

davx1992 commented 7 months ago

@ThallesP you are using in the code depreciated methods. Have you updated your method so far?

getsentry / sentry-javascript