Skip to content

refactor(flagd): migrate JSONLogic evaluator to datalogic-rs v5 via C FFI#105

Open
shankar-gpio wants to merge 1 commit into
open-feature:mainfrom
shankar-gpio:feat/migrate-flagd-to-datalogic-rs
Open

refactor(flagd): migrate JSONLogic evaluator to datalogic-rs v5 via C FFI#105
shankar-gpio wants to merge 1 commit into
open-feature:mainfrom
shankar-gpio:feat/migrate-flagd-to-datalogic-rs

Conversation

@shankar-gpio
Copy link
Copy Markdown

This PR

Replaces the hand-written JSONLogic engine and four flagd-specific operators (~1700 LOC across providers/flagd/src/evaluator/json_logic/, flagd_ops.{cpp,h}, and murmur_hash/) with FFI calls into the datalogic-rs v5 C ABI.

The flagd-specific operators (starts_with, ends_with, sem_ver, fractional) are now native in datalogic-rs's flagd Cargo feature, using the same MurmurHash3-x86-32 bucketing as the flagd reference implementation. JSONLogic spec conformance is now maintained upstream and shared with every other language binding (.NET, JVM, Python, Node, PHP, Go, WASM) so behaviour stays aligned across the OpenFeature ecosystem.

What changed

Added

  • providers/flagd/src/evaluator/datalogic_engine.{h,cpp} — thin RAII wrapper (~50 LOC) over the datalogic-rs C engine handle; exposes one Apply(rule, data) → absl::StatusOr<nlohmann::json> entry point that serializes via nlohmann::json, calls datalogic_engine_apply, and surfaces errors through datalogic's thread-local last-error state.
  • providers/flagd/third_party/datalogic/BUILD.bazel — wraps the per-platform staticlibs as a single cc_library :datalogic_c with select() over @platforms constraints.
  • providers/flagd/tests/evaluator/datalogic_engine_test.cpp — focused unit tests for the wrapper (var, starts_with, sem_ver, fractional stability, parse-error surfacing).

Modified

  • MODULE.bazel — adds bazel_dep platforms, declares http_archive for the four go-staticlib-<os>-<arch>.tar.gz artifacts from the v5.0.0 GitHub release (linux/darwin × amd64/arm64), SHA-pinned.
  • .bazelrc — gates the abseil -Wl,--gc-sections workaround (and -fno-asynchronous-unwind-tables) behind build:linux via --enable_platform_specific_config. macOS ld64 rejects --gc-sections.
  • providers/flagd/src/evaluator/evaluator.{h,cpp} — swaps the json_logic::JsonLogic member for DatalogicEngine; drops the four RegisterOperation calls (now native upstream).
  • providers/flagd/src/evaluator/BUILD, providers/flagd/tests/evaluator/BUILD — refresh deps.

Deleted (~1700 LOC)

  • providers/flagd/src/evaluator/json_logic/ (whole directory, 7 source files + BUILD)
  • providers/flagd/src/evaluator/flagd_ops.{cpp,h}
  • providers/flagd/src/evaluator/murmur_hash/ (whole directory, including the vendored MurmurHash3 from aappleby/smhasher)
  • providers/flagd/tests/evaluator/json_logic/ — JSONLogic spec tests, now maintained upstream in crates/datalogic-rs/tests/suites/
  • providers/flagd/tests/evaluator/flagd_ops_test.cpp, flagd_fractional_op_test.cpp — covered upstream in crates/datalogic-rs/tests/suites/flagd/

Design choices

  • One-shot evaluation (datalogic_engine_apply per call) over compile-cache + session. Simpler code; the C ABI explicitly exposes the string-in/string-out form because Rust-native types can't cross FFI. Rule-compile cache and arena-reusing Session can be added behind the same DatalogicEngine abstraction later if profiling motivates it.
  • Static linking via prebuilt libdatalogic_c.a per platform. Self-contained, no runtime LD_LIBRARY_PATH/rpath setup for downstream consumers.
  • Per-platform SHA pinning end-to-end — both the staticlib and the cbindgen header are pulled from the same SHA-pinned tarball, so a tampered artifact would fail Bazel's verification.
  • JSON-string FFI marshalling: the C ABI takes JSON strings (const char*) in and out. nlohmann::json::dump() / parse() round-trip is unavoidable across the boundary and matches the contract exactly.

Related Issues

Supersedes the in-house implementation from #65 and #92. Closes the maintenance burden of:

How to test

bazelisk test //providers/flagd/tests/...

All 7 flagd tests pass — including the unmodified 627-line evaluator_test, which is the end-to-end regression contract covering targeting rules, fractional bucketing, sem_ver gates, variant selection, and metadata enrichment:

//providers/flagd/tests:configuration_test                  PASSED
//providers/flagd/tests:provider_test                       PASSED
//providers/flagd/tests:sync_test                           PASSED
//providers/flagd/tests/evaluator:datalogic_engine_test     PASSED
//providers/flagd/tests/evaluator:evaluator_test            PASSED
//providers/flagd/tests/smoke:openfeature                   PASSED
//providers/flagd/tests/smoke:sync                          PASSED

Verified on darwin-arm64; CI will cover the other supported platforms.

Platform coverage

Currently pulling staticlibs for: linux-amd64, linux-arm64, darwin-amd64, darwin-arm64. Windows is published upstream (the v5.0.0 release ships go-staticlib-windows-{amd64,arm64}.tar.gz) and can be added to the Bazel select() if/when the SDK targets Windows.

@shankar-gpio shankar-gpio force-pushed the feat/migrate-flagd-to-datalogic-rs branch from f02a997 to 5169226 Compare May 15, 2026 04:13
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the custom C++ JsonLogic implementation with the datalogic-rs engine via a C FFI. The changes include removing the previous flagd_ops and json_logic source files, updating the Bazel build configuration to fetch platform-specific datalogic-rs binaries, and introducing the DatalogicEngine RAII wrapper. Feedback highlights a critical need to check for null pointers on the engine handle to prevent segmentation faults and suggests documenting the performance implications of serializing JSON to strings on every evaluation.

Comment thread providers/flagd/src/evaluator/datalogic_engine.cpp
Comment thread providers/flagd/src/evaluator/datalogic_engine.cpp
… FFI

Replaces the hand-written JSONLogic engine and four flagd-specific
operators (~1700 LOC across providers/flagd/src/evaluator/json_logic/,
flagd_ops.{cpp,h}, and murmur_hash/) with FFI calls into the
datalogic-rs v5 C ABI.

The flagd-specific operators (starts_with, ends_with, sem_ver,
fractional) are now native in datalogic-rs's `flagd` Cargo feature,
using the same MurmurHash3-x86-32 bucketing as the flagd reference
implementation. JSONLogic spec conformance is now maintained upstream
across all language bindings (.NET, JVM, Python, Node, PHP, Go, WASM,
C++).

Per-platform staticlibs are pulled in via http_archive against the
SHA-pinned v5.0.0 release tarballs (linux/darwin x amd64/arm64). The
thin DatalogicEngine C++ wrapper (~50 LOC) provides RAII over the C
engine handle and a single Apply(rule, data) entry point that
serializes via nlohmann::json and surfaces errors through datalogic's
thread-local last-error state.

The .bazelrc workaround for abseil discarded-section issues
(-Wl,--gc-sections) is gated behind build:linux since macOS ld64
rejects the flag.

How to test:
  bazelisk test //providers/flagd/tests/...

All 7 flagd tests pass, including the unmodified 627-line
evaluator_test which is the end-to-end regression contract covering
targeting rules, fractional bucketing, sem_ver gates, variant
selection, and metadata enrichment.

Signed-off-by: shankar-gpio <shankar@goplasmatic.io>
@shankar-gpio shankar-gpio force-pushed the feat/migrate-flagd-to-datalogic-rs branch from 5169226 to ddaf818 Compare May 15, 2026 04:24
@shankar-gpio
Copy link
Copy Markdown
Author

Thanks for the review! Addressed both points in the amended commit (ddaf818):

  • HIGH (null guard at datalogic_engine.cpp:31) — Added an engine_ == nullptr check at the top of Apply() that returns absl::InternalError. The upstream contract at bindings/c/include/datalogic.h:207 documents datalogic_engine_new as "Never returns NULL", so this is defensive against a future API change. One pointer compare on the hot path is cheap. The destructor already handles nullptr safely per datalogic.h:212.
  • MEDIUM (per-call dump()) — Documented the trade-off inline at the Apply() declaration in datalogic_engine.h, pointing to datalogic_engine_compile + datalogic_rule_evaluate as the compile-once-evaluate-many optimization we'd cache behind if profiling motivates it. Did not file a tracking issue yet — happy to open one if maintainers prefer that over an inline comment.

All 7 flagd tests still pass locally.

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