From 9f087c7f737e158da732580be9d81c4627492cca Mon Sep 17 00:00:00 2001 From: Vincent Fortin Date: Tue, 3 Jun 2025 11:22:11 -0400 Subject: [PATCH 1/2] fix union types --- .../src/utils.ts | 47 +++++-- .../test/graphql.test.ts | 121 +++++++++++++++++ .../test/schema.ts | 125 ++++++++++++++---- 3 files changed, 258 insertions(+), 35 deletions(-) diff --git a/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts b/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts index 9bb98ed975..c592df1bd9 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/src/utils.ts @@ -305,11 +305,7 @@ export function wrapFields( tracer: api.Tracer, getConfig: () => GraphQLInstrumentationParsedConfig ): void { - if ( - !type || - typeof type.getFields !== 'function' || - type[OTEL_PATCHED_SYMBOL] - ) { + if (!type || type[OTEL_PATCHED_SYMBOL]) { return; } const fields = type.getFields(); @@ -328,16 +324,47 @@ export function wrapFields( } if (field.type) { - let unwrappedType: any = field.type; - - while (unwrappedType.ofType) { - unwrappedType = unwrappedType.ofType; - } + const unwrappedTypes = unwrapType(field.type); + for (const unwrappedType of unwrappedTypes) { wrapFields(unwrappedType, tracer, getConfig); + } } }); } +function unwrapType( + type: graphqlTypes.GraphQLOutputType +): readonly graphqlTypes.GraphQLObjectType[] { + // unwrap wrapping types (non-nullable and list types) + if ('ofType' in type) { + return unwrapType(type.ofType); + } + + // unwrap union types + if (isGraphQLUnionType(type)) { + return type.getTypes(); + } + + // return object types + if (isGraphQLObjectType(type)) { + return [type]; + } + + return []; +} + +function isGraphQLUnionType( + type: graphqlTypes.GraphQLType +): type is graphqlTypes.GraphQLUnionType { + return 'getTypes' in type && typeof type.getTypes === 'function'; +} + +function isGraphQLObjectType( + type: graphqlTypes.GraphQLType +): type is graphqlTypes.GraphQLObjectType { + return 'getFields' in type && typeof type.getFields === 'function'; +} + const handleResolveSpanError = ( resolveSpan: api.Span, err: any, diff --git a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts index 50ca20f561..9af85810be 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/test/graphql.test.ts @@ -86,6 +86,21 @@ const sourceFindUsingVariable = ` } `; +const sourceSearch = ` + query Search ($name: String!) { + search(name: $name) { + ... on Book { + __typename + name + } + ... on EBook { + __typename + name + } + } + } +`; + const badQuery = ` query foo bar `; @@ -244,6 +259,7 @@ describe('graphql', () => { assert.ok(times[RESOLVE].end <= times[EXECUTE].end); }); }); + describe('AND source is query with param', () => { let spans: ReadableSpan[]; @@ -338,6 +354,7 @@ describe('graphql', () => { ); }); }); + describe('AND source is query with param and variables', () => { let spans: ReadableSpan[]; @@ -442,6 +459,110 @@ describe('graphql', () => { ); }); }); + + describe('AND source is query to get a list of union type', () => { + let spans: ReadableSpan[]; + beforeEach(async () => { + create({}); + await graphql({ + schema, + source: sourceSearch, + variableValues: { name: 'first' }, + }); + spans = exporter.getFinishedSpans(); + }); + + afterEach(() => { + exporter.reset(); + graphQLInstrumentation.disable(); + spans = []; + }); + + it('should have 6 spans', () => { + assert.deepStrictEqual(spans.length, 6); + }); + + it('should instrument parse', () => { + const parseSpan = spans[0]; + assert.deepStrictEqual( + parseSpan.attributes[AttributeNames.SOURCE], + sourceSearch + ); + assert.deepStrictEqual(parseSpan.name, SpanNames.PARSE); + }); + + it('should instrument validate', () => { + const validateSpan = spans[1]; + + assert.deepStrictEqual(validateSpan.name, SpanNames.VALIDATE); + assert.deepStrictEqual( + validateSpan.parentSpanContext?.spanId, + undefined + ); + }); + + it('should instrument execute', () => { + const executeSpan = spans[5]; + + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.SOURCE], + sourceSearch + ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_TYPE], + 'query' + ); + assert.deepStrictEqual( + executeSpan.attributes[AttributeNames.OPERATION_NAME], + 'Search' + ); + assert.deepStrictEqual(executeSpan.name, 'query Search'); + assert.deepStrictEqual( + executeSpan.parentSpanContext?.spanId, + undefined + ); + }); + + it('should instrument resolvers', () => { + const [, , resolveParentSpan, span1, span2, executeSpan] = spans; + + assertResolveSpan( + resolveParentSpan, + 'search', + 'search', + '[SearchResult]', + 'search(name: $name) {\n' + + ' ... on Book {\n' + + ' __typename\n' + + ' name\n' + + ' }\n' + + ' ... on EBook {\n' + + ' __typename\n' + + ' name\n' + + ' }\n' + + ' }', + executeSpan.spanContext().spanId + ); + + const parentId = resolveParentSpan.spanContext().spanId; + assertResolveSpan( + span1, + 'name', + 'search.0.name', + 'String', + 'name', + parentId + ); + assertResolveSpan( + span2, + 'name', + 'search.1.name', + 'String', + 'name', + parentId + ); + }); + }); }); describe('when depth is set to 0', () => { diff --git a/plugins/node/opentelemetry-instrumentation-graphql/test/schema.ts b/plugins/node/opentelemetry-instrumentation-graphql/test/schema.ts index 51ec9420af..d21c965d16 100644 --- a/plugins/node/opentelemetry-instrumentation-graphql/test/schema.ts +++ b/plugins/node/opentelemetry-instrumentation-graphql/test/schema.ts @@ -38,32 +38,41 @@ function getData(url: string): any { }); } -const authors: Author[] = []; -const books: Book[] = []; - interface Book { + __typename: 'Book'; + id: number; + name: string; + authorIds: number[]; +} + +interface EBook { + __typename: 'EBook'; id: number; name: string; authorIds: number[]; } interface Address { + __typename: 'Address'; country: string; city: string; } interface Author { + __typename: 'Author'; id: number; name: string; address: Address; } -function addBook(name: string, authorIds: string | number[] = []) { - if (typeof authorIds === 'string') { - authorIds = authorIds.split(',').map(id => parseInt(id, 10)); - } +const books: Book[] = []; +const ebooks: EBook[] = []; +const authors: Author[] = []; + +function addBook(name: string, authorIds: number[] = []) { const id = books.length; books.push({ + __typename: 'Book', id: id, name: name, authorIds: authorIds, @@ -71,9 +80,25 @@ function addBook(name: string, authorIds: string | number[] = []) { return books[books.length - 1]; } +function addEBook(name: string, authorIds: number[] = []) { + const id = books.length; + ebooks.push({ + __typename: 'EBook', + id: id, + name: name, + authorIds: authorIds, + }); + return ebooks[ebooks.length - 1]; +} + function addAuthor(name: string, country: string, city: string) { const id = authors.length; - authors.push({ id, name, address: { country, city } }); + authors.push({ + __typename: 'Author', + id, + name, + address: { __typename: 'Address', country, city }, + }); return authors[authors.length - 1]; } @@ -93,11 +118,30 @@ function prepareData() { addBook('First Book', [0, 1]); addBook('Second Book', [2]); addBook('Third Book', [3]); + addEBook('First EBook', [1, 3]); } prepareData(); export function buildTestSchema() { + const Address = new graphql.GraphQLObjectType({ + name: 'Address', + fields: { + country: { + type: graphql.GraphQLString, + resolve(obj, args) { + return obj.country; + }, + }, + city: { + type: graphql.GraphQLString, + resolve(obj, args) { + return obj.city; + }, + }, + }, + }); + const Author = new graphql.GraphQLObjectType({ name: 'Author', fields: { @@ -124,23 +168,7 @@ export function buildTestSchema() { }, }, address: { - type: new graphql.GraphQLObjectType({ - name: 'Address', - fields: { - country: { - type: graphql.GraphQLString, - resolve(obj, args) { - return obj.country; - }, - }, - city: { - type: graphql.GraphQLString, - resolve(obj, args) { - return obj.city; - }, - }, - }, - }), + type: Address, resolve(obj, args) { return obj.address; }, @@ -174,6 +202,40 @@ export function buildTestSchema() { }, }); + // DO NOT RE-USE THIS TYPE DIRECTLY + // To truly test union type support, we need a type with sub-resolvers that is only found under a union type. + // This type is currently used only under the 'SearchResult' union type. + const EBook = new graphql.GraphQLObjectType({ + name: 'EBook', + fields: { + id: { + type: graphql.GraphQLInt, + resolve(obj, args) { + return obj.id; + }, + }, + name: { + type: graphql.GraphQLString, + resolve(obj, args) { + return obj.name; + }, + }, + authors: { + type: new graphql.GraphQLList(Author), + resolve(obj, args) { + return obj.authorIds.map((id: number) => { + return authors[id]; + }); + }, + }, + }, + }); + + const searchResult = new graphql.GraphQLUnionType({ + name: 'SearchResult', + types: [Book, EBook], + }); + const query = new graphql.GraphQLObjectType({ name: 'Query', fields: { @@ -207,6 +269,19 @@ export function buildTestSchema() { return Promise.resolve(books); }, }, + search: { + type: new graphql.GraphQLList(searchResult), + args: { + name: { type: new graphql.GraphQLNonNull(graphql.GraphQLString) }, + }, + resolve(obj, args, context) { + const searchName = args.name.toLowerCase(); + const results = [...books, ...ebooks].filter(item => + item.name.toLowerCase().includes(searchName) + ); + return Promise.resolve(results); + }, + }, }, }); From 8a08afd515cccf762df35630f59728359b4c694a Mon Sep 17 00:00:00 2001 From: Vincent Fortin Date: Thu, 10 Jul 2025 10:03:47 -0400 Subject: [PATCH 2/2] lint --- packages/instrumentation-graphql/src/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/instrumentation-graphql/src/utils.ts b/packages/instrumentation-graphql/src/utils.ts index c592df1bd9..fbea7248d6 100644 --- a/packages/instrumentation-graphql/src/utils.ts +++ b/packages/instrumentation-graphql/src/utils.ts @@ -326,7 +326,7 @@ export function wrapFields( if (field.type) { const unwrappedTypes = unwrapType(field.type); for (const unwrappedType of unwrappedTypes) { - wrapFields(unwrappedType, tracer, getConfig); + wrapFields(unwrappedType, tracer, getConfig); } } });