Skip to content

fix: align dependent flag failure semantics - #456

Open
typotter wants to merge 4 commits into
mainfrom
typo/dependent-flags-aaron-feedback
Open

typotter wants to merge 4 commits into
mainfrom
typo/dependent-flags-aaron-feedback

Conversation

@typotter

@typotter typotter commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Applies the evaluator findings synthesized from Aaron’s dependent-flags reviews across the Java, .NET, Go, libdatadog, Python, PHP, Ruby, and JavaScript workstreams.

  • tolerate unknown future fields in JSON flagEvaluation objects while continuing to require a non-empty key
  • keep prerequisite-specific error codes on prerequisite evaluation events while returning GENERAL for an existing root flag whose graph fails
  • include the failing prerequisite key in root error messages
  • treat malformed evaluatorParams blocks and invalid or wrongly typed maxDependencyDepth values as absent, log an error, and use the hard-coded depth of 10
  • keep malformed evaluator options from producing evaluation errors or dropping otherwise valid precomputed assignments
  • align precomputed-assignment tests with the root-versus-prerequisite error-code distinction

Scope

This PR is limited to evaluator parsing, failure semantics, and regressions. Capability negotiation, tracer integration, release pins, and system-test activation remain in their owning workstreams.

Validation

  • corepack yarn build
  • corepack yarn typecheck
  • corepack yarn lint
  • corepack yarn format:check
  • corepack yarn test:build — 126 passed
  • corepack yarn test
    • core: 1,323 passed, 6 skipped
    • Node: 737 passed
    • browser: 648 passed
  • focused dependent-flags and precompute suites: 118 passed
  • git diff --check

Apply cross-SDK evaluator review findings for forward-compatible parsing and root error propagation.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

OpenFeature Browser Provider Bundle Sizes

Measured from the Vite production output after installing packed @datadog/flagging-core and @datadog/openfeature-browser tarballs.

Both scenarios initialize an OpenFeature provider, evaluate a boolean flag, change context, and evaluate again. Telemetry is disabled for DatadogProvider; no tracking hooks are registered for DatadogCoreProvider. DatadogProvider fetches precomputed assignments for each context; DatadogCoreProvider receives rules from fetchRulesConfiguration once and evaluates locally.

Sizes include OpenFeature and the same small scenario harness. Configuration responses are supplied by Playwright and are not bundled. These are complete scenario JS sizes, not configuration payload sizes or isolated provider/Protobuf costs; the difference between rows is not a decoder-only delta.

Scenario HTML JS Assets Raw JS Gzip JS Protobuf Markers
DatadogProvider (precomputed fetching) provider.html 15 107.5 KiB 38.7 KiB no
DatadogCoreProvider + fetchRulesConfiguration core-provider.html 15 207.7 KiB 66.4 KiB yes

Dependency checks passed: no Protobuf markers in default/precomputed scenarios; markers present in rules-based scenarios.

OpenFeature Browser Tracking Hook Bundle Sizes

Synthetic entrypoints import and call tracking hook factories from the packed @datadog/openfeature-browser/rules-based ESM package. Deltas are measured against the no-hook baseline from the same Vite production build.

Scenario HTML JS Assets Raw JS Raw Δ Gzip JS Gzip Δ
tracking hook baseline tracking-baseline.html 5 2.7 KiB 0 B 1.5 KiB 0 B
exposure logging hook tracking-exposure.html 10 71.3 KiB +68.6 KiB 26.4 KiB +24.9 KiB
evaluation logging hook tracking-evaluation.html 11 67.1 KiB +64.4 KiB 25.0 KiB +23.5 KiB
RUM tracking hook tracking-rum.html 8 6.8 KiB +4.1 KiB 3.5 KiB +2.0 KiB
all tracking hooks tracking-all.html 13 76.1 KiB +73.4 KiB 28.6 KiB +27.1 KiB

OpenFeature Node Server Bundle Sizes

Independent Webpack production builds of the installed core and Node SDK tarballs, targeting Node 18 with ESM (module export condition). Node built-ins remain external. OpenFeature, dd-trace, and configuration fixtures are not bundled.

Scenario Raw JS Gzip JS Protobuf Modules
Node provider only 57.2 KiB 17.1 KiB no
SSR helpers only (fetch client rules + serialize) 84.8 KiB 24.9 KiB yes
Node provider + SSR helpers 140.2 KiB 40.7 KiB yes
Precompute only (parsed rules) 36.1 KiB 11.8 KiB no
Node provider + precompute 60.4 KiB 18.0 KiB no

Adding SSR helpers to the provider adds 83.1 KiB raw / 23.6 KiB gzip in this build. This includes fetching, parsing, and serialization, not just the decoder. SSR helpers fetch rules for the browser, not for configuring that provider (which uses its existing JSON API).

Adding precomputation to the provider adds 3.2 KiB raw / 1.0 KiB gzip in this build. Both precompute scenarios import precomputeConfiguration from /rules-based and receive already-parsed rules from the test harness; they do not fetch, parse, or serialize configuration.

These are bundler sizes, not npm install sizes, unbundled Node require/import costs, payload sizes, or latency. Gzip compares compressed artifacts. Deltas compare scenarios in this build, not changes against the base branch. The browser scenarios include OpenFeature and a different harness, so their sizes are not directly comparable.

Dependency checks passed: no Protobuf runtime or generated schema modules in the provider-only or either precompute bundle; present in both SSR bundles (checked using Webpack module statistics).

This report shows current PR artifact sizes only; it does not compare against the base branch.

Keep malformed evaluator parameters from failing configuration evaluation and log the depth fallback.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
@aarsilv

aarsilv commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

🤖 Comment from Claude working with Aaron Silverman:

Heads up: this PR duplicates #453 (same author, same change). #453 contains everything in this PR's two commits, plus 12fe487 ("validate dependency depth representations"). The only source difference is that #453 converts a bigint depth only for protobuf configurations (packages/core/src/evaluation/evaluation.ts:882-883); #453 also has more tests. #453 carries the review threads, and #458 is stacked on its branch.

Suggest closing this PR in favor of #453. Nothing here needs to move.

typotter and others added 2 commits November 5, 2026 22:37
Preserve JSON and protobuf depth representation semantics and assert prerequisite error messages.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
Normalize graph errors, tolerate malformed evaluator options, and preserve dependency telemetry ordering.

Environment: Datadog workspace

Co-Authored-By: Codex GPT-5 <noreply@localhost>
@typotter
typotter marked this pull request as ready for review November 6, 2026 15:29
@typotter
typotter requested a review from a team as a code owner November 6, 2026 15:29

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.

2 participants