Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apollo performance tracing not working with nestjs graphql #5808

Closed
3 tasks done
kaufmo opened this issue Sep 23, 2022 · 14 comments · Fixed by #7194
Closed
3 tasks done

Apollo performance tracing not working with nestjs graphql #5808

kaufmo opened this issue Sep 23, 2022 · 14 comments · Fixed by #7194
Assignees

Comments

@kaufmo
Copy link

kaufmo commented Sep 23, 2022

Is there an existing issue for this?

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 😅

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
Copy link
Member

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
Copy link
Author

kaufmo commented Sep 26, 2022

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
Copy link
Collaborator

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

@kaufmo
Copy link
Author

kaufmo commented Sep 27, 2022

@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
Copy link
Collaborator

Is it ok when you have to set dsn manually?

That's totally fine @kaufmo, thanks.

@kaufmo
Copy link
Author

kaufmo commented Sep 28, 2022

@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
Copy link
Collaborator

onurtemizkan commented Sep 28, 2022

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:

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
Copy link
Member

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

@kaufmo
Copy link
Author

kaufmo commented Sep 29, 2022

@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
Copy link
Contributor

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
Copy link

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
Copy link

ThallesP commented Jan 8, 2024

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
Copy link

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

@wp-harm
Copy link

wp-harm commented Dec 5, 2024

I still couldn't get it to work out of the box with the current Sentry NestJS package (8.42.0). This is my current approach that works with distributed tracing:

import {
	ApolloServerPlugin,
	GraphQLRequestExecutionListener,
	GraphQLRequestListener
} from '@apollo/server';
import {GraphQLContext} from '../graphql.context';
import * as Sentry from '@sentry/core';

export const sentryTracingPlugin: () => ApolloServerPlugin<GraphQLContext> = () => {
	return {
		async requestDidStart(requestContext) {
			return Sentry.withScope(async (scope) => {
				const propagationContext = Sentry.propagationContextFromHeaders(
					requestContext.request.http?.headers.get('sentry-trace'),
					requestContext.request.http?.headers.get('baggage')
				);
				scope.setPropagationContext(propagationContext);

				const account = requestContext.contextValue.req?.user?.account;
				if (account) {
					scope.setUser({
						id: account.id,
						email: account.email
					});
				}

				return Sentry.startSpanManual(
					{
						op: 'gql',
						name: requestContext.request.operationName || 'GraphQLRequest',
						scope
					},
					async (requestSpan) => {
						return Promise.resolve<GraphQLRequestListener<GraphQLContext>>({
							willSendResponse: async () => {
								requestSpan.end();
								return Promise.resolve();
							},
							async executionDidStart(_requestContext) {
								return Promise.resolve<GraphQLRequestExecutionListener<GraphQLContext>>({
									willResolveField({info}) {
										if (!info.path.prev) {
											return Sentry.startSpanManual(
												{
													op: 'resolver',
													name: info.path.typename
														? `${info.path.typename}.${info.fieldName}`
														: info.fieldName,
													parentSpan: requestSpan
												},
												(resolverSpan) => {
													return () => {
														resolverSpan.end();
													};
												}
											);
										}
									}
								});
							}
						});
					}
				);
			});
		}
	};
};

I can imagine you don't have the same GraphQLContext as I have, so act accordingly.
Please also note it only traces the root resolver. This too is a preference of mine to prevent having too many spans being created for each resolver.

Usage in the NestJS Graphql Apollo module:

const plugins: Array<ApolloServerPlugin<GraphQLContext>> = [
	sentryTracingPlugin(),
];

Hopefully it helps someone 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants