Skip to content

Commit 6daace9

Browse files
committed
Remove the otelExchange from the default exchanges in favour of injecting serverside
The otel api and associated tracing functions add some bundle size to the API client that aren't at all necessary on the browser. The browser can't export spans to our tracing config, so we really don't need this code there. But, we definitely want it server side in the sandbox where it gives us important visibility on how the api client is performing there. Instead of paying the bundle size penalty all the time, we can inject the otel exchange with the new exchange injection facility, and use normal otel monkeypatching to add the other spans. Before: ``` vite v4.4.7 building for production... ✓ 223 modules transformed. dist/api-read-stats.html 227.36 kB │ gzip: 48.87 kB dist/test-bundle-api-read.js 253.91 kB │ gzip: 47.33 kB dist/api-read-stats.html 227.90 kB │ gzip: 48.94 kB dist/test-bundle-api-read.cjs 168.40 kB │ gzip: 39.39 kB ✓ built in 1.01s vite v4.4.7 building for production... ✓ 499 modules transformed. dist/react-read-stats.html 320.54 kB │ gzip: 59.02 kB dist/test-bundle-react-read.js 359.09 kB │ gzip: 76.48 kB dist/react-read-stats.html 323.17 kB │ gzip: 59.05 kB dist/test-bundle-react-read.cjs 243.18 kB │ gzip: 64.60 kB ✓ built in 1.34s vite v4.4.7 building for production... ✓ 1033 modules transformed. dist/shopify-read-stats.html 532.78 kB │ gzip: 79.62 kB dist/test-bundle-shopify-read.js 678.46 kB │ gzip: 135.16 kB dist/shopify-read-stats.html 533.31 kB │ gzip: 79.69 kB dist/test-bundle-shopify-read.cjs 477.27 kB │ gzip: 115.70 kB ``` After: ``` ✓ 176 modules transformed. dist/api-read-stats.html 212.12 kB │ gzip: 47.04 kB dist/test-bundle-api-read.js 236.85 kB │ gzip: 43.50 kB dist/api-read-stats.html 212.64 kB │ gzip: 47.09 kB dist/test-bundle-api-read.cjs 155.96 kB │ gzip: 36.01 kB ✓ built in 861ms vite v4.4.7 building for production... ✓ 452 modules transformed. dist/react-read-stats.html 303.58 kB │ gzip: 57.00 kB dist/test-bundle-react-read.js 342.04 kB │ gzip: 72.64 kB dist/react-read-stats.html 307.55 kB │ gzip: 57.01 kB dist/test-bundle-react-read.cjs 230.74 kB │ gzip: 61.17 kB ✓ built in 1.23s vite v4.4.7 building for production... ✓ 986 modules transformed. dist/shopify-read-stats.html 517.13 kB │ gzip: 77.52 kB dist/test-bundle-shopify-read.js 661.41 kB │ gzip: 131.38 kB dist/shopify-read-stats.html 517.64 kB │ gzip: 77.55 kB dist/test-bundle-shopify-read.cjs 464.83 kB │ gzip: 112.29 kB ✓ built in 2.39s ``` 7% decompressed bundle size savings which seems ok but not great
1 parent efa1a4d commit 6daace9

File tree

5 files changed

+81
-225
lines changed

5 files changed

+81
-225
lines changed

packages/api-client-core/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
"prerelease": "gitpkg publish"
2929
},
3030
"dependencies": {
31-
"@opentelemetry/api": "^1.4.0",
3231
"@urql/core": "^4.0.10",
3332
"cross-fetch": "^3.1.5",
3433
"tiny-graphql-query-compiler": "^0.2.2",

packages/api-client-core/src/GadgetConnection.ts

Lines changed: 81 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,8 @@ import { GadgetTransaction, TransactionRolledBack } from "./GadgetTransaction.js
1111
import type { BrowserStorage } from "./InMemoryStorage.js";
1212
import { InMemoryStorage } from "./InMemoryStorage.js";
1313
import { operationNameExchange } from "./exchanges/operationNameExchange.js";
14-
import { otelExchange } from "./exchanges/otelExchange.js";
1514
import { urlParamExchange } from "./exchanges/urlParamExchange.js";
16-
import {
17-
GadgetUnexpectedCloseError,
18-
GadgetWebsocketConnectionTimeoutError,
19-
getCurrentSpan,
20-
isCloseEvent,
21-
storageAvailable,
22-
traceFunction,
23-
} from "./support.js";
15+
import { GadgetUnexpectedCloseError, GadgetWebsocketConnectionTimeoutError, isCloseEvent, storageAvailable } from "./support.js";
2416

2517
export type TransactionRun<T> = (transaction: GadgetTransaction) => Promise<T>;
2618
export interface GadgetSubscriptionClientOptions extends Partial<SubscriptionClientOptions> {
@@ -182,96 +174,90 @@ export class GadgetConnection {
182174
transaction: {
183175
<T>(options: GadgetSubscriptionClientOptions, run: TransactionRun<T>): Promise<T>;
184176
<T>(run: TransactionRun<T>): Promise<T>;
185-
} = traceFunction(
186-
"api-client.transaction",
187-
async <T>(optionsOrRun: GadgetSubscriptionClientOptions | TransactionRun<T>, maybeRun?: TransactionRun<T>): Promise<T> => {
188-
let run: TransactionRun<T>;
189-
let options: GadgetSubscriptionClientOptions;
190-
191-
if (maybeRun) {
192-
run = maybeRun;
193-
options = optionsOrRun as GadgetSubscriptionClientOptions;
194-
} else {
195-
run = optionsOrRun as TransactionRun<T>;
196-
options = {};
197-
}
177+
} = async <T>(optionsOrRun: GadgetSubscriptionClientOptions | TransactionRun<T>, maybeRun?: TransactionRun<T>): Promise<T> => {
178+
let run: TransactionRun<T>;
179+
let options: GadgetSubscriptionClientOptions;
198180

199-
if (this.currentTransaction) {
200-
return await run(this.currentTransaction);
201-
}
181+
if (maybeRun) {
182+
run = maybeRun;
183+
options = optionsOrRun as GadgetSubscriptionClientOptions;
184+
} else {
185+
run = optionsOrRun as TransactionRun<T>;
186+
options = {};
187+
}
202188

203-
getCurrentSpan()?.setAttributes({ applicationId: this.options.applicationId, environmentName: this.environment });
189+
if (this.currentTransaction) {
190+
return await run(this.currentTransaction);
191+
}
204192

205-
let subscriptionClient: SubscriptionClient | null = null;
206-
let transaction;
193+
let subscriptionClient: SubscriptionClient | null = null;
194+
let transaction;
195+
try {
196+
// The server will error if it receives any operations before the auth dance has been completed, so we block on that happening before sending our first operation. It's important that this happens synchronously after instantiating the client so we don't miss any messages
197+
subscriptionClient = await this.waitForOpenedConnection({
198+
isFatalConnectionProblem(errorOrCloseEvent) {
199+
// any interruption of the transaction is fatal to the transaction
200+
console.warn("Transport error encountered during transaction processing", errorOrCloseEvent);
201+
return true;
202+
},
203+
connectionAckWaitTimeout: DEFAULT_CONN_ACK_TIMEOUT,
204+
...options,
205+
lazy: false,
206+
// super ultra critical option that ensures graphql-ws doesn't automatically close the websocket connection when there are no outstanding operations. this is key so we can start a transaction then make mutations within it
207+
lazyCloseTimeout: 100000,
208+
retryAttempts: 0,
209+
});
210+
211+
const client = new Client({
212+
url: "/-", // not used because there's no fetch exchange, set for clarity
213+
requestPolicy: "network-only", // skip any cached data during transactions
214+
exchanges: [
215+
...this.exchanges.beforeAll,
216+
operationNameExchange,
217+
...this.exchanges.beforeAsync,
218+
subscriptionExchange({
219+
forwardSubscription(request) {
220+
const input = { ...request, query: request.query || "" };
221+
return {
222+
subscribe: (sink) => {
223+
const dispose = subscriptionClient!.subscribe(input, sink as Sink<ExecutionResult>);
224+
return {
225+
unsubscribe: dispose,
226+
};
227+
},
228+
};
229+
},
230+
enableAllOperations: true,
231+
}),
232+
...this.exchanges.afterAll,
233+
],
234+
});
235+
(client as any)[$gadgetConnection] = this;
236+
237+
transaction = new GadgetTransaction(client, subscriptionClient);
238+
this.currentTransaction = transaction;
239+
await transaction.start();
240+
const result = await run(transaction);
241+
await transaction.commit();
242+
return result;
243+
} catch (error) {
207244
try {
208-
// The server will error if it receives any operations before the auth dance has been completed, so we block on that happening before sending our first operation. It's important that this happens synchronously after instantiating the client so we don't miss any messages
209-
subscriptionClient = await this.waitForOpenedConnection({
210-
isFatalConnectionProblem(errorOrCloseEvent) {
211-
// any interruption of the transaction is fatal to the transaction
212-
console.warn("Transport error encountered during transaction processing", errorOrCloseEvent);
213-
return true;
214-
},
215-
connectionAckWaitTimeout: DEFAULT_CONN_ACK_TIMEOUT,
216-
...options,
217-
lazy: false,
218-
// super ultra critical option that ensures graphql-ws doesn't automatically close the websocket connection when there are no outstanding operations. this is key so we can start a transaction then make mutations within it
219-
lazyCloseTimeout: 100000,
220-
retryAttempts: 0,
221-
});
222-
223-
const client = new Client({
224-
url: "/-", // not used because there's no fetch exchange, set for clarity
225-
requestPolicy: "network-only", // skip any cached data during transactions
226-
exchanges: [
227-
...this.exchanges.beforeAll,
228-
operationNameExchange,
229-
otelExchange,
230-
...this.exchanges.beforeAsync,
231-
subscriptionExchange({
232-
forwardSubscription(request) {
233-
const input = { ...request, query: request.query || "" };
234-
return {
235-
subscribe: (sink) => {
236-
const dispose = subscriptionClient!.subscribe(input, sink as Sink<ExecutionResult>);
237-
return {
238-
unsubscribe: dispose,
239-
};
240-
},
241-
};
242-
},
243-
enableAllOperations: true,
244-
}),
245-
...this.exchanges.afterAll,
246-
],
247-
});
248-
(client as any)[$gadgetConnection] = this;
249-
250-
transaction = new GadgetTransaction(client, subscriptionClient);
251-
this.currentTransaction = transaction;
252-
await transaction.start();
253-
const result = await run(transaction);
254-
await transaction.commit();
255-
return result;
256-
} catch (error) {
257-
try {
258-
if (transaction?.open) await transaction.rollback();
259-
} catch (rollbackError) {
260-
if (!(rollbackError instanceof TransactionRolledBack)) {
261-
console.warn("Encountered another error while rolling back a Gadget transaction that errored. The other error:", rollbackError);
262-
}
263-
}
264-
if (isCloseEvent(error)) {
265-
throw new GadgetUnexpectedCloseError(error);
266-
} else {
267-
throw error;
245+
if (transaction?.open) await transaction.rollback();
246+
} catch (rollbackError) {
247+
if (!(rollbackError instanceof TransactionRolledBack)) {
248+
console.warn("Encountered another error while rolling back a Gadget transaction that errored. The other error:", rollbackError);
268249
}
269-
} finally {
270-
await subscriptionClient?.dispose();
271-
this.currentTransaction = null;
272250
}
251+
if (isCloseEvent(error)) {
252+
throw new GadgetUnexpectedCloseError(error);
253+
} else {
254+
throw error;
255+
}
256+
} finally {
257+
await subscriptionClient?.dispose();
258+
this.currentTransaction = null;
273259
}
274-
);
260+
};
275261

276262
close() {
277263
this.disposeClient(this.baseSubscriptionClient);
@@ -290,7 +276,7 @@ export class GadgetConnection {
290276
* // fetch a relative URL from the endpoint this API client is configured to fetch from
291277
* await api.connection.fetch("/foo/bar");
292278
**/
293-
fetch = traceFunction("api-client.fetch", async (input: RequestInfo | URL, init: RequestInit = {}) => {
279+
fetch = async (input: RequestInfo | URL, init: RequestInit = {}) => {
294280
input = processMaybeRelativeInput(input, this.options.baseRouteURL ?? this.options.endpoint);
295281

296282
if (this.isGadgetRequest(input)) {
@@ -311,7 +297,7 @@ export class GadgetConnection {
311297
}
312298

313299
return response;
314-
});
300+
};
315301

316302
private isGadgetRequest(input: RequestInfo | URL) {
317303
let requestUrl;
@@ -345,7 +331,7 @@ export class GadgetConnection {
345331
}
346332

347333
private newBaseClient() {
348-
const exchanges = [...this.exchanges.beforeAll, operationNameExchange, otelExchange, urlParamExchange];
334+
const exchanges = [...this.exchanges.beforeAll, operationNameExchange, urlParamExchange];
349335

350336
// apply urql's default caching behaviour when client side (but skip it server side)
351337
if (typeof window != "undefined") {

packages/api-client-core/src/exchanges/otelExchange.ts

Lines changed: 0 additions & 34 deletions
This file was deleted.

packages/api-client-core/src/support.ts

Lines changed: 0 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
import type { Span, SpanOptions } from "@opentelemetry/api";
2-
import { SpanStatusCode, context as contextApi, trace as traceApi } from "@opentelemetry/api";
31
import type { OperationResult } from "@urql/core";
42
import { CombinedError } from "@urql/core";
53
import { DataHydrator } from "./DataHydrator.js";
@@ -424,91 +422,6 @@ export const errorMessage = (error: unknown) => {
424422
}
425423
};
426424

427-
export const tracer = traceApi.getTracer("gadget-api-client");
428-
429-
export const onSpanError = (span: Span, error: any) => {
430-
span.recordException(error);
431-
span.setStatus({ code: SpanStatusCode.ERROR, message: errorMessage(error) });
432-
};
433-
434-
export function withSpan<T>(name: string, fn: (span: Span) => T): T;
435-
export function withSpan<T>(name: string, options: SpanOptions, fn: (span: Span) => T): T;
436-
export function withSpan<T>(name: string, fnOrOptions: SpanOptions | ((span: Span) => T), fn?: (span: Span) => T): T {
437-
let func: (span: Span) => T;
438-
let options: SpanOptions | undefined;
439-
440-
if (fn) {
441-
func = fn;
442-
options = fnOrOptions as SpanOptions;
443-
} else {
444-
func = fnOrOptions as typeof func;
445-
options = {};
446-
}
447-
448-
return tracer.startActiveSpan(name, options, (span) => func(span));
449-
}
450-
451-
export async function trace<T extends Promise<any>>(name: string, fn: (span: Span) => T): Promise<Awaited<T>>;
452-
export async function trace<T extends Promise<any>>(name: string, options: SpanOptions, fn: (span: Span) => T): Promise<Awaited<T>>;
453-
export async function trace<T extends Promise<any>>(
454-
name: string,
455-
fnOrOptions: SpanOptions | ((span: Span) => T),
456-
fn?: (span: Span) => T
457-
): Promise<Awaited<T>> {
458-
let func: (span: Span) => T;
459-
let options: SpanOptions | undefined;
460-
461-
if (fn) {
462-
func = fn;
463-
options = fnOrOptions as SpanOptions;
464-
} else {
465-
func = fnOrOptions as typeof func;
466-
options = {};
467-
}
468-
469-
return await withSpan(name, options, async (span) => {
470-
try {
471-
const result = await func(span);
472-
span.end();
473-
return result;
474-
} catch (error) {
475-
onSpanError(span, error);
476-
span.end();
477-
throw error;
478-
}
479-
});
480-
}
481-
482-
/** Wrap a function in tracing, and return it */
483-
export function traceFunction<This, Fn extends (this: This, ...args: any[]) => Promise<any>>(name: string, fn: Fn): Fn;
484-
export function traceFunction<This, Fn extends (this: This, ...args: any[]) => Promise<any>>(
485-
name: string,
486-
options: SpanOptions,
487-
fn: Fn
488-
): Fn;
489-
export function traceFunction<This, Fn extends (this: This, ...args: any[]) => Promise<any>>(
490-
name: string,
491-
fnOrOptions: SpanOptions | Fn,
492-
fn?: Fn
493-
): Fn {
494-
let func: Fn;
495-
let options: SpanOptions;
496-
497-
if (fn) {
498-
func = fn;
499-
options = fnOrOptions as SpanOptions;
500-
} else {
501-
func = fnOrOptions as Fn;
502-
options = {};
503-
}
504-
505-
return async function (this: any, ...args: Parameters<Fn>) {
506-
return await trace(name, options, () => func.apply(this, args));
507-
} as Fn;
508-
}
509-
510-
export const getCurrentSpan = () => traceApi.getSpan(contextApi.active());
511-
512425
// Gadget Storage Test Key that minifies well
513426
const key = "gstk";
514427

pnpm-lock.yaml

Lines changed: 0 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)