feat(tracing): GraphQL and Apollo Integrations #3953

onurtemizkan · 2021-09-02T15:38:44Z

Summary:

This PR introduces basic tracing functionality for server-side GraphQL query resolutions.
My original intent was to implement a single generic integration for GraphQL but trying to patch resolvers inside graphql-js ended up being very painful (if not impossible) as most of the resolver-level execution logic is strictly unexposed and there seem to be no instances that store resolvers we can monkey-patch.

So I ended up implementing two integrations for both GraphQL and Apollo.

GraphQL integration has very basic functionality and simply works as the parent node for all resolvers in the execution tree when used with Apollo. Without Apollo, it's just a simple span for the whole GQL execution process. But may still be useful.
Apollo integration works on the resolver level, supports nested resolutions, and treats DB operations as children.

I think there is a lot of room for improvement for both integrations. But wanted to get opinions and reviews before investing more time in them.

Resolves: #3774

github-actions · 2021-09-02T15:52:09Z

size-limit report

Path	Size
@sentry/browser - CDN Bundle (gzipped)	22.49 KB (+0.02% 🔺)
@sentry/browser - Webpack	23.37 KB (0%)
@sentry/react - Webpack	23.4 KB (0%)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)	29.96 KB (+0.13% 🔺)

thinkocapo · 2021-09-14T18:27:33Z

Hi @onurtemizkan I worked with these developers on these blog articles, this one used a Resolver in his approach too
https://blog.sentry.io/2021/08/31/guest-post-performance-monitoring-in-graphql

and this used didEncounterErrors for error monitoring
https://blog.sentry.io/2020/07/22/handling-graphql-errors-using-sentry

onurtemizkan · 2021-09-16T18:42:06Z

Hi @thinkocapo, yeah I guess that's the sensible way to trace GraphQL transactions. Just if we could track GraphQL resolvers without relying on Apollo (as resolver is a core GraphQL concept), our plugin would be available for a wider range of projects. But still, this works, and most projects are using Apollo anyways, so we should be fine for now.

github-actions · 2021-10-14T12:01:38Z

This pull request 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 🥀

andreialecu · 2021-10-18T16:50:43Z

Hi @onurtemizkan I worked with these developers on these blog articles, this one used a Resolver in his approach too blog.sentry.io/2021/08/31/guest-post-performance-monitoring-in-graphql

and this used didEncounterErrors for error monitoring blog.sentry.io/2020/07/22/handling-graphql-errors-using-sentry

The approach in the first blog post doesn't work properly when batching is involved, or when a request contains multiple queries.

Here's a more detailed report of what I mean: https://forum.sentry.io/t/transactions-with-graphql/13702

To clarify, it completely breaks down with https://www.apollographql.com/docs/react/api/link/apollo-link-batch-http/

I'm hoping the approach in this PR works better in this regard.

Is there anything missing here?

rhcarvalho · 2021-11-23T13:33:43Z

Hi @onurtemizkan! Could you please help me summarize the current status for this PR?

Are we settled on the current approach?
Any drawbacks to be documented?
Any work pending?
Ready for a pass of code reviews?
What are the most important decisions and assumptions that were made that reviewers should be aware of?

Thanks!

onurtemizkan · 2021-11-23T16:55:40Z

Hi @rhcarvalho. At this stage, it would be wonderful to get some reviews about the approach taken.

The summary of this approach:

The original request about tracing functionality [Apollo tracing integration package? #3774] was about tracing resolvers that may or may not contain database operations.
To generalize it, since resolver is a GraphQL concept, and Apollo itself uses graphql-js internally to execute provided resolvers, I tried to monkey-patch/wrap those resolvers or their wrappers to create a single GraphQL integration. But, it was not possible because the implementation of graphql-js isn't exposed, and I couldn't find an alternative way around it.
But it exposes the whole execution process which is the parent of all parallel/serial resolvers. It is actually a quite rough information. Still, it feels worthwhile to track. (Please correct me if I'm wrong).
After all, to track resolvers, Apollo's constructSchema appeared to be the most relevant and approachable method to access them and their hierarchy (the resolvers object still contain the parent/child relationship in a tree structure, when passed as an argument to constructSchema).
These two integrations work together when used with an Apollo server project, where the GraphQL span is the container and resolvers are children (grand-times-x children). Resolvers also can be parents of db operations from any of our database tracing instrumentations.

I'll be happy to put more work on this PR to refine/test more, after getting reviews, but this implementation has the basic functionality and is working in the current state.

github-actions · 2022-04-14T07:53:17Z

size-limit report 📦

Path	Size
@sentry/browser - ES5 CDN Bundle (gzipped + minified)	19.43 KB (added)
@sentry/browser - ES5 CDN Bundle (minified)	60.2 KB (added)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)	18.24 KB (added)
@sentry/browser - ES6 CDN Bundle (minified)	53.82 KB (added)
@sentry/browser - Webpack (gzipped + minified)	19.99 KB (added)
@sentry/browser - Webpack (minified)	65.19 KB (added)
@sentry/react - Webpack (gzipped + minified)	20.02 KB (added)
@sentry/nextjs Client - Webpack (gzipped + minified)	43.95 KB (added)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)	25.56 KB (added)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)	24.11 KB (added)

AbhiPrasad · 2022-04-19T13:18:14Z

packages/tracing/src/hubextensions.ts

+    apollo() {
+      const integration = dynamicRequire(module, './integrations/apollo') as {
+        Apollo: IntegrationClass<Integration>;
+      };
+      return new integration.Apollo();
+    },
+    graphql() {
+      const integration = dynamicRequire(module, './integrations/graphql') as {
+        GraphQL: IntegrationClass<Integration>;
+      };
+      return new integration.GraphQL();
+    },


tbh I'd rather make these opt-in rather than auto-enabled.

Any possible way we can integration test this? We can do it in the PR moving forward as well.

@AbhiPrasad, updated and added the tests. 👍 Docs are coming up.

Can we make sure these are not auto-enabled?

AbhiPrasad · 2022-04-19T13:18:28Z

We'll also need docs for this!

AbhiPrasad · 2022-04-21T18:20:12Z

packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts

@@ -0,0 +1,33 @@
+import { assertSentryTransaction, getEnvelopeRequest, runServer, conditionalTest } from '../../../utils';
+
+conditionalTest({ min: 12 })(() => {


not supported by node 10?

Looks like they dropped support for Node 10.
https://github.com/graphql/graphql-js/blob/main/package.json#L30

They're using flatMap

Cool, can we leave a comment above the test linking to the package.json?

lforst · 2022-06-10T09:19:53Z

Thanks for this @onurtemizkan! Let's not forget to merge getsentry/sentry-docs#5089 when this is released.

onurtemizkan requested review from kamilogorek, AbhiPrasad and lobsterkatie September 2, 2021 15:39

onurtemizkan marked this pull request as ready for review September 3, 2021 13:02

kamilogorek marked this pull request as draft September 14, 2021 14:48

kamilogorek marked this pull request as ready for review September 14, 2021 14:49

kamilogorek changed the title ~~[WIP] feat(tracing): GraphQL and Apollo Integrations~~ feat(tracing): GraphQL and Apollo Integrations Sep 14, 2021

github-actions bot added the Status: Stale label Oct 14, 2021

onurtemizkan added Status: In Progress and removed Status: Stale labels Oct 15, 2021

kamilogorek removed their request for review March 2, 2022 20:34

onurtemizkan force-pushed the onur/graphql-tracing-integration branch from 69c89f9 to 94074fc Compare April 14, 2022 07:38

onurtemizkan changed the base branch from master to 7.x April 14, 2022 07:38

onurtemizkan force-pushed the onur/graphql-tracing-integration branch 2 times, most recently from 311dc78 to c634e2c Compare April 14, 2022 16:01

AbhiPrasad reviewed Apr 19, 2022

View reviewed changes

onurtemizkan force-pushed the onur/graphql-tracing-integration branch 4 times, most recently from beed667 to 4dcc09e Compare April 21, 2022 18:19

AbhiPrasad reviewed Apr 21, 2022

View reviewed changes

AbhiPrasad approved these changes Apr 21, 2022

View reviewed changes

Lms24 force-pushed the 7.x branch from 70e80af to 515efbd Compare April 26, 2022 15:34

lobsterkatie force-pushed the 7.x branch 2 times, most recently from 0040b0f to 299aeb3 Compare April 26, 2022 22:40

onurtemizkan force-pushed the onur/graphql-tracing-integration branch from 87b80a9 to 44e8e46 Compare April 29, 2022 07:48

onurtemizkan mentioned this pull request May 13, 2022

[JS v7] Add Prisma ORM Integration Docs. getsentry/sentry-docs#4961

Merged

onurtemizkan mentioned this pull request May 30, 2022

Add Node Apollo and GraphQL Integrations getsentry/sentry-docs#5089

Merged

onurtemizkan added 14 commits June 9, 2022 16:54

[WIP] - Add basic GraphQL tracing functionality

8e8e91c

Remove debugger.

abe4e9d

[WIP] Wrap apollo resolvers.

1a5882d

Simplify and document a bit.

b1399fa

Add basic tests.

4afb799

Remove eslint-ignores.

288f591

Backticks to quotes.

b19bab0

Move into integrations/node folder

a421332

Fix comment.

682a18c

Fix linter.

9fbb231

Refactor Apollo and GraphQL integrations.

ff645a4

Add integration test.

8d99b88

Update tests for refactored implementation.

50aa1db

Disable integration test on Node 10.

c80739f

onurtemizkan force-pushed the onur/graphql-tracing-integration branch from 44e8e46 to f520528 Compare June 9, 2022 16:04

onurtemizkan changed the base branch from 7.x to master June 9, 2022 16:05

Disable auto-instrumentation.

117a868

onurtemizkan force-pushed the onur/graphql-tracing-integration branch from f520528 to 117a868 Compare June 9, 2022 16:23

lforst merged commit cafd6cb into master Jun 10, 2022

lforst deleted the onur/graphql-tracing-integration branch June 10, 2022 09:20

vladanpaunovic removed the Status: In Progress label Jun 20, 2022

marandaneto mentioned this pull request Jun 22, 2022

Support for Apollo in Sentry Performance getsentry/sentry-react-native#2012

Open

		@@ -0,0 +1,33 @@
		import { assertSentryTransaction, getEnvelopeRequest, runServer, conditionalTest } from '../../../utils';

		conditionalTest({ min: 12 })(() => {

Uh oh!

feat(tracing): GraphQL and Apollo Integrations #3953

feat(tracing): GraphQL and Apollo Integrations #3953

Uh oh!

Conversation

onurtemizkan commented Sep 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report

Uh oh!

thinkocapo commented Sep 14, 2021

Uh oh!

onurtemizkan commented Sep 16, 2021

Uh oh!

github-actions bot commented Oct 14, 2021

Uh oh!

andreialecu commented Oct 18, 2021

Uh oh!

rhcarvalho commented Nov 23, 2021

Uh oh!

onurtemizkan commented Nov 23, 2021

Uh oh!

github-actions bot commented Apr 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

AbhiPrasad Apr 19, 2022

Choose a reason for hiding this comment

Uh oh!

onurtemizkan Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

AbhiPrasad Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

AbhiPrasad commented Apr 19, 2022

Uh oh!

AbhiPrasad Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

onurtemizkan Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

AbhiPrasad Apr 21, 2022

Choose a reason for hiding this comment

Uh oh!

lforst commented Jun 10, 2022

Uh oh!

Uh oh!

onurtemizkan commented Sep 2, 2021 •

edited

Loading

github-actions bot commented Sep 2, 2021 •

edited

Loading

github-actions bot commented Apr 14, 2022 •

edited

Loading