Skip to content

fix(serverless-aws): Overwrite root span name with GraphQL if set #16010

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

Merged
merged 3 commits into from
Apr 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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,
});
});
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const { loggingTransport } = require('@sentry-internal/node-integration-tests');
const Sentry = require('@sentry/aws-serverless');

Sentry.init({
dsn: 'https://[email protected]/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();
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { afterAll, describe, expect, test } from 'vitest';
import { cleanupChildProcesses, createRunner } from '../../../../utils/runner';

const EXPECTED_TRANSCATION = {
transaction: 'Test Transaction (query GetHello)',
spans: expect.arrayContaining([
expect.objectContaining({
description: 'query GetHello',
origin: 'auto.graphql.otel.graphql',
status: 'ok',
}),
]),
};

describe('graphqlIntegration', () => {
afterAll(() => {
cleanupChildProcesses();
});

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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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)',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this lacking a query name on purpose? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query in this test doesn't have a name:

      await server.executeOperation({
        query: '{hello}',
      });

Whereas, when it has one like this:

    await apolloServer.executeOperation({
     query: 'query GetHello {hello}',
   });

the transaction is named like this: Test Transaction (query GetHello)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh right, makes sense 👍

spans: expect.arrayContaining([
expect.objectContaining({
data: {
Expand All @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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: {
Expand Down
28 changes: 28 additions & 0 deletions packages/node/src/integrations/tracing/graphql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -71,6 +72,16 @@ export const instrumentGraphql = generateInstrumentOnce<GraphqlOptions>(
} else {
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).data['original-description']} (${getGraphqlOperationNamesFromAttribute(
existingOperations,
)})`,
);
}
},
});
Expand Down Expand Up @@ -114,3 +125,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}`;
}
Loading