Skip to content

fix: retain stripped spans after customizer hook failures - #2614

Draft
Andrew Kent (realark) wants to merge 1 commit into
mainfrom
ark/SDK-472-customizer-error-handling
Draft

Andrew Kent (realark) wants to merge 1 commit into
mainfrom
ark/SDK-472-customizer-error-handling

Conversation

@realark

@realark Andrew Kent (realark) commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

AI Summary

Implements ONLY span-customizer Hook failures.

  • Exceptions and invalid returns stop the chain and export only original identity, routing/protocol fields, and existing duration timestamps, with context replaced by { customizer_error: true }.
  • Removes other content and previous hook mutations; no failed-record attachment uploads. Plain-object validation accepts null-prototype and cross-realm objects, rejects custom prototypes and promises, and consumes rejected promises.
  • Safe failure logging is once per JavaScript global environment, shared across SDK bundles; diagnostics contain no payload or exception details.
  • Memoized stripped records bypass downstream masking and form ordered, separate transport groups. This avoids mutation by in-place merging and preserves transaction ordering without duplicate same-span IDs in a request. Successful records keep normal merge/masking behavior within those boundaries.
  • Removes obsolete null/drop plumbing and updates regression tests, API docs, instrumentation docs, and changelog.

Verification

  • pnpm --filter braintrust check:typings — passed (source and test TypeScript projects).
  • vitest run src/span-customizer.test.ts src/masking.test.ts src/logger.test.ts — 181 tests passed.
  • Runtime HTTP exporter regression covers failed/successful incremental records, ordered requests, HTTP 503 retry, unchanged original timestamps, masking immunity, attachment exclusion, unrelated attachment upload, and hook/log counts.
  • pnpm --filter braintrust build — all JS entrypoints and declarations built successfully.
  • Node smoke against built ESM and CJS SDK bundles with injected fetch transport — 6 stripped records exported, exactly 1 safe failure log across both bundles, no payload leak.
  • Scoped ESLint — 0 errors, 117 warnings (including tests ignored by repository ESLint configuration).
  • Scoped Prettier check — passed.

Scope / limitations

Native SDK spans only; existing OTel compatibility restrictions are unchanged. Incremental stripping cannot retract earlier successful exports. Runtime transport smoke used an injected local fetch implementation, not a live Braintrust service. Existing untracked js/tests/auto-instrumentations/fixtures/test-files/ was preserved and excluded from the commit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant