Skip to content

fix(analytics): allow capability to offload reportExposure to async thread (SDK-80)#157

Merged
tylerjroach merged 5 commits into
masterfrom
feat/async-exposure-executor
Jul 22, 2026
Merged

fix(analytics): allow capability to offload reportExposure to async thread (SDK-80)#157
tylerjroach merged 5 commits into
masterfrom
feat/async-exposure-executor

Conversation

@tylerjroach

@tylerjroach tylerjroach commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Exposure tracking called the tracker_callback — and therefore an HTTP POST — inline, blocking every get_variant / get_variant_value / is_enabled call by the full /track round trip.

Add an optional :exposure_executor config key on both LocalFlagsProvider and RemoteFlagsProvider. The executor is duck-typed — anything that responds to #post(&block) works:

  • Concurrent::ExecutorService from concurrent-ruby
  • A 5-line Thread.new wrapper for users who don't want the dependency

When set, the tracker call is dispatched on the executor so flag evaluation returns as soon as the local logic finishes. Defaults to nil — preserves the existing inline behavior, no breaking change for current users.

Usage

# With concurrent-ruby
require 'concurrent'
executor = Concurrent::FixedThreadPool.new(1)
tracker = Mixpanel::Tracker.new(token, error_handler,
  local_flags_config: { exposure_executor: executor })

# Without concurrent-ruby
class ThreadPerCall
  def post(&block) = Thread.new(&block)
end
executor = ThreadPerCall.new

Context

Linear: SDK-80. Mirrors mixpanel-java#85. Audit-driven; same fix being applied to mixpanel-python and mixpanel-go in parallel PRs.

Test plan

  • Full flags spec suite passes (70 examples, including the existing inline-exposure specs — :exposure_executor defaults to nil so the old behavior is unchanged)
  • New spec asserts the tracker runs on a thread other than the calling thread when :exposure_executor is configured

…hread

Exposure tracking called the tracker_callback (and therefore an HTTP
POST) inline, blocking every get_variant / get_variant_value /
is_enabled call by the full /track round trip.

Add an optional :exposure_executor config key on both local and
remote flags configs. The executor is duck-typed — anything that
responds to #post(&block) works (Concurrent::ExecutorService, or
a Thread.new wrapper). When set, the tracker call is dispatched
on the executor so flag evaluation returns as soon as the local
logic finishes. Defaults to nil — preserves existing inline behavior.

Mirrors mixpanel-java#85.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@tylerjroach
tylerjroach requested review from a team and ketanmixpanel June 30, 2026 17:16
@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.57143% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.16%. Comparing base (76d0695) to head (9bfef62).

Files with missing lines Patch % Lines
lib/mixpanel-ruby/flags/flags_provider.rb 78.57% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #157      +/-   ##
==========================================
- Coverage   97.25%   97.16%   -0.10%     
==========================================
  Files          15       15              
  Lines         764      775      +11     
==========================================
+ Hits          743      753      +10     
- Misses         21       22       +1     
Flag Coverage Δ
openfeature 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…rage

- Add an "Async Exposure Tracking" section to the openfeature-provider
  README showing both the concurrent-ruby and Thread.new wrapper
  patterns.
- Add specs covering the local provider with executor (previously only
  the remote provider had coverage), the manual track_exposure_event
  path, and explicit "default is inline" sanity checks on both providers.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@tylerjroach tylerjroach changed the title fix(analytics): allow capability to offload reportExposure to async thread fix(analytics): allow capability to offload reportExposure to async thread (SDK-80) Jun 30, 2026
@linear-code

linear-code Bot commented Jun 30, 2026

Copy link
Copy Markdown

SDK-80

@tylerjroach

Copy link
Copy Markdown
Contributor Author

@greptileai

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

Safe to merge — the change is additive, defaults to nil (existing inline behavior), and is well-tested.

The executor path is correctly isolated: the block-level rescue catches non-MixpanelError exceptions from the background thread and routes them to the error handler rather than silently killing the thread. The inline path is unchanged. All four spec scenarios (inline, async dispatch, async error reporting, inline propagation) are covered with bounded waits.

No files require special attention.

Important Files Changed

Filename Overview
lib/mixpanel-ruby/flags/flags_provider.rb Core change: adds dispatch_exposure / invoke_tracker split with correct sync/async error routing. Block-level rescue syntax is valid Ruby 3.0+.
lib/mixpanel-ruby/flags/local_flags_provider.rb Adds exposure_executor: nil to DEFAULT_CONFIG and passes it through to FlagsProvider. Straightforward.
lib/mixpanel-ruby/flags/remote_flags_provider.rb Same exposure_executor wiring as local provider. Clean.
spec/mixpanel-ruby/flags/local_flags_spec.rb Four new specs for executor path; uses Timeout.timeout(2) to bound Queue#pop waits. Covers inline, async, error-handler, and propagation cases.
spec/mixpanel-ruby/flags/remote_flags_spec.rb Two new specs mirroring the local inline/async tests; adds require 'timeout'. Clean.
openfeature-provider/README.md Documents async exposure tracking usage including executor lifecycle warning. Ruby 3.0+ one-liner syntax is consistent with gem's required_ruby_version.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant FlagsProvider
    participant Executor
    participant Tracker

    Caller->>FlagsProvider: get_variant / is_enabled
    FlagsProvider->>FlagsProvider: track_exposure_event
    FlagsProvider->>FlagsProvider: dispatch_exposure

    alt exposure_executor nil (default)
        FlagsProvider->>Tracker: invoke_tracker (inline)
        Tracker-->>FlagsProvider: result / MixpanelError → error_handler
        FlagsProvider-->>Caller: returns
    else exposure_executor configured
        FlagsProvider->>Executor: "post { invoke_tracker }"
        FlagsProvider-->>Caller: returns immediately
        Executor->>Tracker: invoke_tracker (async thread)
        alt MixpanelError
            Tracker-->>Executor: error_handler.handle(e)
        else StandardError
            Executor-->>Executor: wrap in MixpanelError → error_handler
        end
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Caller
    participant FlagsProvider
    participant Executor
    participant Tracker

    Caller->>FlagsProvider: get_variant / is_enabled
    FlagsProvider->>FlagsProvider: track_exposure_event
    FlagsProvider->>FlagsProvider: dispatch_exposure

    alt exposure_executor nil (default)
        FlagsProvider->>Tracker: invoke_tracker (inline)
        Tracker-->>FlagsProvider: result / MixpanelError → error_handler
        FlagsProvider-->>Caller: returns
    else exposure_executor configured
        FlagsProvider->>Executor: "post { invoke_tracker }"
        FlagsProvider-->>Caller: returns immediately
        Executor->>Tracker: invoke_tracker (async thread)
        alt MixpanelError
            Tracker-->>Executor: error_handler.handle(e)
        else StandardError
            Executor-->>Executor: wrap in MixpanelError → error_handler
        end
    end
Loading

Reviews (3): Last reviewed commit: "fix(flags): keep inline path non-swallow..." | Re-trigger Greptile

Comment thread spec/mixpanel-ruby/flags/local_flags_spec.rb Outdated
Comment thread spec/mixpanel-ruby/flags/remote_flags_spec.rb
Comment thread lib/mixpanel-ruby/flags/flags_provider.rb
…#pop

invoke_tracker only rescued MixpanelError. On the async path any other
exception (RuntimeError, NoMethodError, unwrapped network errors)
terminated the executor thread silently — the error_handler never saw
it. Added a StandardError branch that wraps into MixpanelError so
consumers see a consistent type.

Also bound the two async-executor specs with Timeout.timeout(2) around
tracker_ran.pop. If a future change makes the tracker block raise
before pushing :done, the spec now fails with a clear timeout instead
of hanging CI forever.

New spec 'reports non-MixpanelError exceptions from the async tracker'
locks in the widened rescue.
@tylerjroach

Copy link
Copy Markdown
Contributor Author

Pushed 1ea39be addressing all three Greptile threads.

P1 — blocking Queue#pop in both specs (local_flags_spec.rb:764, remote_flags_spec.rb:158): wrapped both tracker_ran.pop calls in Timeout.timeout(2) so a future change that makes the tracker block raise before pushing :done fails the spec with a clear timeout instead of hanging CI forever.

P2 — invoke_tracker only rescues MixpanelError (flags_provider.rb:132): added a rescue StandardError branch that wraps the underlying error into MixpanelError (preserving class + message) and dispatches to @error_handler. Otherwise RuntimeError / NoMethodError / unwrapped network errors raised on the executor thread would terminate it silently. New spec reports non-MixpanelError exceptions from the async tracker to error_handler covers this.

All 88 flag specs pass locally.

Comment thread openfeature-provider/README.md
@ketanmixpanel
ketanmixpanel requested a review from Copilot July 21, 2026 09:35
ketanmixpanel
ketanmixpanel previously approved these changes Jul 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds optional async dispatch for feature-flag exposure tracking so get_variant* / is_enabled? don’t block on the /track HTTP round trip unless desired, by introducing a duck-typed :exposure_executor configuration.

Changes:

  • Introduces :exposure_executor config plumbing for both LocalFlagsProvider and RemoteFlagsProvider, and dispatches exposure tracking via #post(&block) when configured.
  • Extends exposure tracking implementation to route through a dedicated dispatch helper and adds handling for async execution failures.
  • Adds specs covering default inline behavior vs executor-backed async behavior, plus documentation for OpenFeature users.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/mixpanel-ruby/flags/flags_provider.rb Implements inline-vs-executor exposure dispatch and centralizes tracker invocation/error handling.
lib/mixpanel-ruby/flags/local_flags_provider.rb Adds :exposure_executor to default config and passes it into the base provider config.
lib/mixpanel-ruby/flags/remote_flags_provider.rb Adds :exposure_executor to default config and passes it into the base provider config.
spec/mixpanel-ruby/flags/local_flags_spec.rb Adds tests for inline vs async exposure dispatch and async error reporting behavior.
spec/mixpanel-ruby/flags/remote_flags_spec.rb Adds tests for inline vs async exposure dispatch in the remote provider.
openfeature-provider/README.md Documents how to configure async exposure tracking for OpenFeature provider usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/mixpanel-ruby/flags/flags_provider.rb
Comment thread openfeature-provider/README.md
…-executor

# Conflicts:
#	lib/mixpanel-ruby/flags/flags_provider.rb
#	lib/mixpanel-ruby/flags/local_flags_provider.rb
#	lib/mixpanel-ruby/flags/remote_flags_provider.rb
#	spec/mixpanel-ruby/flags/local_flags_spec.rb
…ycle

Copilot flagged two things on the merge into #157:

1. invoke_tracker had grown a rescue StandardError branch to keep the
   async executor thread from dying silently, but that same rescue
   was catching non-MixpanelError exceptions on the inline path too.
   Previously the inline path let those propagate to the flag
   evaluator's caller — the widened rescue changed that silently.
   Moved the StandardError trap into the .post block of
   dispatch_exposure so it only applies on the async path. invoke_tracker
   is back to rescuing MixpanelError only, matching the pre-SDK-80
   inline behavior. New spec 'lets non-MixpanelError exceptions
   propagate on the inline path' pins this.

2. The Async Exposure Tracking README section created a
   Concurrent::FixedThreadPool but never told callers to shut it
   down — provider.shutdown only tears down polling, not a
   user-supplied executor. Added a lifecycle note pointing at
   at_exit + shutdown/wait_for_termination.

All 108 flag specs pass.
@tylerjroach

Copy link
Copy Markdown
Contributor Author

Pushed `9bfef62` addressing the two Copilot threads.

Thread 2 — invoke_tracker widened rescue changed inline behavior: moved the rescue StandardError into the .post do ... end block of dispatch_exposure so it only fires on the async path. invoke_tracker is back to rescue MixpanelError only, so non-MixpanelError from the tracker propagates to the flag evaluator's caller on the inline path exactly as it did pre-SDK-80. New spec lets non-MixpanelError exceptions propagate on the inline path pins this: raises NoMethodError, asserts error_handler.handle is never called, and asserts the exception propagates.

Thread 3 — executor lifecycle undocumented: added a > Executor lifecycle block-quote note under the Async Exposure Tracking section explaining that provider.shutdown only tears down polling and pointing callers at at_exit { exposure_executor.shutdown; exposure_executor.wait_for_termination(5) } for Concurrent::FixedThreadPool.

All 108 flag specs pass locally.

@tylerjroach
tylerjroach merged commit a125e26 into master Jul 22, 2026
14 of 16 checks passed
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.

3 participants