✨ add trace sampling rules by rgaignault · Pull Request #199 · DataDog/electron-sdk

rgaignault · 2026-08-14T12:31:27Z

Motivation

Let Electron applications reduce noisy main-process traces with targeted sampling rules.

Changes

Adds ordered traceSamplingRules for service, operation, resource, and tag matching. The first matching rule samples the whole trace; unmatched traces are kept.

Sampled-out traces are not sent to APM or propagated through Electron HTTP requests. Their RUM Resources are still sent without trace identifiers.
Note: Trace sampling only controls APM spans. The associated RUM Resource is preserved; customers who also want to discard it can do so separately through RUM beforeSend..

Adds unit and E2E coverage, documentation, and playground controls for manual testing.

Test instructions

yarn test:unit
yarn typecheck
yarn playwright test -c e2e --project=e2e e2e/scenarios/trace-sampling-rules.scenario.ts
Playground: use Main Fetch — Keep Trace and Main Fetch — Drop Trace

Checklist

Tested locally (playground)
Added unit tests for this change.
Added e2e/integration tests for this change.
Updated related documentation.
Agentic code review findings addressed or explicitly dismissed.

rgaignault · 2026-08-14T12:37:56Z

@codex review

cursor

Stale comment

PR Review — Score: 4.8 / 5

This is a well-designed feature that wires ordered trace sampling rules through dd-trace, keeps RUM resources for dropped HTTP traces without linkage, and blocks propagation of rejected traces through Electron net — all with solid validation, unit tests, e2e coverage, and customer-facing docs. I would approve.

Why 4.8: Clear config surface (TraceSamplingRule), strict validation, correct end-to-end behavior (sampler → SpanProcessor → net propagation → RUM linkage), and an e2e scenario that exercises both drop and keep paths.

Why not 5: Rule wiring relies on dd-trace private _prioritySampler.configure with optional chaining, so a dd-trace internals change would silently skip customer rules rather than surfacing a misconfiguration. Rules also apply only after SDK init(), so spans between instrument import and init() still use dd-trace defaults.

Findings

[Minor] Silent sampler wiring — _prioritySampler?.configure optional chain means configured rules can be ignored without any observable failure.

[Nit] Pre-init sampling window — Traces started after @datadog/electron-sdk/instrument but before init() bypass traceSamplingRules.

Architectural flow
sequenceDiagram
    participant App as Customer app
    participant Init as SDK init
    participant Trace as Tracing
    participant DD as dd-trace sampler
    participant Net as net patch
    participant SP as SpanProcessor
    participant Intake as Transport

    App->>Init: init with traceSamplingRules
    Init->>Trace: new Tracing config
    Trace->>DD: prioritySampler.configure rules
    App->>Net: net.request or net.fetch
    Net->>DD: startSpan and inject
    DD-->>Net: carrier with sampling priority
    alt trace rejected
        Net->>Net: skip header propagation
    end
    DD->>SP: diagnostics channel export
    SP->>SP: isTraceSampled trace
    SP->>Intake: RUM resource for HTTP spans
    alt trace kept
        SP->>Intake: span envelope
    else trace dropped
        Note over SP: RUM resource without trace ids
    end
Loading
Before: Main-process traces were kept by default, always linked in RUM resources, and always propagated through outgoing HTTP headers.

After: Ordered traceSamplingRules configure dd-trace's priority sampler at SDK init. Rejected traces skip the spans intake and outgoing trace headers, but HTTP spans still produce unlinked RUM resources so request observability is preserved without APM cost.

_{Sent by Cursor Automation: electron-sdk reviews}

chatgpt-codex-connector · 2026-08-14T12:42:44Z

Codex Review: Didn't find any major issues. You're on a roll.

Reviewed commit: 5e2a1dc543

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

cursor

PR Review — Score: 4.8 / 5

This is a well-designed feature that wires ordered trace sampling rules through dd-trace, keeps RUM resources for dropped HTTP traces without linkage, and blocks propagation of rejected traces through Electron net — all with solid validation, unit tests, e2e coverage, and customer-facing docs. I would approve.

Why 4.8: Clear config surface (TraceSamplingRule), strict validation, correct end-to-end behavior (sampler → SpanProcessor → net propagation → RUM linkage), and an e2e scenario that exercises both drop and keep paths.

Why not 5: Rule wiring relies on dd-trace private _prioritySampler.configure with optional chaining, so a dd-trace internals change would silently skip customer rules rather than surfacing a misconfiguration.

Findings

[Minor] Silent sampler wiring — _prioritySampler?.configure optional chain means configured rules can be ignored without any observable failure.

Architectural flow

sequenceDiagram
    participant App as Customer app
    participant Init as SDK init
    participant Trace as Tracing
    participant DD as dd-trace sampler
    participant Net as net patch
    participant SP as SpanProcessor
    participant Intake as Transport

    App->>Init: init with traceSamplingRules
    Init->>Trace: new Tracing config
    Trace->>DD: prioritySampler.configure rules
    App->>Net: net.request or net.fetch
    Net->>DD: startSpan and inject
    DD-->>Net: carrier with sampling priority
    alt trace rejected
        Net->>Net: skip header propagation
    end
    DD->>SP: diagnostics channel export
    SP->>SP: isTraceSampled trace
    SP->>Intake: RUM resource for HTTP spans
    alt trace kept
        SP->>Intake: span envelope
    else trace dropped
        Note over SP: RUM resource without trace ids
    end

Before: Main-process traces were kept by default, always linked in RUM resources, and always propagated through outgoing HTTP headers.

After: Ordered traceSamplingRules configure dd-trace's priority sampler at SDK init. Rejected traces skip the spans intake and outgoing trace headers, but HTTP spans still produce unlinked RUM resources so request observability is preserved without APM cost.

_{Sent by Cursor Automation: electron-sdk reviews}

bcaudan · 2026-08-17T08:06:29Z

+   * determines the percentage of traces to keep. Traces that do not match a rule are kept.
+   * @example [{ tags: { 'http.url': '*health' }, sampleRate: 0 }]
+   */
+  traceSamplingRules?: TraceSamplingRule[];


💬 suggestion: ‏we should add something like useTraceSamplingRules to configuration telemetry

I created the PR here : DataDog/rum-events-format#430, I will do an update after the schema change.

bcaudan · 2026-08-17T08:35:46Z

+function shouldPropagateTrace(carrier: Record<string, string>): boolean {
+  const datadogPriority = Number(carrier['x-datadog-sampling-priority']);
+  if (Number.isFinite(datadogPriority) && datadogPriority <= 0) {
+    return false;
+  }
+  if (carrier['x-b3-sampled'] === '0' || carrier['x-b3-sampled'] === 'false') {
+    return false;
+  }
+  const b3SamplingState = carrier.b3?.split('-')[2] ?? carrier.b3;
+  if (b3SamplingState === '0') {
+    return false;
+  }
+  const traceParent = carrier.traceparent;
+  if (traceParent) {
+    const flags = Number.parseInt(traceParent.split('-')[3] ?? '', 16);
+    if (Number.isFinite(flags) && (flags & 1) === 0) {
+      return false;
+    }
+  }
+  return true;
+}


💬 suggestion: ‏is is copy/pasted from dd-trace? if so, it could be nice to add a comment around that.
Do we need to support all the b3 cases?

This is not copied from a single dd-trace helper. We inspect the carrier because there is no public API for reading the final sampling decision. B3 is not the default, but it is a supported injection style, so a B3-only setup still needs these checks. I’ll add a short comment explaining that.

bcaudan · 2026-08-18T08:25:30Z

+function toDdTraceSamplingRules(rules: TraceSamplingRule[], service: string): DdTraceSamplingRule[] {
+  return rules.flatMap(({ service: servicePattern, sampleRate, ...rule }) => {
+    if (servicePattern !== undefined && !matchesGlob(servicePattern, service)) {
+      return [];
+    }
+    return [{ ...rule, sampleRate: sampleRate / 100 }];
+  });
+}
+
+function matchesGlob(pattern: string, value: string): boolean {
+  const escaped = pattern
+    .replace(/[.+^${}()|[\]\\]/g, '\\$&')
+    .replace(/\*/g, '.*')
+    .replace(/\?/g, '.');
+  return new RegExp(`^${escaped}$`, 'i').test(value);
+}


All good for sample rate.
However for service, FMU, all the spans generated by the tracer will be for the configured service.
Then, should we just not allow to configure service in the TraceSamplingRule?

bcaudan · 2026-08-18T09:24:22Z

+      traceSamplingRules: [
+        { name: 'electron.main.handle', resource: 'mainNetRequest', sampleRate: 0 },
+        { name: 'electron.main.handle', resource: 'mainNetRequest', sampleRate: 100 },
+      ],


💬 suggestion: ‏in order to cover dd-trace behavior, what about adding:

name/resource only cases

tags matching case

rgaignault added 2 commits August 14, 2026 14:29

✨ add trace sampling rules

bb4ad6b

🧪 cover trace sampling rules

5e2a1dc

cursor Bot reviewed Aug 14, 2026

View reviewed changes

Comment thread src/domain/tracing/Tracing.ts

Comment thread src/domain/tracing/Tracing.ts

rgaignault marked this pull request as ready for review August 17, 2026 06:41

rgaignault requested a review from a team as a code owner August 17, 2026 06:41

cursor Bot reviewed Aug 17, 2026

View reviewed changes

Comment thread src/domain/tracing/Tracing.ts

sbarrio requested a review from bcaudan August 17, 2026 07:05

bcaudan reviewed Aug 17, 2026

View reviewed changes

rgaignault added 2 commits August 17, 2026 12:42

👌 address trace sampling feedback

f611da9

🧪 consolidate trace sampling coverage

c208823

bcaudan reviewed Aug 18, 2026

View reviewed changes

rgaignault added 2 commits August 18, 2026 14:23

♻️ refine trace sampling rules

386f654

📊 report trace sampling rules usage

9f2ff3a

rgaignault requested a review from bcaudan August 18, 2026 12:26

Uh oh!

Conversation

rgaignault commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Test instructions

Checklist

Uh oh!

rgaignault commented Aug 14, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

PR Review — Score: 4.8 / 5

Findings

Architectural flow

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Aug 14, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

PR Review — Score: 4.8 / 5

Findings

Architectural flow

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcaudan Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

rgaignault Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bcaudan Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

rgaignault Aug 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bcaudan Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bcaudan Aug 18, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rgaignault commented Aug 14, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading