From 64b9e927410c63c325fe2b557880f0e10b26138f Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 8 Apr 2025 13:01:12 +0200 Subject: [PATCH 1/3] fix(serverless-aws): Overwrite root span name with GraphQL if set --- .../apollo-server.js | 35 ++++++++++++++++++ .../useOperationNameForRootSpan/scenario.js | 27 ++++++++++++++ .../useOperationNameForRootSpan/test.ts | 37 +++++++++++++++++++ .../node/src/integrations/tracing/graphql.ts | 23 ++++++++++++ 4 files changed, 122 insertions(+) create mode 100644 dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/apollo-server.js create mode 100644 dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/scenario.js create mode 100644 dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts diff --git a/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/apollo-server.js b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/apollo-server.js new file mode 100644 index 000000000000..6561adaf67ce --- /dev/null +++ b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/apollo-server.js @@ -0,0 +1,35 @@ +const { ApolloServer, gql } = require('apollo-server'); +const Sentry = require('@sentry/aws-serverless'); + +module.exports = () => { + return Sentry.startSpan({ name: 'Test Server Start' }, () => { + return new ApolloServer({ + typeDefs: gql` + type Query { + hello: String + world: String + } + type Mutation { + login(email: String): String + } + `, + resolvers: { + Query: { + hello: () => { + return 'Hello!'; + }, + world: () => { + return 'World!'; + }, + }, + Mutation: { + login: async (_, { email }) => { + return `${email}--token`; + }, + }, + }, + introspection: false, + debug: false, + }); + }); +}; diff --git a/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/scenario.js b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/scenario.js new file mode 100644 index 000000000000..4023421921b5 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/scenario.js @@ -0,0 +1,27 @@ +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); +const Sentry = require('@sentry/aws-serverless'); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +async function run() { + const apolloServer = require('./apollo-server')(); + + await Sentry.startSpan({ name: 'Test Transaction' }, async span => { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await apolloServer.executeOperation({ + query: 'query GetHello {hello}', + }); + + setTimeout(() => { + span.end(); + apolloServer.stop(); + }, 500); + }); +} + +run(); diff --git a/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts new file mode 100644 index 000000000000..c994d44e6c5b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts @@ -0,0 +1,37 @@ +import { afterAll, describe, expect, test } from 'vitest'; +import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; + +const EXPECTED_TRANSCATION = { + transaction: 'Test Server Start (query GetHello)', + spans: expect.arrayContaining([ + expect.objectContaining({ + description: 'query GetHello', + origin: 'auto.graphql.otel.graphql', + data: expect.objectContaining({ + data: { + 'graphql.operation.name': 'GetHello', + 'graphql.operation.type': 'query', + 'graphql.source': 'query GetHello {hello}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query GetHello', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + }), + ]), +}; + +describe('graphqlIntegration', () => { + afterAll(() => { + cleanupChildProcesses(); + }); + + test('should use GraphQL operation name for root span if option is set', async () => { + await createRunner(__dirname, 'scenario.js') + .ignore('event') + .expect({ transaction: EXPECTED_TRANSCATION }) + .start() + .completed(); + }); +}); diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index 945327064df2..7c91692a8854 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -5,6 +5,7 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '@sentry/opentelemet import { generateInstrumentOnce } from '../../otel/instrument'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; +import type { AttributeValue } from '@opentelemetry/api'; interface GraphqlOptions { /** @@ -71,6 +72,11 @@ export const instrumentGraphql = generateInstrumentOnce( } else { rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, newOperation); } + + // Important for e.g. @sentry/aws-serverless because this would otherwise overwrite the name again + rootSpan.updateName( + `${spanToJSON(rootSpan).description} (${getGraphqlOperationNamesFromAttribute(existingOperations)})`, + ); } }, }); @@ -114,3 +120,20 @@ function getOptionsWithDefaults(options?: GraphqlOptions): GraphqlOptions { ...options, }; } + +// copy from packages/opentelemetry/utils +function getGraphqlOperationNamesFromAttribute(attr: AttributeValue): string { + if (Array.isArray(attr)) { + const sorted = attr.slice().sort(); + + // Up to 5 items, we just add all of them + if (sorted.length <= 5) { + return sorted.join(', '); + } else { + // Else, we add the first 5 and the diff of other operations + return `${sorted.slice(0, 5).join(', ')}, +${sorted.length - 5}`; + } + } + + return `${attr}`; +} From 691eeceed7d44b18591448f043ee995a36dbceda Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Tue, 8 Apr 2025 17:32:25 +0200 Subject: [PATCH 2/3] fix updating name --- .../graphql/useOperationNameForRootSpan/test.ts | 17 ++++------------- .../useOperationNameForRootSpan/test.ts | 6 +++--- .../node/src/integrations/tracing/graphql.ts | 7 ++++++- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts index c994d44e6c5b..84098edb46ae 100644 --- a/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts +++ b/dev-packages/node-integration-tests/suites/aws-serverless/graphql/useOperationNameForRootSpan/test.ts @@ -2,22 +2,12 @@ import { afterAll, describe, expect, test } from 'vitest'; import { cleanupChildProcesses, createRunner } from '../../../../utils/runner'; const EXPECTED_TRANSCATION = { - transaction: 'Test Server Start (query GetHello)', + transaction: 'Test Transaction (query GetHello)', spans: expect.arrayContaining([ expect.objectContaining({ description: 'query GetHello', origin: 'auto.graphql.otel.graphql', - data: expect.objectContaining({ - data: { - 'graphql.operation.name': 'GetHello', - 'graphql.operation.type': 'query', - 'graphql.source': 'query GetHello {hello}', - 'sentry.origin': 'auto.graphql.otel.graphql', - }, - description: 'query GetHello', - status: 'ok', - origin: 'auto.graphql.otel.graphql', - }), + status: 'ok', }), ]), }; @@ -27,9 +17,10 @@ describe('graphqlIntegration', () => { cleanupChildProcesses(); }); - test('should use GraphQL operation name for root span if option is set', async () => { + test('should use GraphQL operation name for root span if useOperationNameForRootSpan is set', async () => { await createRunner(__dirname, 'scenario.js') .ignore('event') + .expect({ transaction: { transaction: 'Test Server Start (query IntrospectionQuery)' } }) .expect({ transaction: EXPECTED_TRANSCATION }) .start() .completed(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts index 4aa7616cc73c..b77dcd34777b 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts @@ -1,9 +1,9 @@ import { createRunner } from '../../../../utils/runner'; -import { describe, test, expect } from 'vitest' +import { describe, test, expect } from 'vitest'; // Graphql Instrumentation emits some spans by default on server start const EXPECTED_START_SERVER_TRANSACTION = { - transaction: 'Test Server Start', + transaction: 'Test Server Start (query IntrospectionQuery)', }; describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { @@ -61,7 +61,7 @@ describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { test('useOperationNameForRootSpan ignores an invalid root span', async () => { const EXPECTED_TRANSACTION = { - transaction: 'test span name', + transaction: 'test span name (query GetHello)', spans: expect.arrayContaining([ expect.objectContaining({ data: { diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index 7c91692a8854..c2316eb4ac8f 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -73,9 +73,14 @@ export const instrumentGraphql = generateInstrumentOnce( rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, newOperation); } + if (!spanToJSON(rootSpan).data['original-description']) { + rootSpan.setAttribute('original-description', spanToJSON(rootSpan).description); + } // Important for e.g. @sentry/aws-serverless because this would otherwise overwrite the name again rootSpan.updateName( - `${spanToJSON(rootSpan).description} (${getGraphqlOperationNamesFromAttribute(existingOperations)})`, + `${spanToJSON(rootSpan).data['original-description']} (${getGraphqlOperationNamesFromAttribute( + existingOperations, + )})`, ); } }, From 1ca4a4faf7d3dd6c934cd28f46bdd038d6d6984c Mon Sep 17 00:00:00 2001 From: s1gr1d Date: Wed, 9 Apr 2025 12:09:26 +0200 Subject: [PATCH 3/3] fix tests --- .../suites/tracing/apollo-graphql/test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts index c9289efbde8e..2abe2932ece2 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/test.ts @@ -3,13 +3,13 @@ import { createRunner } from '../../../utils/runner'; // Graphql Instrumentation emits some spans by default on server start const EXPECTED_START_SERVER_TRANSACTION = { - transaction: 'Test Server Start', + transaction: 'Test Server Start (query IntrospectionQuery)', }; describe('GraphQL/Apollo Tests', () => { test('should instrument GraphQL queries used from Apollo Server.', async () => { const EXPECTED_TRANSACTION = { - transaction: 'Test Transaction', + transaction: 'Test Transaction (query)', spans: expect.arrayContaining([ expect.objectContaining({ data: { @@ -33,7 +33,7 @@ describe('GraphQL/Apollo Tests', () => { test('should instrument GraphQL mutations used from Apollo Server.', async () => { const EXPECTED_TRANSACTION = { - transaction: 'Test Transaction', + transaction: 'Test Transaction (mutation Mutation)', spans: expect.arrayContaining([ expect.objectContaining({ data: {