Skip to content

Commit 17e7787

Browse files
committed
chore: add logs
1 parent d063858 commit 17e7787

File tree

2 files changed

+118
-25
lines changed

2 files changed

+118
-25
lines changed

packages/web-sdk/src/transport/FetchTransport/FetchTransport.test.ts

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
1+
import { DiagLogLevel, diag } from '@opentelemetry/api';
12
import * as chai from 'chai';
2-
import sinonChai from 'sinon-chai';
33
import {
44
fakeFetchGetKeepalive,
55
fakeFetchInstall,
66
fakeFetchRespondWith,
77
fakeFetchRestore,
8+
InMemoryDiagLogger,
89
} from '../../../tests/utils/index.ts';
910
import { _resetKeepaliveTracking, FetchTransport } from './FetchTransport.ts';
1011

11-
chai.use(sinonChai);
1212
const { expect } = chai;
1313

1414
const makeTransport = (
@@ -41,13 +41,18 @@ function createDeferred(): Deferred {
4141
}
4242

4343
describe('FetchTransport', () => {
44+
let diagLogger: InMemoryDiagLogger;
45+
4446
beforeEach(() => {
4547
_resetKeepaliveTracking();
4648
fakeFetchInstall();
49+
diagLogger = new InMemoryDiagLogger();
50+
diag.setLogger(diagLogger, DiagLogLevel.ALL);
4751
});
4852

4953
afterEach(() => {
5054
fakeFetchRestore();
55+
diag.disable();
5156
});
5257

5358
describe('keepalive budget', () => {
@@ -60,6 +65,16 @@ describe('FetchTransport', () => {
6065
expect(fakeFetchGetKeepalive()).to.equal(true);
6166
});
6267

68+
it('should set keepalive to true for payloads exactly at the budget', async () => {
69+
fakeFetchRespondWith('ok', { status: 200 });
70+
const transport = makeTransport();
71+
72+
const exactPayload = new Uint8Array(49152);
73+
await transport.send(exactPayload, 1000);
74+
75+
expect(fakeFetchGetKeepalive()).to.equal(true);
76+
});
77+
6378
it('should set keepalive to false for payloads exceeding the budget', async () => {
6479
fakeFetchRespondWith('ok', { status: 200 });
6580
const transport = makeTransport();
@@ -214,4 +229,75 @@ describe('FetchTransport', () => {
214229
});
215230
});
216231
});
232+
233+
describe('diagnostic logging', () => {
234+
it('should warn via diag when keepalive is downgraded for byte budget', async () => {
235+
fakeFetchRespondWith('ok', { status: 200 });
236+
const transport = makeTransport();
237+
238+
await transport.send(largePayload, 1000);
239+
240+
const warns = diagLogger.getWarnLogs();
241+
expect(warns).to.have.lengthOf(1);
242+
expect(warns[0]).to.include('inflight bytes');
243+
});
244+
245+
it('should warn via diag when keepalive is downgraded for concurrent count', async () => {
246+
const deferreds: Deferred[] = [];
247+
fakeFetchRestore();
248+
const stub = fakeFetchInstall();
249+
250+
for (let i = 0; i < 9; i++) {
251+
const d = createDeferred();
252+
deferreds.push(d);
253+
stub.onCall(i).returns(d.promise);
254+
}
255+
stub.onCall(9).resolves(new Response('ok', { status: 200 }));
256+
257+
const transport = makeTransport();
258+
const tinyPayload = new Uint8Array(1);
259+
260+
const pending = [];
261+
for (let i = 0; i < 9; i++) {
262+
pending.push(transport.send(tinyPayload, 5000));
263+
await Promise.resolve();
264+
}
265+
266+
// 10th request should trigger the warning
267+
await transport.send(tinyPayload, 5000);
268+
269+
const warns = diagLogger.getWarnLogs();
270+
expect(warns).to.have.lengthOf(1);
271+
expect(warns[0]).to.include('concurrent count');
272+
273+
for (const d of deferreds) {
274+
d.resolve(new Response('ok', { status: 200 }));
275+
}
276+
await Promise.all(pending);
277+
});
278+
279+
it('should warn via diag when fetch throws', async () => {
280+
fakeFetchRestore();
281+
fakeFetchInstall().rejects(new TypeError('Failed to fetch'));
282+
283+
const transport = makeTransport();
284+
await transport.send(smallPayload, 1000);
285+
286+
const warns = diagLogger.getWarnLogs();
287+
expect(warns).to.have.lengthOf(1);
288+
expect(warns[0]).to.include('Fetch transport failed');
289+
});
290+
291+
it('should log debug when HTTP response is not ok', async () => {
292+
fakeFetchRespondWith('error', { status: 500 });
293+
const transport = makeTransport();
294+
295+
await transport.send(smallPayload, 1000);
296+
297+
const match = diagLogger
298+
.getDebugLogs()
299+
.find((msg) => msg.includes('HTTP 500'));
300+
expect(match).to.be.a('string');
301+
});
302+
});
217303
});

packages/web-sdk/src/transport/FetchTransport/FetchTransport.ts

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,6 @@ export class FetchTransport implements IExporterTransport {
8585
...this._config.headers,
8686
};
8787

88-
if (this._config.compression === 'gzip') {
89-
request = await FetchTransport._compressRequest(data);
90-
headers['Content-Encoding'] = 'gzip';
91-
headers['Content-Length'] = request.length.toString();
92-
}
93-
9488
// Use AbortSignal.timeout if available, otherwise fallback to AbortController
9589
// https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/timeout_static
9690
let signal: AbortSignal;
@@ -106,21 +100,29 @@ export class FetchTransport implements IExporterTransport {
106100
}, timeoutMillis);
107101
}
108102

109-
const wouldExceedBytes =
110-
inflightKeepaliveBytes + request.byteLength > KEEPALIVE_BYTE_BUDGET;
111-
const wouldExceedCount = inflightKeepaliveCount >= MAX_KEEPALIVE_REQUESTS;
112-
const keepalive = !wouldExceedBytes && !wouldExceedCount;
113-
114-
if (keepalive) {
115-
inflightKeepaliveBytes += request.byteLength;
116-
inflightKeepaliveCount++;
117-
} else {
118-
diag.warn(
119-
`Sending without keepalive: ${wouldExceedBytes ? `inflight bytes (${inflightKeepaliveBytes} + ${request.byteLength}) would exceed ${KEEPALIVE_BYTE_BUDGET}B budget` : `concurrent count (${inflightKeepaliveCount}) at limit of ${MAX_KEEPALIVE_REQUESTS}`}`,
120-
);
121-
}
103+
let keepalive = false;
122104

123105
try {
106+
if (this._config.compression === 'gzip') {
107+
request = await FetchTransport._compressRequest(data);
108+
headers['Content-Encoding'] = 'gzip';
109+
headers['Content-Length'] = request.length.toString();
110+
}
111+
112+
const wouldExceedBytes =
113+
inflightKeepaliveBytes + request.byteLength > KEEPALIVE_BYTE_BUDGET;
114+
const wouldExceedCount = inflightKeepaliveCount >= MAX_KEEPALIVE_REQUESTS;
115+
keepalive = !wouldExceedBytes && !wouldExceedCount;
116+
117+
if (keepalive) {
118+
inflightKeepaliveBytes += request.byteLength;
119+
inflightKeepaliveCount++;
120+
} else {
121+
diag.warn(
122+
`Sending without keepalive: ${wouldExceedBytes ? `inflight bytes (${inflightKeepaliveBytes} + ${request.byteLength}) would exceed ${KEEPALIVE_BYTE_BUDGET}B budget` : `concurrent count (${inflightKeepaliveCount}) at limit of ${MAX_KEEPALIVE_REQUESTS}`}`,
123+
);
124+
}
125+
124126
const response = await fetch(this._config.url, {
125127
method: 'POST',
126128
keepalive,
@@ -132,16 +134,21 @@ export class FetchTransport implements IExporterTransport {
132134
if (response.ok) {
133135
return { status: 'success' };
134136
} else {
137+
diag.debug(
138+
`Fetch transport received HTTP ${response.status} (keepalive=${keepalive})`,
139+
);
135140
return {
136141
status: 'failure',
137142
error: new Error(`${response.status} Fetch request failed`),
138143
};
139144
}
140145
} catch (error) {
141-
return {
142-
status: 'failure',
143-
error: error instanceof Error ? error : new Error(String(error)),
144-
};
146+
const resolved =
147+
error instanceof Error ? error : new Error(String(error));
148+
diag.warn(
149+
`Fetch transport failed (keepalive=${keepalive}): ${resolved.message}`,
150+
);
151+
return { status: 'failure', error: resolved };
145152
} finally {
146153
if (timeoutId !== undefined) {
147154
clearTimeout(timeoutId);

0 commit comments

Comments
 (0)