Skip to content

Commit 48a3c78

Browse files
committed
fix(openfeature): retry exposure fallback failures
1 parent 65047b4 commit 48a3c78

3 files changed

Lines changed: 20 additions & 12 deletions

File tree

packages/dd-trace/src/evp_proxy/discovery.js

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,10 @@ const TRAILING_SLASHES = /\/+$/
2929
* `/info` endpoint returns an error through the shared request timeout and
3030
* retry policy. A valid response without a compatible path returns no route.
3131
* Discovery sends no events, so the caller can safely select direct intake
32-
* after either result. The caller also owns later delivery failures. It can
33-
* switch future batches after an ambiguous timeout or reset, but it must not
34-
* replay the current batch because the first receiver might have accepted it.
32+
* after either result. The caller also owns later delivery failures. Exposure
33+
* delivery uses retries and can therefore produce duplicates. After local
34+
* retries fail, the caller can retry through direct intake and use that route
35+
* for future batches.
3536
*
3637
* Reference implementations:
3738
*

packages/dd-trace/src/openfeature/writers/base.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,18 +40,19 @@ const log = require('../../log')
4040
* @returns {boolean} Whether direct retry is safe
4141
*/
4242
function isDefinitiveRejection (error, statusCode) {
43-
return error?.code === 'ECONNREFUSED' || error?.code === 'ENOENT' ||
43+
return error?.code === 'EAI_AGAIN' || error?.code === 'ECONNREFUSED' ||
44+
error?.code === 'ENOENT' || error?.code === 'ENOTFOUND' ||
4445
statusCode === 403 || statusCode === 404 || statusCode === 405
4546
}
4647

4748
/**
4849
* Tests whether a local route can have accepted an event batch before failing.
4950
*
5051
* @param {Error | null} error - Request error
51-
* @returns {boolean} Whether the current batch must not be replayed
52+
* @returns {boolean} Whether the delivery result is ambiguous
5253
*/
5354
function isAmbiguousNetworkFailure (error) {
54-
return error?.code === 'ECONNRESET' || error?.code === 'ETIMEDOUT'
55+
return error?.code === 'ECONNRESET' || error?.code === 'EPIPE' || error?.code === 'ETIMEDOUT'
5556
}
5657

5758
/**
@@ -257,7 +258,7 @@ class BaseFFEWriter {
257258
}
258259

259260
/**
260-
* Sends an encoded batch and retries it directly only after definitive rejection.
261+
* Sends an encoded batch and retries it through direct intake after a local route failure.
261262
*
262263
* @param {string} payload - Encoded event batch
263264
* @param {number} eventCount - Event count
@@ -282,13 +283,15 @@ class BaseFFEWriter {
282283

283284
if (fallbackRoute && isAmbiguousNetworkFailure(error)) {
284285
log.debug(
285-
'%s switching future batches from %s%s to direct intake after ambiguous failure',
286+
'%s retrying through direct intake and switching future batches from %s%s after ambiguous failure',
286287
this.constructor.name,
287288
route.url.href,
288289
route.endpoint
289290
)
290291
this.#activateRoute(fallbackRoute)
291292
this._fallbackRoute = undefined
293+
this.#sendRequest(payload, eventCount, fallbackRoute)
294+
return
292295
}
293296

294297
if (error) {

packages/dd-trace/test/openfeature/writers/exposures.spec.js

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -424,11 +424,13 @@ describe('OpenFeature Exposures Writer', () => {
424424
})
425425

426426
for (const [name, error, statusCode] of [
427+
['temporary DNS failure', Object.assign(new Error('getaddrinfo EAI_AGAIN'), { code: 'EAI_AGAIN' })],
427428
['connection refusal', Object.assign(new Error('connect ECONNREFUSED'), { code: 'ECONNREFUSED' })],
428429
['missing Unix socket', Object.assign(
429430
new Error('connect ENOENT /var/run/datadog/apm.socket'),
430431
{ code: 'ENOENT' }
431432
)],
433+
['unresolvable hostname', Object.assign(new Error('getaddrinfo ENOTFOUND'), { code: 'ENOTFOUND' })],
432434
['HTTP 403', Object.assign(new Error('Forbidden'), { status: 403 }), 403],
433435
['HTTP 404', Object.assign(new Error('Not Found'), { status: 404 }), 404],
434436
['HTTP 405', Object.assign(new Error('Method Not Allowed'), { status: 405 }), 405],
@@ -476,9 +478,10 @@ describe('OpenFeature Exposures Writer', () => {
476478

477479
for (const [name, error, statusCode] of [
478480
['connection reset', Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' })],
481+
['broken pipe', Object.assign(new Error('write EPIPE'), { code: 'EPIPE' })],
479482
['timeout', Object.assign(new Error('request timed out'), { code: 'ETIMEDOUT' })],
480483
]) {
481-
it(`should switch future batches after ambiguous local ${name} without replaying the current batch`, async () => {
484+
it(`should retry ambiguous local ${name} through direct intake and switch future batches`, async () => {
482485
const localUrl = new URL('http://serverless-init:8126')
483486
const directUrl = new URL('https://event-platform-intake.datadoghq.com')
484487
request.onFirstCall().yieldsAsync(error, null, statusCode)
@@ -501,14 +504,15 @@ describe('OpenFeature Exposures Writer', () => {
501504
writer.flush()
502505
await clock.tickAsync(0)
503506

504-
sinon.assert.calledOnce(request)
507+
sinon.assert.calledTwice(request)
505508
assert.strictEqual(request.firstCall.args[1].url, localUrl)
509+
assert.strictEqual(request.secondCall.args[1].url, directUrl)
506510

507511
writer.append(exposureEvent)
508512
writer.flush()
509513

510-
sinon.assert.calledTwice(request)
511-
assert.strictEqual(request.secondCall.args[1].url, directUrl)
514+
sinon.assert.calledThrice(request)
515+
assert.strictEqual(request.thirdCall.args[1].url, directUrl)
512516
})
513517
}
514518

0 commit comments

Comments
 (0)