Skip to content

Commit 49346b2

Browse files
cecchiEmrysMyrddin
andauthored
feat: add option to ignore default resolvers in opentelemetry (#2747)
* feat[useOnResolve]: add skipDefaultResolvers option * feat[opentelemetry]: add defaultResolvers option * fix[useOnResolve]: ensure resolver is not the default resolver * add changeset * update changeset * use the resolver name instead of the default resovler value for better compatibility * also ignore stitching default resolver --------- Co-authored-by: Valentin Cocaud <[email protected]>
1 parent 8212c35 commit 49346b2

File tree

5 files changed

+180
-34
lines changed

5 files changed

+180
-34
lines changed

.changeset/frank-pigs-teach.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
---
2+
'@envelop/on-resolve': minor
3+
'@envelop/opentelemetry': minor
4+
---
5+
6+
Add option to ignore default resolvers in opentelemetry instrumentation
7+
8+
To reduce telemetry data volume and noise in traces, it is recommended to ignore resolvers with the
9+
default implementation since they probably doesn't do anything worth tracking.
10+
11+
## Usage
12+
13+
```ts
14+
import { execute, parse, specifiedRules, subscribe, validate } from 'graphql'
15+
import { envelop, useEngine } from '@envelop/core'
16+
import { useOpenTelemetry } from '@envelop/opentelemetry'
17+
18+
const getEnveloped = envelop({
19+
plugins: [
20+
useEngine({ parse, validate, specifiedRules, execute, subscribe }),
21+
// ... other plugins ...
22+
23+
useOpenTelemetry({
24+
resolvers: true,
25+
defaultResolvers: false // explicitly disable default resolvers tracing. Defaults to `true`
26+
})
27+
]
28+
})
29+
```

packages/plugins/on-resolve/src/index.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ export type UseOnResolveOptions = {
4040
* @default true
4141
*/
4242
skipIntrospection?: boolean;
43+
44+
/**
45+
* Skip executing the `onResolve` hook on fields with default resolvers.
46+
*
47+
* @default false
48+
*/
49+
skipDefaultResolvers?: boolean;
4350
};
4451

4552
/**
@@ -49,7 +56,7 @@ export type UseOnResolveOptions = {
4956
*/
5057
export function useOnResolve<PluginContext extends Record<string, any> = {}>(
5158
onResolve: OnResolve<PluginContext>,
52-
opts: UseOnResolveOptions = { skipIntrospection: true },
59+
opts: UseOnResolveOptions = { skipIntrospection: true, skipDefaultResolvers: false },
5360
): Plugin<PluginContext> {
5461
const hasWrappedResolveSymbol = Symbol('hasWrappedResolve');
5562
return {
@@ -61,6 +68,14 @@ export function useOnResolve<PluginContext extends Record<string, any> = {}>(
6168
if ((!opts.skipIntrospection || !isIntrospectionType(type)) && isObjectType(type)) {
6269
for (const field of Object.values(type.getFields())) {
6370
if ((field as { [hasWrappedResolveSymbol]?: true })[hasWrappedResolveSymbol]) continue;
71+
if (
72+
opts.skipDefaultResolvers &&
73+
(!field.resolve ||
74+
field.resolve?.name === 'defaultFieldResolver' ||
75+
field.resolve?.name === 'defaultMergedResolver')
76+
) {
77+
continue;
78+
}
6479

6580
let resolver = (field.resolve || defaultFieldResolver) as Resolver<PluginContext>;
6681

packages/plugins/on-resolve/test/use-on-resolve.spec.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,20 @@ describe('useOnResolve', () => {
88
type Query {
99
value1: String!
1010
value2: String!
11+
obj: Obj!
12+
}
13+
14+
type Obj {
15+
field1: String!
1116
}
1217
`,
1318
resolvers: {
1419
Query: {
1520
value1: () => 'value1',
1621
value2: () => 'value2',
22+
obj: () => ({
23+
field1: 'field1',
24+
}),
1725
},
1826
},
1927
});
@@ -85,6 +93,47 @@ describe('useOnResolve', () => {
8593
expect(onResolveDoneFn).toHaveBeenCalledTimes(0);
8694
});
8795

96+
it('should invoke the callback for default resolvers when not skipping', async () => {
97+
const onResolveDoneFn = jest.fn();
98+
const onResolveFn = jest.fn((_opts: OnResolveOptions) => onResolveDoneFn);
99+
const testkit = createTestkit(
100+
[useOnResolve(onResolveFn, { skipDefaultResolvers: false })],
101+
schema,
102+
);
103+
104+
await testkit.execute('{ obj { field1 } }');
105+
106+
expect(onResolveFn).toHaveBeenCalledTimes(2);
107+
expect(onResolveDoneFn).toHaveBeenCalledTimes(2);
108+
109+
let i = 0;
110+
for (const field of ['obj', 'field1']) {
111+
expect(onResolveFn.mock.calls[i][0].context).toBeDefined();
112+
expect(onResolveFn.mock.calls[i][0].root).toBeDefined();
113+
expect(onResolveFn.mock.calls[i][0].args).toBeDefined();
114+
expect(onResolveFn.mock.calls[i][0].info).toBeDefined();
115+
expect(onResolveFn.mock.calls[i][0].info.fieldName).toBe(field);
116+
expect(onResolveFn.mock.calls[i][0].resolver).toBeInstanceOf(Function);
117+
expect(onResolveFn.mock.calls[i][0].replaceResolver).toBeInstanceOf(Function);
118+
119+
i++;
120+
}
121+
});
122+
123+
it('should not invoke the callback for default resolvers when skipping', async () => {
124+
const onResolveDoneFn = jest.fn();
125+
const onResolveFn = jest.fn((_opts: OnResolveOptions) => onResolveDoneFn);
126+
const testkit = createTestkit(
127+
[useOnResolve(onResolveFn, { skipDefaultResolvers: true })],
128+
schema,
129+
);
130+
131+
await testkit.execute('{ obj { field1 } }');
132+
133+
expect(onResolveFn).toHaveBeenCalledTimes(1);
134+
expect(onResolveDoneFn).toHaveBeenCalledTimes(1);
135+
});
136+
88137
it('should replace the result using the after hook', async () => {
89138
const testkit = createTestkit(
90139
[

packages/plugins/opentelemetry/src/index.ts

Lines changed: 35 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ export type ExcludeOperationNamesFn = (operationName: string | undefined) => boo
3838
export type TracingOptions = {
3939
document?: boolean;
4040
resolvers?: boolean;
41+
defaultResolvers?: boolean;
4142
variables?: boolean | ResolveVariablesAttributesFn;
4243
result?: boolean;
4344
traceIdInResult?: string;
@@ -89,39 +90,42 @@ export const useOpenTelemetry = (
8990
onPluginInit({ addPlugin }) {
9091
if (options.resolvers) {
9192
addPlugin(
92-
useOnResolve(({ info, context, args }) => {
93-
const parentSpan = spanByContext.get(context);
94-
if (parentSpan) {
95-
const ctx = opentelemetry.trace.setSpan(getCurrentOtelContext(context), parentSpan);
96-
const { fieldName, returnType, parentType } = info;
97-
98-
const resolverSpan = tracer.startSpan(
99-
`${spanPrefix}${parentType.name}.${fieldName}`,
100-
{
101-
attributes: {
102-
[AttributeName.RESOLVER_FIELD_NAME]: fieldName,
103-
[AttributeName.RESOLVER_TYPE_NAME]: parentType.toString(),
104-
[AttributeName.RESOLVER_RESULT_TYPE]: returnType.toString(),
105-
[AttributeName.RESOLVER_ARGS]: JSON.stringify(args || {}),
93+
useOnResolve(
94+
({ info, context, args }) => {
95+
const parentSpan = spanByContext.get(context);
96+
if (parentSpan) {
97+
const ctx = opentelemetry.trace.setSpan(getCurrentOtelContext(context), parentSpan);
98+
const { fieldName, returnType, parentType } = info;
99+
100+
const resolverSpan = tracer.startSpan(
101+
`${spanPrefix}${parentType.name}.${fieldName}`,
102+
{
103+
attributes: {
104+
[AttributeName.RESOLVER_FIELD_NAME]: fieldName,
105+
[AttributeName.RESOLVER_TYPE_NAME]: parentType.toString(),
106+
[AttributeName.RESOLVER_RESULT_TYPE]: returnType.toString(),
107+
[AttributeName.RESOLVER_ARGS]: JSON.stringify(args || {}),
108+
},
106109
},
107-
},
108-
ctx,
109-
);
110-
111-
return ({ result }) => {
112-
if (result instanceof Error) {
113-
resolverSpan.recordException({
114-
name: AttributeName.RESOLVER_EXCEPTION,
115-
message: JSON.stringify(result),
116-
});
117-
} else {
118-
resolverSpan.end();
119-
}
120-
};
121-
}
110+
ctx,
111+
);
112+
113+
return ({ result }) => {
114+
if (result instanceof Error) {
115+
resolverSpan.recordException({
116+
name: AttributeName.RESOLVER_EXCEPTION,
117+
message: JSON.stringify(result),
118+
});
119+
} else {
120+
resolverSpan.end();
121+
}
122+
};
123+
}
122124

123-
return () => {};
124-
}),
125+
return () => {};
126+
},
127+
{ skipDefaultResolvers: options.defaultResolvers === false },
128+
),
125129
);
126130
}
127131
},

packages/plugins/opentelemetry/test/use-open-telemetry.spec.ts

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,11 @@ describe('useOpenTelemetry', () => {
3131
echo(message: String): String
3232
error: String
3333
context: String
34+
obj: Obj
35+
}
36+
37+
type Obj {
38+
field1: String!
3439
}
3540
3641
type Subscription {
@@ -48,6 +53,9 @@ describe('useOpenTelemetry', () => {
4853
error: () => {
4954
throw new GraphQLError('boom');
5055
},
56+
obj: () => ({
57+
field1: 'field1',
58+
}),
5159
},
5260
Subscription: {
5361
counter: {
@@ -130,6 +138,49 @@ describe('useOpenTelemetry', () => {
130138
expect(actual[1].name).toBe('query.anonymous');
131139
});
132140

141+
it('Should add default resolver spans if enabled / unspecified', async () => {
142+
const exporter = new InMemorySpanExporter();
143+
const testInstance = createTestkit(
144+
[useTestOpenTelemetry(exporter, { resolvers: true })],
145+
schema,
146+
);
147+
148+
await testInstance.execute(/* GraphQL */ `
149+
query {
150+
obj {
151+
field1
152+
}
153+
}
154+
`);
155+
156+
const actual = exporter.getFinishedSpans();
157+
expect(actual.length).toBe(3);
158+
expect(actual[0].name).toBe('Query.obj');
159+
expect(actual[1].name).toBe('Obj.field1');
160+
expect(actual[2].name).toBe('query.anonymous');
161+
});
162+
163+
it('Should not add default resolver spans if disabled', async () => {
164+
const exporter = new InMemorySpanExporter();
165+
const testInstance = createTestkit(
166+
[useTestOpenTelemetry(exporter, { resolvers: true, defaultResolvers: false })],
167+
schema,
168+
);
169+
170+
await testInstance.execute(/* GraphQL */ `
171+
query {
172+
obj {
173+
field1
174+
}
175+
}
176+
`);
177+
178+
const actual = exporter.getFinishedSpans();
179+
expect(actual.length).toBe(2);
180+
expect(actual[0].name).toBe('Query.obj');
181+
expect(actual[1].name).toBe('query.anonymous');
182+
});
183+
133184
it('query should add trace_id to extensions', async () => {
134185
const exporter = new InMemorySpanExporter();
135186
const testInstance = createTestkit(
@@ -299,8 +350,6 @@ describe('useOpenTelemetry', () => {
299350
selector: '1',
300351
});
301352

302-
console.log(resp);
303-
304353
assertSingleValue(resp);
305354

306355
const actual = exporter.getFinishedSpans();

0 commit comments

Comments
 (0)