Skip to content

Commit 0620925

Browse files
metal-messiahmetal-messiah
committed
better fallback
1 parent 58663ec commit 0620925

7 files changed

Lines changed: 219 additions & 27 deletions

File tree

‎src/features/ajax/instrument/index.js‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -367,11 +367,9 @@ function subscribeToEvents (agentRef, ee, handler, dt) {
367367
this.params.status = res ? res.status : 0
368368

369369
const finishAndReport = () => {
370-
// convert rxSize to a number
371-
let responseSize
372-
if (typeof this.rxSize === 'string' && this.rxSize.length > 0) {
373-
responseSize = +this.rxSize
374-
}
370+
// convert rxSize to a number - handle both string (from content-length header) and number (from fallback)
371+
const num = +this.rxSize
372+
const responseSize = this.rxSize != null && !isNaN(num) ? num : undefined
375373

376374
const metrics = {
377375
txSize: this.txSize,
@@ -393,8 +391,8 @@ function subscribeToEvents (agentRef, ee, handler, dt) {
393391
// Clone the response to read the body without consuming the original
394392
res.clone().text().then((text) => {
395393
this.responseBody = text
396-
// Use captured payload size as fallback if content-length header was missing
397-
if (!this.rxSize && text !== undefined) {
394+
// Use captured payload size as fallback if content-length header was missing or is 0 with a body
395+
if ((!this.rxSize || this.rxSize === '0' || this.rxSize === 0) && text !== undefined) {
398396
this.rxSize = dataSize(text)
399397
}
400398
if (res?.headers) {
@@ -441,8 +439,8 @@ function subscribeToEvents (agentRef, ee, handler, dt) {
441439
this.responseBody = xhr.response
442440
}
443441

444-
// Use captured payload size as fallback if not already determined
445-
if (!metrics.rxSize && this.responseBody !== undefined) {
442+
// Use captured payload size as fallback if not already determined or is 0 with a body
443+
if ((!metrics.rxSize || metrics.rxSize === 0) && this.responseBody !== undefined) {
446444
const size = dataSize(this.responseBody)
447445
if (size !== undefined) metrics.rxSize = size
448446
}
@@ -466,7 +464,7 @@ function subscribeToEvents (agentRef, ee, handler, dt) {
466464
ctx.params.status = xhr.status
467465

468466
var size = responseSizeFromXhr(xhr, ctx.lastSize)
469-
if (size) ctx.metrics.rxSize = size
467+
if (size !== undefined) ctx.metrics.rxSize = size
470468

471469
if (ctx.sameOrigin && xhr.getAllResponseHeaders().indexOf(NR_CAT_HEADER) >= 0) {
472470
var header = xhr.getResponseHeader(NR_CAT_HEADER)

‎tests/assets/ajax/fetch-empty-204.html‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>Fetch Empty 204</title>
5+
{init} {config} {loader}
6+
</head>
7+
<body>
8+
<div>
9+
This page tests fetch with a 204 No Content response (empty body).
10+
The agent should report rxSize as 0.
11+
</div>
12+
<div><button id="sendAjax">Send Ajax</button></div>
13+
<script>
14+
window.disableAjaxHashChange = false
15+
document.getElementById('sendAjax').addEventListener('click', function () {
16+
fetch('/empty-204').then(function () {
17+
if (!disableAjaxHashChange) {
18+
window.location.hash = Math.random()
19+
}
20+
})
21+
})
22+
</script>
23+
</body>
24+
</html>

‎tests/assets/ajax/fetch-no-content-length.html‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>Fetch No Content-Length</title>
5+
{init} {config} {loader}
6+
</head>
7+
<body>
8+
<div>
9+
This page tests fetch with a response that has no content-length header.
10+
The agent should fall back to using the captured payload size.
11+
</div>
12+
<div><button id="sendAjax">Send Ajax</button></div>
13+
<script>
14+
window.disableAjaxHashChange = false
15+
document.getElementById('sendAjax').addEventListener('click', function () {
16+
fetch('/json-no-content-length').then(function () {
17+
if (!disableAjaxHashChange) {
18+
window.location.hash = Math.random()
19+
}
20+
})
21+
})
22+
</script>
23+
</body>
24+
</html>

‎tests/assets/ajax/fetch-zero-content-length.html‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
<title>Fetch Zero Content-Length</title>
5+
{init} {config} {loader}
6+
</head>
7+
<body>
8+
<div>
9+
This page tests fetch with a response that has content-length: 0 but actual body content.
10+
The agent should fall back to using the captured payload size.
11+
</div>
12+
<div><button id="sendAjax">Send Ajax</button></div>
13+
<script>
14+
window.disableAjaxHashChange = false
15+
document.getElementById('sendAjax').addEventListener('click', function () {
16+
fetch('/json-zero-content-length').then(function () {
17+
if (!disableAjaxHashChange) {
18+
window.location.hash = Math.random()
19+
}
20+
})
21+
})
22+
</script>
23+
</body>
24+
</html>

‎tests/components/ajax/instrument.test.js‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -229,21 +229,6 @@ describe('payload capture configuration', () => {
229229
})
230230

231231
describe('response size fallback from payload capture', () => {
232-
describe('fetch', () => {
233-
// Note: fetch tests are integration tests in tests/specs/ajax/fetch.e2e.js
234-
// These unit tests verify the logic is correct
235-
test('verifies fallback logic exists in source code', () => {
236-
const fs = require('fs')
237-
const path = require('path')
238-
const instrumentPath = path.join(__dirname, '../../../src/features/ajax/instrument/index.js')
239-
const instrumentCode = fs.readFileSync(instrumentPath, 'utf8')
240-
241-
// Verify the fetch fallback logic exists
242-
expect(instrumentCode).toContain('if (!this.rxSize && text !== undefined)')
243-
expect(instrumentCode).toContain('this.rxSize = dataSize(text)')
244-
})
245-
})
246-
247232
describe('xhr', () => {
248233
test('uses captured payload size when content-length header is missing', done => {
249234
const responseBody = '<html><body>Test Response</body></html>'
@@ -361,8 +346,8 @@ describe('response size fallback from payload capture', () => {
361346
const instrumentPath = path.join(__dirname, '../../../src/features/ajax/instrument/index.js')
362347
const instrumentCode = fs.readFileSync(instrumentPath, 'utf8')
363348

364-
// Verify the XHR fallback logic exists
365-
expect(instrumentCode).toContain('if (!metrics.rxSize && this.responseBody !== undefined)')
349+
// Verify the XHR fallback logic exists and handles missing or zero rxSize
350+
expect(instrumentCode).toContain('if ((!metrics.rxSize || metrics.rxSize === 0) && this.responseBody !== undefined)')
366351
expect(instrumentCode).toContain('const size = dataSize(this.responseBody)')
367352
expect(instrumentCode).toContain('if (size !== undefined) metrics.rxSize = size')
368353
})

‎tests/specs/ajax/fetch.e2e.js‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,4 +377,114 @@ describe('Fetch Ajax', () => {
377377
expect(ajaxMetric.metrics.rxSize.t).toBeUndefined()
378378
expect(ajaxMetric.metrics.rxSize.c).toEqual(1)
379379
})
380+
381+
describe('response size fallback from payload capture', () => {
382+
it('uses captured payload size when content-length header is missing', async () => {
383+
await browser.url(await browser.testHandle.assetURL('ajax/fetch-no-content-length.html', {
384+
init: { ajax: { capture_payloads: 'all' } },
385+
loader: 'full'
386+
}))
387+
.then(() => browser.waitForAgentLoad())
388+
.then(() => browser.execute(function () {
389+
window.disableAjaxHashChange = true
390+
}))
391+
392+
const [ajaxEventsHarvest, ajaxMetricsHarvest] = await Promise.all([
393+
ajaxEventsCapture.waitForResult({ timeout: 10000 }),
394+
ajaxMetricsCapture.waitForResult({ timeout: 10000 }),
395+
$('#sendAjax').click()
396+
])
397+
398+
ajaxEventsHarvest.forEach(harvest =>
399+
checkAjaxEvents(harvest.request, { specificPath: '/json-no-content-length' })
400+
)
401+
ajaxMetricsHarvest.forEach(harvest =>
402+
checkAjaxMetrics(harvest.request, { specificPath: '/json-no-content-length', isFetch: true })
403+
)
404+
405+
const ajaxEvent = ajaxEventsHarvest
406+
.flatMap(harvest => harvest.request.body)
407+
.find(event => event.path === '/json-no-content-length')
408+
expect(ajaxEvent.responseBodySize).toBeGreaterThan(0)
409+
410+
const ajaxMetric = ajaxMetricsHarvest
411+
.flatMap(harvest => harvest.request.body.xhr)
412+
.find(metric => metric.params.pathname === '/json-no-content-length')
413+
expect(ajaxMetric.metrics.rxSize.t).toBeGreaterThan(0)
414+
// Verify the size matches the expected JSON response body
415+
expect(ajaxMetric.metrics.rxSize.t).toEqual(JSON.stringify({ text: 'response without content-length header' }).length)
416+
})
417+
418+
it('uses captured payload size when content-length is 0 with actual body', async () => {
419+
await browser.url(await browser.testHandle.assetURL('ajax/fetch-zero-content-length.html', {
420+
init: { ajax: { capture_payloads: 'all' } },
421+
loader: 'full'
422+
}))
423+
.then(() => browser.waitForAgentLoad())
424+
.then(() => browser.execute(function () {
425+
window.disableAjaxHashChange = true
426+
}))
427+
428+
const [ajaxEventsHarvest, ajaxMetricsHarvest] = await Promise.all([
429+
ajaxEventsCapture.waitForResult({ timeout: 10000 }),
430+
ajaxMetricsCapture.waitForResult({ timeout: 10000 }),
431+
$('#sendAjax').click()
432+
])
433+
434+
ajaxEventsHarvest.forEach(harvest =>
435+
checkAjaxEvents(harvest.request, { specificPath: '/json-zero-content-length' })
436+
)
437+
ajaxMetricsHarvest.forEach(harvest =>
438+
checkAjaxMetrics(harvest.request, { specificPath: '/json-zero-content-length', isFetch: true })
439+
)
440+
441+
const ajaxEvent = ajaxEventsHarvest
442+
.flatMap(harvest => harvest.request.body)
443+
.find(event => event.path === '/json-zero-content-length')
444+
expect(ajaxEvent.responseBodySize).toBeGreaterThan(0)
445+
446+
const ajaxMetric = ajaxMetricsHarvest
447+
.flatMap(harvest => harvest.request.body.xhr)
448+
.find(metric => metric.params.pathname === '/json-zero-content-length')
449+
expect(ajaxMetric.metrics.rxSize.t).toBeGreaterThan(0)
450+
// Verify the size matches the expected JSON response body
451+
expect(ajaxMetric.metrics.rxSize.t).toEqual(JSON.stringify({ text: 'content-length says 0 but body exists' }).length)
452+
})
453+
454+
it('reports rxSize as 0 for empty 204 No Content response', async () => {
455+
await browser.url(await browser.testHandle.assetURL('ajax/fetch-empty-204.html', {
456+
init: { ajax: { capture_payloads: 'all' } },
457+
loader: 'full'
458+
}))
459+
.then(() => browser.waitForAgentLoad())
460+
.then(() => browser.execute(function () {
461+
window.disableAjaxHashChange = true
462+
}))
463+
464+
const [ajaxEventsHarvest, ajaxMetricsHarvest] = await Promise.all([
465+
ajaxEventsCapture.waitForResult({ timeout: 10000 }),
466+
ajaxMetricsCapture.waitForResult({ timeout: 10000 }),
467+
$('#sendAjax').click()
468+
])
469+
470+
ajaxEventsHarvest.forEach(harvest =>
471+
checkAjaxEvents(harvest.request, { specificPath: '/empty-204' })
472+
)
473+
ajaxMetricsHarvest.forEach(harvest =>
474+
checkAjaxMetrics(harvest.request, { specificPath: '/empty-204', isFetch: true })
475+
)
476+
477+
const ajaxEvent = ajaxEventsHarvest
478+
.flatMap(harvest => harvest.request.body)
479+
.find(event => event.path === '/empty-204')
480+
expect(ajaxEvent.responseBodySize).toEqual(0)
481+
expect(ajaxEvent.status).toEqual(204)
482+
483+
const ajaxMetric = ajaxMetricsHarvest
484+
.flatMap(harvest => harvest.request.body.xhr)
485+
.find(metric => metric.params.pathname === '/empty-204')
486+
expect(ajaxMetric.metrics.rxSize.t).toEqual(0)
487+
expect(ajaxMetric.params.status).toEqual(204)
488+
})
489+
})
380490
})

‎tools/testing-server/routes/mock-apis.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,4 +416,31 @@ module.exports = fp(async function (fastify, testServer) {
416416
socket.send(message, { binary: isBinary })
417417
})
418418
})
419+
420+
// Response size fallback test endpoints
421+
fastify.get('/json-no-content-length', {
422+
compress: false
423+
}, (request, reply) => {
424+
const responseBody = JSON.stringify({ text: 'response without content-length header' })
425+
reply
426+
.removeHeader('content-length')
427+
.type('application/json')
428+
.send(responseBody)
429+
})
430+
431+
fastify.get('/json-zero-content-length', {
432+
compress: false
433+
}, (request, reply) => {
434+
const responseBody = JSON.stringify({ text: 'content-length says 0 but body exists' })
435+
reply
436+
.header('content-length', '0')
437+
.type('application/json')
438+
.send(responseBody)
439+
})
440+
441+
fastify.get('/empty-204', {
442+
compress: false
443+
}, (request, reply) => {
444+
reply.code(204).send()
445+
})
419446
})

0 commit comments

Comments
 (0)