fix: three v0.2.0 gaps — partial span flush, intake exclusion by origin, crash faulting address - #12
Merged
Merged
Conversation
Independent fix 1 of 3. RUM resource events for main-process HTTP calls are derived from the spans dd-trace exports. dd-trace hands a trace to its exporter only once every span it started has finished, or once `flushMinSpans` spans have finished — and that threshold defaults to 1000, which a desktop application never reaches. In practice only the first condition applied. So a single request that never came back — a hung endpoint, a socket left open — withheld the resource events of every other request issued from the same `ipcMain.handle` invocation, silently and for the rest of the process' life. Setting `flushMinSpans: 1` exports each span as it finishes; the unfinished ones stay in the trace and are exported later, if ever. Attribution does not move: a resource event is placed by its own span's start time, not the trace's, so producing it earlier only makes the view lookup more accurate. The new e2e scenario issues two requests from one IPC handler, one of them to an endpoint that never answers, and asserts the other still produces its resource event. It times out without this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ented Independent fix 2 of 3. `isIntakeRequest()` compared the hostname of a request against the hostname of the intake, which is wrong in both directions: - with `proxy` set, `computeIntakeHostname` dropped its port, so every application request sharing the proxy's host was discarded whatever its port — the normal shape of a self-hosted deployment; - with no `proxy` and a `site` carrying a port (`rum.example.internal:8443`, a valid configuration), the stored value kept the port while `new URL(url) .hostname` never has one, so the comparison matched nothing, the SDK reported its own uploads as resources, and those resources produced further uploads. It also matched on `span.resource`, which dd-trace sets to the HTTP method for client spans, so that branch could only ever produce false positives. Both directions come from the same mistake, and both are fixed by comparing origins — `computeIntakeHostname` becomes `computeIntakeOrigin`. The exclusion also moves to where every other FlashCat SDK puts it. On iOS, Android and HarmonyOS the uploader is simply not part of what gets instrumented; Electron was the only one filtering after the fact, at export time. `Tracing` now passes dd-trace a `blocklist` for the intake origin, which `HttpClientPlugin` applies before recording — and all three span-producing paths (`node:http`, global `fetch`, Electron `net.request`) derive from it. A blocked request produces no span at all. `SpanProcessor` keeps a corrected check as a second line of defense. The e2e harness could not see any of this: it pointed the intake at `localhost` while the test server answered on `127.0.0.1`, so the two never collided as hostnames and a host-only exclusion passed every scenario. Both now use `127.0.0.1` and differ only by port, and a new scenario asserts the two are told apart. With the fix reverted, five of the six resource scenarios fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Independent fix 3 of 3. The minidump processor has always resolved `crash_info.address` and `CrashCollection` dropped it, keeping only the exception type. It goes into `error.meta.exception_codes` — the RUM schema's field for "CPU specific information about the exception encoded into 64-bit hexadecimal number" — rather than a name of our own, because the intake decodes `error.meta` into a fixed set of fields and drops the rest, so an invented key would never reach the console. This closes an investigation into native crashes reported as `unknown 0x00000000 / 0x00000000`, which turned out to need no fix. Verified in a real Electron app on macOS: `process.crash()` writes through a null pointer, Crashpad records the Mach exception, and the report reads `EXC_BAD_ACCESS / KERN_INVALID_ADDRESS`. The signal-to-name mapping in the processor works. `unknown 0x…` is what the processor falls back to when the minidump carries no exception record at all, which is a property of how the process died rather than a gap in the SDK — on macOS every signal delivered with `kill()` produces such a dump. That is exactly when the faulting address is the only lead left, which is what makes adding it worth doing. No signal-name table of our own: the processor already has complete ones, and a second would only be a second truth. `crash.scenario.ts` now asserts the exception type is a real name and the address is present; both fail if the address is dropped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three independent fixes, one commit each. They share no code and can be reverted separately.
1. Export a finished span without waiting for its trace siblings
src/entries/instrument.ts—flushMinSpans: 1.RUM
resourceevents are derived from the spans dd-trace exports, and dd-trace exports a trace only once every span in it has finished — or onceflushMinSpanshave, a threshold defaulting to 1000 that a desktop app never reaches. A single request that never returned therefore withheld the resource events of every other request made from the sameipcMain.handleinvocation, for the rest of the process' life.Attribution does not move: a resource event is placed by its own span's start time, not the trace's.
Verified in a real Electron app, not only in Node. New scenario: two requests from one IPC handler, one to an endpoint that never answers.
status_code: 200, correct URLNo loss and no duplication elsewhere: the three single-request scenarios still assert exactly one resource event each, with
_dd.trace_id/_dd.span_idintact.2. Exclude the SDK's own uploads by origin, before they are instrumented
isIntakeRequest()compared hostnames, which is wrong in both directions:proxyset, its port was dropped, so every application request sharing the proxy's host was discarded whatever its port — the normal shape of a self-hosted deployment;proxyand asitecarrying a port (rum.example.internal:8443, a valid configuration), the stored value kept the port whilenew URL(url).hostnamenever has one, so the comparison matched nothing and the SDK reported its own uploads as resources — which produced further uploads.It also matched on
span.resource, which dd-trace sets to the HTTP method for client spans, so that branch could only ever produce false positives. It is gone.Both directions are fixed by comparing origins (
computeIntakeHostname→computeIntakeOrigin), and the exclusion moves to where every other FlashCat SDK puts it. On iOS, Android and HarmonyOS the uploader is simply not part of what gets instrumented; Electron was the only one filtering after the fact, at export time.Tracingnow hands dd-trace ablocklistfor the intake origin, applied byHttpClientPluginbefore a span is recorded — and all three span-producing paths (node:http, globalfetch, Electronnet.request) derive from it.SpanProcessorkeeps a corrected check as a second line of defense.The e2e harness could not see any of this: it pointed the intake at
localhostwhile the test server answered on127.0.0.1, so the two never collided as hostnames and a host-only exclusion passed every scenario. Both now use127.0.0.1and differ only by port.Mutation-verified, in a real Electron app:
3. Report the faulting address of a native crash — and why the
unknown 0x…type is not a bugVerified in a real Electron app on macOS before changing anything.
process.crash()writes through a null pointer, Crashpad records the Mach exception, and the report readsEXC_BAD_ACCESS / KERN_INVALID_ADDRESSwith address0x0. The processor's exception mapping works; no fix was needed.unknown 0x00000000 / 0x00000000is the fallback for a minidump carrying no exception record — a property of how the process died, not a gap in the SDK. Probing the same app with signals delivered bykill()(SIGSEGV,SIGABRT,SIGILL,SIGFPE) produced exactly that string every time, on macOS. So the sole change here is additive: the faulting address, which the processor had always resolved and the SDK dropped, now goes toerror.meta.exception_codes— that field rather than a name of our own because the intake decodeserror.metainto a fixed set of fields and drops the rest. It is the only lead left precisely when the type has no name. No signal-name table of our own.crash.scenario.tsnow asserts the type is a real name and the address is present; both fail if the address is dropped.Verification
yarn test:unityarn test:e2eyarn test:integrationyarn typecheck,yarn test:e2e:typecheckandyarn format:checkclean.yarn lintreports the same 2 pre-existing errors aspublish, in files this branch does not touch.Every behavior change has a guard proven to fail without it — see each section above.