From a79d6fbc8c4f283a4727b42c9a5ced1eee5e02c7 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 10:27:35 +0200 Subject: [PATCH 1/9] feat(node): Add `useOperationNameForRootSpan` to`graphqlIntegration` --- .../tracing/apollo-graphql/apollo-server.js | 29 ++++++ .../apollo-graphql/scenario-mutation.js | 25 +----- .../tracing/apollo-graphql/scenario-query.js | 17 +--- .../suites/tracing/apollo-graphql/test.ts | 4 +- .../scenario-invalid-root-span.js | 34 +++++++ .../scenario-mutation.js | 46 ++++++++++ .../scenario-no-operation-name.js | 41 +++++++++ .../scenario-query.js | 41 +++++++++ .../useOperationNameForRootSpan/test.ts | 88 +++++++++++++++++++ .../node/src/integrations/tracing/graphql.ts | 38 +++++++- packages/opentelemetry/src/utils/spanTypes.ts | 2 +- 11 files changed, 322 insertions(+), 43 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js new file mode 100644 index 000000000000..e32ada318d05 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js @@ -0,0 +1,29 @@ +const { ApolloServer, gql } = require('apollo-server'); + +module.exports = () => + 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/tracing/apollo-graphql/scenario-mutation.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js index 9cecf2302315..2511e6246f29 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js @@ -12,7 +12,7 @@ Sentry.init({ setInterval(() => {}, 1000); async function run() { - const { ApolloServer, gql } = require('apollo-server'); + const { gql } = require('apollo-server'); await Sentry.startSpan( { @@ -20,28 +20,7 @@ async function run() { op: 'transaction', }, async span => { - const server = new ApolloServer({ - typeDefs: gql` - type Query { - hello: String - } - type Mutation { - login(email: String): String - } - `, - resolvers: { - Query: { - hello: () => { - return 'Hello world!'; - }, - }, - Mutation: { - login: async (_, { email }) => { - return `${email}--token`; - }, - }, - }, - }); + const server = require('./apollo-server')(); // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js index f0c140fd4b24..bbaf4e5ffe7f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js @@ -12,28 +12,13 @@ Sentry.init({ setInterval(() => {}, 1000); async function run() { - const { ApolloServer, gql } = require('apollo-server'); - await Sentry.startSpan( { name: 'Test Transaction', op: 'transaction', }, async span => { - const typeDefs = gql`type Query { hello: String }`; - - const resolvers = { - Query: { - hello: () => { - return 'Hello world!'; - }, - }, - }; - - const server = new ApolloServer({ - typeDefs, - resolvers, - }); + const server = require('./apollo-server')(); // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ 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 5bf91f7653c1..bcbd19d7cb56 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 @@ -1,7 +1,7 @@ import { createRunner } from '../../../utils/runner'; describe('GraphQL/Apollo Tests', () => { - test('CJS - should instrument GraphQL queries used from Apollo Server.', done => { + test('should instrument GraphQL queries used from Apollo Server.', done => { const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', spans: expect.arrayContaining([ @@ -21,7 +21,7 @@ describe('GraphQL/Apollo Tests', () => { createRunner(__dirname, 'scenario-query.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); }); - test('CJS - should instrument GraphQL mutations used from Apollo Server.', done => { + test('should instrument GraphQL mutations used from Apollo Server.', done => { const EXPECTED_TRANSACTION = { transaction: 'Test Transaction', spans: expect.arrayContaining([ diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js new file mode 100644 index 000000000000..9bba6218702b --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js @@ -0,0 +1,34 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + await tracer.startActiveSpan('test span name', async span => { + const server = require('../apollo-server')(); + + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query GetHello {hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js new file mode 100644 index 000000000000..2e179a472212 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js @@ -0,0 +1,46 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const { gql } = require('apollo-server'); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET' }, + }, + async span => { + const server = require('../apollo-server')(); + + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: gql`mutation TestMutation($email: String){ + login(email: $email) + }`, + variables: { email: 'test@email.com' }, + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js new file mode 100644 index 000000000000..b84e7f3d7cfa --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js @@ -0,0 +1,41 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET' }, + }, + async span => { + const server = require('../apollo-server')(); + + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query {hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js new file mode 100644 index 000000000000..830de1a22bb5 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js @@ -0,0 +1,41 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET' }, + }, + async span => { + const server = require('../apollo-server')(); + + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query GetHello {hello}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); 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 new file mode 100644 index 000000000000..ca3d11d79581 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/test.ts @@ -0,0 +1,88 @@ +import { createRunner } from '../../../../utils/runner'; + +describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { + test('useOperationNameForRootSpan works with single query operation', done => { + const EXPECTED_TRANSACTION = { + transaction: 'query GetHello', + spans: expect.arrayContaining([ + 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', + }), + ]), + }; + + createRunner(__dirname, 'scenario-query.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + }); + + test('useOperationNameForRootSpan works with single mutation operation', done => { + const EXPECTED_TRANSACTION = { + transaction: 'mutation TestMutation', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.name': 'TestMutation', + 'graphql.operation.type': 'mutation', + 'graphql.source': `mutation TestMutation($email: String) { + login(email: $email) +}`, + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'mutation TestMutation', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-mutation.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + }); + + test('useOperationNameForRootSpan ignores an invalid root span', done => { + const EXPECTED_TRANSACTION = { + transaction: 'test span name', + spans: expect.arrayContaining([ + 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', + }), + ]), + }; + + createRunner(__dirname, 'scenario-invalid-root-span.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + }); + + test('useOperationNameForRootSpan works with single query operation without name', done => { + const EXPECTED_TRANSACTION = { + transaction: 'query', + spans: expect.arrayContaining([ + expect.objectContaining({ + data: { + 'graphql.operation.type': 'query', + 'graphql.source': 'query {hello}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-no-operation-name.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + }); +}); diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index 097ee3ba43f8..1f433161f82c 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -1,5 +1,7 @@ +import { SpanKind } from '@opentelemetry/api'; import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql'; -import { defineIntegration } from '@sentry/core'; +import { defineIntegration, getRootSpan, spanToJSON } from '@sentry/core'; +import { spanHasKind } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; import { generateInstrumentOnce } from '../../otel/instrument'; @@ -18,6 +20,17 @@ interface GraphqlOptions { * This option can reduce noise and number of spans created. */ ignoreTrivalResolveSpans?: boolean; + + /** + * By default, an incoming GraphQL request will have a http.server root span, + * which has one or multiple GraphQL operation spans as children. + * If you want your http.server root span to have the name of the GraphQL operation, + * you can opt-in to this behavior by setting this option to true. + * + * Please note that this may not work as expected if you have multiple GraphQL operations - + * the last operation to come in will determine the root span name in this scenario. + */ + useOperationNameForRootSpan?: boolean; } const INTEGRATION_NAME = 'Graphql'; @@ -28,6 +41,7 @@ export const instrumentGraphql = generateInstrumentOnce( const options = { ignoreResolveSpans: true, ignoreTrivialResolveSpans: true, + useOperationNameForRootSpan: false, ..._options, }; @@ -35,6 +49,28 @@ export const instrumentGraphql = generateInstrumentOnce( ...options, responseHook(span) { addOriginToSpan(span, 'auto.graphql.otel.graphql'); + + const attributes = spanToJSON(span).data || {}; + + // If operation.name is not set, we fall back to use operation.type only + const operationType = attributes['graphql.operation.type']; + const operationName = attributes['graphql.operation.name']; + + if (options.useOperationNameForRootSpan && operationType) { + const rootSpanName = `${operationType}${operationName ? ` ${operationName}` : ''}`; + const rootSpan = getRootSpan(span); + + // We guard to only do this on http.server spans + if ( + spanToJSON(rootSpan).data?.['http.method'] && + spanHasKind(rootSpan) && + rootSpan.kind === SpanKind.SERVER + ) { + // Ensure the default http.server span name inferral is skipped + rootSpan.setAttribute('sentry.skip_span_data_inference', true); + rootSpan.updateName(rootSpanName); + } + } }, }); }, diff --git a/packages/opentelemetry/src/utils/spanTypes.ts b/packages/opentelemetry/src/utils/spanTypes.ts index f92d411200a1..65777d62b31e 100644 --- a/packages/opentelemetry/src/utils/spanTypes.ts +++ b/packages/opentelemetry/src/utils/spanTypes.ts @@ -22,7 +22,7 @@ export function spanHasAttributes( */ export function spanHasKind(span: SpanType): span is SpanType & { kind: SpanKind } { const castSpan = span as ReadableSpan; - return !!castSpan.kind; + return 'kind' in castSpan; } /** From d4cb5303fc8bad76bd60688051482cf58fcb744c Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 10:38:26 +0200 Subject: [PATCH 2/9] fix typo See https://github.com/getsentry/sentry-javascript/pull/12097#issuecomment-2270623316 --- packages/node/src/integrations/tracing/graphql.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index 1f433161f82c..4f7920f7892e 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -19,7 +19,7 @@ interface GraphqlOptions { * If the property is not a function, it's not very interesting to trace. * This option can reduce noise and number of spans created. */ - ignoreTrivalResolveSpans?: boolean; + ignoreTrivialResolveSpans?: boolean; /** * By default, an incoming GraphQL request will have a http.server root span, From e6687bb9a6a568fe23e6b20bb52637c10a6ca939 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 12:52:50 +0200 Subject: [PATCH 3/9] fixes --- .../tracing/apollo-graphql/apollo-server.js | 38 ++++++----- .../apollo-graphql/scenario-mutation.js | 3 +- .../tracing/apollo-graphql/scenario-query.js | 4 +- .../suites/tracing/apollo-graphql/test.ts | 15 ++++- .../scenario-invalid-root-span.js | 4 +- .../scenario-multiple-operations.js | 45 +++++++++++++ .../scenario-mutation.js | 5 +- .../scenario-no-operation-name.js | 6 +- .../scenario-query.js | 6 +- .../useOperationNameForRootSpan/test.ts | 66 +++++++++++++++++-- .../node/src/integrations/tracing/graphql.ts | 35 +++++----- packages/opentelemetry/src/index.ts | 2 + packages/opentelemetry/src/utils/spanTypes.ts | 2 +- .../test/utils/spanTypes.test.ts | 1 + 14 files changed, 175 insertions(+), 57 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js index e32ada318d05..8c1817564196 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/apollo-server.js @@ -1,29 +1,33 @@ const { ApolloServer, gql } = require('apollo-server'); +const Sentry = require('@sentry/node'); -module.exports = () => - new ApolloServer({ - typeDefs: gql`type Query { +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!'; + resolvers: { + Query: { + hello: () => { + return 'Hello!'; + }, + world: () => { + return 'World!'; + }, }, - world: () => { - return 'World!'; + Mutation: { + login: async (_, { email }) => { + return `${email}--token`; + }, }, }, - Mutation: { - login: async (_, { email }) => { - return `${email}--token`; - }, - }, - }, - introspection: false, - debug: false, + introspection: false, + debug: false, + }); }); +}; diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js index 2511e6246f29..6defe777d464 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-mutation.js @@ -13,6 +13,7 @@ setInterval(() => {}, 1000); async function run() { const { gql } = require('apollo-server'); + const server = require('./apollo-server')(); await Sentry.startSpan( { @@ -20,8 +21,6 @@ async function run() { op: 'transaction', }, async span => { - const server = require('./apollo-server')(); - // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: gql`mutation Mutation($email: String){ diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js index bbaf4e5ffe7f..b9a05c4b1c3c 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/scenario-query.js @@ -12,14 +12,14 @@ Sentry.init({ setInterval(() => {}, 1000); async function run() { + const server = require('./apollo-server')(); + await Sentry.startSpan( { name: 'Test Transaction', op: 'transaction', }, async span => { - const server = require('./apollo-server')(); - // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: '{hello}', 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 bcbd19d7cb56..46e05acf940e 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 @@ -1,5 +1,10 @@ import { createRunner } from '../../../utils/runner'; +// Graphql Instrumentation emits some spans by default on server start +const EXPECTED_START_SERVER_TRANSACTION = { + transaction: 'Test Server Start', +}; + describe('GraphQL/Apollo Tests', () => { test('should instrument GraphQL queries used from Apollo Server.', done => { const EXPECTED_TRANSACTION = { @@ -18,7 +23,10 @@ describe('GraphQL/Apollo Tests', () => { ]), }; - createRunner(__dirname, 'scenario-query.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-query.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); test('should instrument GraphQL mutations used from Apollo Server.', done => { @@ -39,6 +47,9 @@ describe('GraphQL/Apollo Tests', () => { ]), }; - createRunner(__dirname, 'scenario-mutation.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-mutation.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js index 9bba6218702b..840a5551b98a 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-invalid-root-span.js @@ -15,9 +15,9 @@ const tracer = client.tracer; setInterval(() => {}, 1000); async function run() { - await tracer.startActiveSpan('test span name', async span => { - const server = require('../apollo-server')(); + const server = require('../apollo-server')(); + await tracer.startActiveSpan('test span name', async span => { // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: 'query GetHello {hello}', diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js new file mode 100644 index 000000000000..1375c16cd207 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js @@ -0,0 +1,45 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const server = require('../apollo-server')(); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, + }, + async span => { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: 'query GetHello {hello}', + }); + + await server.executeOperation({ + query: 'query GetWorld {world}', + }); + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js index 2e179a472212..8ee9154c0e51 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-mutation.js @@ -16,16 +16,15 @@ setInterval(() => {}, 1000); async function run() { const { gql } = require('apollo-server'); + const server = require('../apollo-server')(); await tracer.startActiveSpan( 'test span name', { kind: 1, - attributes: { 'http.method': 'GET' }, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, }, async span => { - const server = require('../apollo-server')(); - // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: gql`mutation TestMutation($email: String){ diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js index b84e7f3d7cfa..14879bc0e79d 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-no-operation-name.js @@ -15,15 +15,15 @@ const tracer = client.tracer; setInterval(() => {}, 1000); async function run() { + const server = require('../apollo-server')(); + await tracer.startActiveSpan( 'test span name', { kind: 1, - attributes: { 'http.method': 'GET' }, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, }, async span => { - const server = require('../apollo-server')(); - // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: 'query {hello}', diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js index 830de1a22bb5..4dc3357ab17f 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-query.js @@ -15,15 +15,15 @@ const tracer = client.tracer; setInterval(() => {}, 1000); async function run() { + const server = require('../apollo-server')(); + await tracer.startActiveSpan( 'test span name', { kind: 1, - attributes: { 'http.method': 'GET' }, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, }, async span => { - const server = require('../apollo-server')(); - // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ query: 'query GetHello {hello}', 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 ca3d11d79581..5c1436e9f0ea 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,14 @@ import { createRunner } from '../../../../utils/runner'; +// Graphql Instrumentation emits some spans by default on server start +const EXPECTED_START_SERVER_TRANSACTION = { + transaction: 'Test Server Start', +}; + describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { test('useOperationNameForRootSpan works with single query operation', done => { const EXPECTED_TRANSACTION = { - transaction: 'query GetHello', + transaction: 'GET /test-graphql (query GetHello)', spans: expect.arrayContaining([ expect.objectContaining({ data: { @@ -19,12 +24,15 @@ describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { ]), }; - createRunner(__dirname, 'scenario-query.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-query.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); test('useOperationNameForRootSpan works with single mutation operation', done => { const EXPECTED_TRANSACTION = { - transaction: 'mutation TestMutation', + transaction: 'GET /test-graphql (mutation TestMutation)', spans: expect.arrayContaining([ expect.objectContaining({ data: { @@ -42,7 +50,10 @@ describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { ]), }; - createRunner(__dirname, 'scenario-mutation.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-mutation.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); test('useOperationNameForRootSpan ignores an invalid root span', done => { @@ -63,12 +74,15 @@ describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { ]), }; - createRunner(__dirname, 'scenario-invalid-root-span.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-invalid-root-span.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); test('useOperationNameForRootSpan works with single query operation without name', done => { const EXPECTED_TRANSACTION = { - transaction: 'query', + transaction: 'GET /test-graphql (query)', spans: expect.arrayContaining([ expect.objectContaining({ data: { @@ -83,6 +97,44 @@ describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { ]), }; - createRunner(__dirname, 'scenario-no-operation-name.js').expect({ transaction: EXPECTED_TRANSACTION }).start(done); + createRunner(__dirname, 'scenario-no-operation-name.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); + + test('useOperationNameForRootSpan works with multiple query operations', done => { + const EXPECTED_TRANSACTION = { + transaction: 'GET /test-graphql (query GetHello)', + spans: expect.arrayContaining([ + 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', + }), + expect.objectContaining({ + data: { + 'graphql.operation.name': 'GetWorld', + 'graphql.operation.type': 'query', + 'graphql.source': 'query GetWorld {world}', + 'sentry.origin': 'auto.graphql.otel.graphql', + }, + description: 'query GetWorld', + status: 'ok', + origin: 'auto.graphql.otel.graphql', + }), + ]), + }; + + createRunner(__dirname, 'scenario-multiple-operations.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); }); }); diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index 4f7920f7892e..46d99ae3df2e 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -1,14 +1,17 @@ -import { SpanKind } from '@opentelemetry/api'; import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql'; import { defineIntegration, getRootSpan, spanToJSON } from '@sentry/core'; -import { spanHasKind } from '@sentry/opentelemetry'; +import { parseSpanDescription } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; import { generateInstrumentOnce } from '../../otel/instrument'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; interface GraphqlOptions { - /** Do not create spans for resolvers. */ + /** + * Do not create spans for resolvers. + * + * Defaults to true. + */ ignoreResolveSpans?: boolean; /** @@ -18,17 +21,16 @@ interface GraphqlOptions { * use the default resolver which just looks for a property with that name on the object. * If the property is not a function, it's not very interesting to trace. * This option can reduce noise and number of spans created. + * + * Defaults to true. */ ignoreTrivialResolveSpans?: boolean; /** - * By default, an incoming GraphQL request will have a http.server root span, - * which has one or multiple GraphQL operation spans as children. - * If you want your http.server root span to have the name of the GraphQL operation, - * you can opt-in to this behavior by setting this option to true. + * If this is enabled, a http.server root span containing this span will automatically be renamed to include the operation name. + * Set this to `false` if you do not want this behavior, and want to keep the default http.server span name. * - * Please note that this may not work as expected if you have multiple GraphQL operations - - * the last operation to come in will determine the root span name in this scenario. + * Defaults to true. */ useOperationNameForRootSpan?: boolean; } @@ -41,7 +43,7 @@ export const instrumentGraphql = generateInstrumentOnce( const options = { ignoreResolveSpans: true, ignoreTrivialResolveSpans: true, - useOperationNameForRootSpan: false, + useOperationNameForRootSpan: true, ..._options, }; @@ -57,15 +59,18 @@ export const instrumentGraphql = generateInstrumentOnce( const operationName = attributes['graphql.operation.name']; if (options.useOperationNameForRootSpan && operationType) { - const rootSpanName = `${operationType}${operationName ? ` ${operationName}` : ''}`; const rootSpan = getRootSpan(span); + const rootSpanDescription = parseSpanDescription(rootSpan); - // We guard to only do this on http.server spans + // We guard to only do this on http.server spans, and only if we have not already set the operation name if ( - spanToJSON(rootSpan).data?.['http.method'] && - spanHasKind(rootSpan) && - rootSpan.kind === SpanKind.SERVER + parseSpanDescription(rootSpan).op === 'http.server' && + !spanToJSON(rootSpan).data?.['sentry.skip_span_data_inference'] ) { + const rootSpanName = `${rootSpanDescription.description} (${operationType}${ + operationName ? ` ${operationName}` : '' + })`; + // Ensure the default http.server span name inferral is skipped rootSpan.setAttribute('sentry.skip_span_data_inference', true); rootSpan.updateName(rootSpanName); diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index ef57ab0fff3d..941f69352356 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -1,3 +1,5 @@ +export { parseSpanDescription } from './utils/parseSpanDescription'; + export { getRequestSpanData } from './utils/getRequestSpanData'; export type { OpenTelemetryClient } from './types'; diff --git a/packages/opentelemetry/src/utils/spanTypes.ts b/packages/opentelemetry/src/utils/spanTypes.ts index 65777d62b31e..39c62219d2ad 100644 --- a/packages/opentelemetry/src/utils/spanTypes.ts +++ b/packages/opentelemetry/src/utils/spanTypes.ts @@ -22,7 +22,7 @@ export function spanHasAttributes( */ export function spanHasKind(span: SpanType): span is SpanType & { kind: SpanKind } { const castSpan = span as ReadableSpan; - return 'kind' in castSpan; + return typeof castSpan.kind === 'number'; } /** diff --git a/packages/opentelemetry/test/utils/spanTypes.test.ts b/packages/opentelemetry/test/utils/spanTypes.test.ts index 99152204adfa..b0a9bc0c81da 100644 --- a/packages/opentelemetry/test/utils/spanTypes.test.ts +++ b/packages/opentelemetry/test/utils/spanTypes.test.ts @@ -24,6 +24,7 @@ describe('spanTypes', () => { it.each([ [{}, false], [{ kind: null }, false], + [{ kind: 0 }, true], [{ kind: 'TEST_KIND' }, true], ])('works with %p', (span, expected) => { const castSpan = span as unknown as Span; From 6cd6c2fe276b9409351860ab9e2220293da4fa84 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 12:54:45 +0200 Subject: [PATCH 4/9] doc fix --- packages/node/src/integrations/tracing/graphql.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index 46d99ae3df2e..c5f0e2f61aba 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -30,6 +30,8 @@ interface GraphqlOptions { * If this is enabled, a http.server root span containing this span will automatically be renamed to include the operation name. * Set this to `false` if you do not want this behavior, and want to keep the default http.server span name. * + * If there are multiple operations in a single http.server request, the first one will take precedence. + * * Defaults to true. */ useOperationNameForRootSpan?: boolean; From dbcd37ae41303d69743d073d12d0cd7d2b3716ac Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 13:08:08 +0200 Subject: [PATCH 5/9] fix test --- packages/opentelemetry/test/utils/spanTypes.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/opentelemetry/test/utils/spanTypes.test.ts b/packages/opentelemetry/test/utils/spanTypes.test.ts index b0a9bc0c81da..af07e5c45af5 100644 --- a/packages/opentelemetry/test/utils/spanTypes.test.ts +++ b/packages/opentelemetry/test/utils/spanTypes.test.ts @@ -25,7 +25,8 @@ describe('spanTypes', () => { [{}, false], [{ kind: null }, false], [{ kind: 0 }, true], - [{ kind: 'TEST_KIND' }, true], + [{ kind: 5 }, true], + [{ kind: 'TEST_KIND' }, false], ])('works with %p', (span, expected) => { const castSpan = span as unknown as Span; const actual = spanHasKind(castSpan); From 991c12bd2bd346a9d5b525e3e2c376ea0a7c0ea8 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 15:32:56 +0200 Subject: [PATCH 6/9] handle multiple operations --- .../scenario-multiple-operations-many.js | 43 +++++++++++++++++++ .../scenario-multiple-operations.js | 4 +- .../useOperationNameForRootSpan/test.ts | 14 +++++- .../node/src/integrations/tracing/graphql.ts | 34 ++++++++------- packages/opentelemetry/src/index.ts | 2 +- .../opentelemetry/src/semanticAttributes.ts | 3 ++ .../src/utils/parseSpanDescription.ts | 29 ++++++++++++- 7 files changed, 108 insertions(+), 21 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations-many.js diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations-many.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations-many.js new file mode 100644 index 000000000000..992ff5337b46 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations-many.js @@ -0,0 +1,43 @@ +const Sentry = require('@sentry/node'); +const { loggingTransport } = require('@sentry-internal/node-integration-tests'); + +const client = Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + release: '1.0', + tracesSampleRate: 1.0, + integrations: [Sentry.graphqlIntegration({ useOperationNameForRootSpan: true })], + transport: loggingTransport, +}); + +const tracer = client.tracer; + +// Stop the process from exiting before the transaction is sent +setInterval(() => {}, 1000); + +async function run() { + const server = require('../apollo-server')(); + + await tracer.startActiveSpan( + 'test span name', + { + kind: 1, + attributes: { 'http.method': 'GET', 'http.route': '/test-graphql' }, + }, + async span => { + for (let i = 1; i < 10; i++) { + // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation + await server.executeOperation({ + query: `query GetHello${i} {hello}`, + }); + } + + setTimeout(() => { + span.end(); + server.stop(); + }, 500); + }, + ); +} + +// eslint-disable-next-line @typescript-eslint/no-floating-promises +run(); diff --git a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js index 1375c16cd207..d9eeca63ae10 100644 --- a/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js +++ b/dev-packages/node-integration-tests/suites/tracing/apollo-graphql/useOperationNameForRootSpan/scenario-multiple-operations.js @@ -26,11 +26,11 @@ async function run() { async span => { // Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation await server.executeOperation({ - query: 'query GetHello {hello}', + query: 'query GetWorld {world}', }); await server.executeOperation({ - query: 'query GetWorld {world}', + query: 'query GetHello {hello}', }); setTimeout(() => { 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 5c1436e9f0ea..234cc4009b38 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 @@ -105,7 +105,7 @@ describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { test('useOperationNameForRootSpan works with multiple query operations', done => { const EXPECTED_TRANSACTION = { - transaction: 'GET /test-graphql (query GetHello)', + transaction: 'GET /test-graphql (query GetHello, query GetWorld)', spans: expect.arrayContaining([ expect.objectContaining({ data: { @@ -137,4 +137,16 @@ describe('GraphQL/Apollo Tests > useOperationNameForRootSpan', () => { .expect({ transaction: EXPECTED_TRANSACTION }) .start(done); }); + + test('useOperationNameForRootSpan works with more than 5 query operations', done => { + const EXPECTED_TRANSACTION = { + transaction: + 'GET /test-graphql (query GetHello1, query GetHello2, query GetHello3, query GetHello4, query GetHello5, +4)', + }; + + createRunner(__dirname, 'scenario-multiple-operations-many.js') + .expect({ transaction: EXPECTED_START_SERVER_TRANSACTION }) + .expect({ transaction: EXPECTED_TRANSACTION }) + .start(done); + }); }); diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index c5f0e2f61aba..cdaf0e8c836a 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -1,6 +1,6 @@ import { GraphQLInstrumentation } from '@opentelemetry/instrumentation-graphql'; import { defineIntegration, getRootSpan, spanToJSON } from '@sentry/core'; -import { parseSpanDescription } from '@sentry/opentelemetry'; +import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '@sentry/opentelemetry'; import type { IntegrationFn } from '@sentry/types'; import { generateInstrumentOnce } from '../../otel/instrument'; @@ -62,20 +62,24 @@ export const instrumentGraphql = generateInstrumentOnce( if (options.useOperationNameForRootSpan && operationType) { const rootSpan = getRootSpan(span); - const rootSpanDescription = parseSpanDescription(rootSpan); - - // We guard to only do this on http.server spans, and only if we have not already set the operation name - if ( - parseSpanDescription(rootSpan).op === 'http.server' && - !spanToJSON(rootSpan).data?.['sentry.skip_span_data_inference'] - ) { - const rootSpanName = `${rootSpanDescription.description} (${operationType}${ - operationName ? ` ${operationName}` : '' - })`; - - // Ensure the default http.server span name inferral is skipped - rootSpan.setAttribute('sentry.skip_span_data_inference', true); - rootSpan.updateName(rootSpanName); + + // We guard to only do this on http.server spans + + const rootSpanAttributes = spanToJSON(rootSpan).data || {}; + + const existingOperations = rootSpanAttributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION] || []; + + const newOperation = operationName ? `${operationType} ${operationName}` : `${operationType}`; + + // We keep track of each operation on the root span + // This can either be a string, or an array of strings (if there are multiple operations) + if (Array.isArray(existingOperations)) { + existingOperations.push(newOperation); + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, existingOperations); + } else if (existingOperations) { + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, [existingOperations, newOperation]); + } else { + rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION, newOperation); } } }, diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 941f69352356..98460b575c8d 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -1,4 +1,4 @@ -export { parseSpanDescription } from './utils/parseSpanDescription'; +export { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from './semanticAttributes'; export { getRequestSpanData } from './utils/getRequestSpanData'; diff --git a/packages/opentelemetry/src/semanticAttributes.ts b/packages/opentelemetry/src/semanticAttributes.ts index 80a80f87a666..2e14c71bf5e9 100644 --- a/packages/opentelemetry/src/semanticAttributes.ts +++ b/packages/opentelemetry/src/semanticAttributes.ts @@ -1,2 +1,5 @@ /** If this attribute is true, it means that the parent is a remote span. */ export const SEMANTIC_ATTRIBUTE_SENTRY_PARENT_IS_REMOTE = 'sentry.parentIsRemote'; + +// These are not standardized yet, but used by the graphql instrumentation +export const SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION = 'sentry.graphql.operation'; diff --git a/packages/opentelemetry/src/utils/parseSpanDescription.ts b/packages/opentelemetry/src/utils/parseSpanDescription.ts index a9d99aa91b8a..c5adae01920b 100644 --- a/packages/opentelemetry/src/utils/parseSpanDescription.ts +++ b/packages/opentelemetry/src/utils/parseSpanDescription.ts @@ -15,6 +15,7 @@ import type { SpanAttributes, TransactionSource } from '@sentry/types'; import { getSanitizedUrlString, parseUrl, stripUrlQueryAndFragment } from '@sentry/utils'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core'; +import { SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION } from '../semanticAttributes'; import type { AbstractSpan } from '../types'; import { getSpanKind } from './getSpanKind'; import { spanHasAttributes, spanHasName } from './spanTypes'; @@ -136,8 +137,16 @@ export function descriptionForHttpMethod( return { op: opParts.join('.'), description: name, source: 'custom' }; } - // Ex. description="GET /api/users". - const description = `${httpMethod} ${urlPath}`; + const graphqlOperations = attributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION]; + + // Ex. GET /api/users + const baseDescription = `${httpMethod} ${urlPath}`; + + // When the http span has a graphql operation, append it to the description + // We add these in the graphqlIntegration + const description = graphqlOperations + ? `${baseDescription} (${getGraphqlOperationNames(graphqlOperations)})` + : baseDescription; // If `httpPath` is a root path, then we can categorize the transaction source as route. const source: TransactionSource = hasRoute || urlPath === '/' ? 'route' : 'url'; @@ -162,6 +171,22 @@ export function descriptionForHttpMethod( }; } +function getGraphqlOperationNames(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}`; +} + /** Exported for tests only */ export function getSanitizedUrl( attributes: Attributes, From 0c6e89722d291e42d1afd3b6d09102d18e4eb096 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 6 Aug 2024 15:35:04 +0200 Subject: [PATCH 7/9] fix jsdoc --- packages/node/src/integrations/tracing/graphql.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/node/src/integrations/tracing/graphql.ts b/packages/node/src/integrations/tracing/graphql.ts index cdaf0e8c836a..914653ac745c 100644 --- a/packages/node/src/integrations/tracing/graphql.ts +++ b/packages/node/src/integrations/tracing/graphql.ts @@ -30,8 +30,6 @@ interface GraphqlOptions { * If this is enabled, a http.server root span containing this span will automatically be renamed to include the operation name. * Set this to `false` if you do not want this behavior, and want to keep the default http.server span name. * - * If there are multiple operations in a single http.server request, the first one will take precedence. - * * Defaults to true. */ useOperationNameForRootSpan?: boolean; From f71275ceec589c7a7f758c891aad940826c7e90a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 11:08:55 +0200 Subject: [PATCH 8/9] PR feedback --- packages/opentelemetry/src/utils/parseSpanDescription.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/opentelemetry/src/utils/parseSpanDescription.ts b/packages/opentelemetry/src/utils/parseSpanDescription.ts index c5adae01920b..a9a9845e2aa0 100644 --- a/packages/opentelemetry/src/utils/parseSpanDescription.ts +++ b/packages/opentelemetry/src/utils/parseSpanDescription.ts @@ -137,15 +137,15 @@ export function descriptionForHttpMethod( return { op: opParts.join('.'), description: name, source: 'custom' }; } - const graphqlOperations = attributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION]; + const graphqlOperationsAttribute = attributes[SEMANTIC_ATTRIBUTE_SENTRY_GRAPHQL_OPERATION]; // Ex. GET /api/users const baseDescription = `${httpMethod} ${urlPath}`; // When the http span has a graphql operation, append it to the description // We add these in the graphqlIntegration - const description = graphqlOperations - ? `${baseDescription} (${getGraphqlOperationNames(graphqlOperations)})` + const description = graphqlOperationsAttribute + ? `${baseDescription} (${getGraphqlOperationNamesFromAttribute(graphqlOperationsAttribute)})` : baseDescription; // If `httpPath` is a root path, then we can categorize the transaction source as route. @@ -171,7 +171,7 @@ export function descriptionForHttpMethod( }; } -function getGraphqlOperationNames(attr: AttributeValue): string { +function getGraphqlOperationNamesFromAttribute(attr: AttributeValue): string { if (Array.isArray(attr)) { const sorted = attr.slice().sort(); From daee8ab4cb72125f55fe40031c4321ab5cb69561 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Thu, 8 Aug 2024 11:10:30 +0200 Subject: [PATCH 9/9] pr fix --- packages/opentelemetry/src/utils/parseSpanDescription.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/opentelemetry/src/utils/parseSpanDescription.ts b/packages/opentelemetry/src/utils/parseSpanDescription.ts index a9a9845e2aa0..6d1c9936899b 100644 --- a/packages/opentelemetry/src/utils/parseSpanDescription.ts +++ b/packages/opentelemetry/src/utils/parseSpanDescription.ts @@ -176,7 +176,7 @@ function getGraphqlOperationNamesFromAttribute(attr: AttributeValue): string { const sorted = attr.slice().sort(); // Up to 5 items, we just add all of them - if (sorted.length < 5) { + if (sorted.length <= 5) { return sorted.join(', '); } else { // Else, we add the first 5 and the diff of other operations