diff --git a/ruby-sdk/Cargo.lock b/ruby-sdk/Cargo.lock index 14185a96..ed21fcbc 100644 --- a/ruby-sdk/Cargo.lock +++ b/ruby-sdk/Cargo.lock @@ -304,7 +304,7 @@ dependencies = [ [[package]] name = "eppo_client" -version = "3.1.2" +version = "3.2.0" dependencies = [ "env_logger", "eppo_core", @@ -312,6 +312,7 @@ dependencies = [ "magnus", "rb-sys", "serde", + "serde_json", "serde_magnus", ] @@ -1295,9 +1296,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.124" +version = "1.0.128" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "66ad62847a56b3dba58cc891acd13884b9c61138d330c0d7b6181713d4fce38d" +checksum = "6ff5456707a1de34e7e37f2a6fd3d3f808c318259cbd01ab6377795054b483d8" dependencies = [ "itoa", "memchr", diff --git a/ruby-sdk/Gemfile.lock b/ruby-sdk/Gemfile.lock index 7ba6f5c2..8f495797 100644 --- a/ruby-sdk/Gemfile.lock +++ b/ruby-sdk/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - eppo-server-sdk (3.1.1) + eppo-server-sdk (3.2.0) GEM remote: https://rubygems.org/ diff --git a/ruby-sdk/ext/eppo_client/Cargo.toml b/ruby-sdk/ext/eppo_client/Cargo.toml index 8cf2a1b3..dfe96c39 100644 --- a/ruby-sdk/ext/eppo_client/Cargo.toml +++ b/ruby-sdk/ext/eppo_client/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "eppo_client" # TODO: this version and lib/eppo_client/version.rb should be in sync -version = "3.1.2" +version = "3.2.0" edition = "2021" license = "MIT" publish = false @@ -14,7 +14,8 @@ crate-type = ["cdylib"] env_logger = { version = "0.11.3", features = ["unstable-kv"] } eppo_core = { version = "4.0.0" } log = { version = "0.4.21", features = ["kv_serde"] } -magnus = { version = "0.6.2" } +magnus = { version = "0.6.4" } serde = { version = "1.0.203", features = ["derive"] } serde_magnus = "0.8.1" rb-sys = "0.9" +serde_json = "1.0.128" diff --git a/ruby-sdk/ext/eppo_client/src/client.rs b/ruby-sdk/ext/eppo_client/src/client.rs index 5a535779..8521f4e1 100644 --- a/ruby-sdk/ext/eppo_client/src/client.rs +++ b/ruby-sdk/ext/eppo_client/src/client.rs @@ -1,20 +1,24 @@ -use std::{cell::RefCell, sync::Arc}; +use std::{cell::RefCell, sync::Arc, time::Duration}; use eppo_core::{ configuration_fetcher::{ConfigurationFetcher, ConfigurationFetcherConfig}, configuration_store::ConfigurationStore, eval::{Evaluator, EvaluatorConfig}, - poller_thread::PollerThread, + poller_thread::{PollerThread, PollerThreadConfig}, ufc::VariationType, - Attributes, ContextAttributes, SdkMetadata, + Attributes, ContextAttributes, }; use magnus::{error::Result, exception, prelude::*, Error, TryConvert, Value}; +use crate::{configuration::Configuration, SDK_METADATA}; + #[derive(Debug)] #[magnus::wrap(class = "EppoClient::Core::Config", size, free_immediately)] pub struct Config { api_key: String, base_url: String, + poll_interval: Option, + poll_jitter: Duration, } impl TryConvert for Config { @@ -22,12 +26,21 @@ impl TryConvert for Config { fn try_convert(val: magnus::Value) -> Result { let api_key = String::try_convert(val.funcall("api_key", ())?)?; let base_url = String::try_convert(val.funcall("base_url", ())?)?; - Ok(Config { api_key, base_url }) + let poll_interval_seconds = + Option::::try_convert(val.funcall("poll_interval_seconds", ())?)?; + let poll_jitter_seconds = u64::try_convert(val.funcall("poll_jitter_seconds", ())?)?; + Ok(Config { + api_key, + base_url, + poll_interval: poll_interval_seconds.map(Duration::from_secs), + poll_jitter: Duration::from_secs(poll_jitter_seconds), + }) } } #[magnus::wrap(class = "EppoClient::Core::Client")] pub struct Client { + configuration_store: Arc, evaluator: Evaluator, // Magnus only allows sharing aliased references (&T) through the API, so we need to use RefCell // to get interior mutability. @@ -41,29 +54,35 @@ impl Client { pub fn new(config: Config) -> Client { let configuration_store = Arc::new(ConfigurationStore::new()); - let sdk_metadata = SdkMetadata { - name: "ruby", - version: env!("CARGO_PKG_VERSION"), + let poller_thread = if let Some(poll_interval) = config.poll_interval { + Some( + PollerThread::start_with_config( + ConfigurationFetcher::new(ConfigurationFetcherConfig { + base_url: config.base_url, + api_key: config.api_key, + sdk_metadata: SDK_METADATA, + }), + configuration_store.clone(), + PollerThreadConfig { + interval: poll_interval, + jitter: config.poll_jitter, + }, + ) + .expect("should be able to start poller thread"), + ) + } else { + None }; - let poller_thread = PollerThread::start( - ConfigurationFetcher::new(ConfigurationFetcherConfig { - base_url: config.base_url, - api_key: config.api_key, - sdk_metadata: sdk_metadata.clone(), - }), - configuration_store.clone(), - ) - .expect("should be able to start poller thread"); - let evaluator = Evaluator::new(EvaluatorConfig { - configuration_store, - sdk_metadata, + configuration_store: configuration_store.clone(), + sdk_metadata: SDK_METADATA, }); Client { + configuration_store, evaluator, - poller_thread: RefCell::new(Some(poller_thread)), + poller_thread: RefCell::new(poller_thread), } } @@ -171,6 +190,17 @@ impl Client { serde_magnus::serialize(&result) } + pub fn get_configuration(&self) -> Option { + self.configuration_store + .get_configuration() + .map(|it| it.into()) + } + + pub fn set_configuration(&self, configuration: &Configuration) { + self.configuration_store + .set_configuration(configuration.clone().into()) + } + pub fn shutdown(&self) { if let Some(t) = self.poller_thread.take() { let _ = t.shutdown(); diff --git a/ruby-sdk/ext/eppo_client/src/configuration.rs b/ruby-sdk/ext/eppo_client/src/configuration.rs new file mode 100644 index 00000000..afcc81b4 --- /dev/null +++ b/ruby-sdk/ext/eppo_client/src/configuration.rs @@ -0,0 +1,113 @@ +use std::sync::Arc; + +use magnus::{function, method, prelude::*, scan_args::get_kwargs, Error, RHash, RString, Ruby}; + +use eppo_core::{ufc::UniversalFlagConfig, Configuration as CoreConfiguration}; + +use crate::{gc_lock::GcLock, SDK_METADATA}; + +pub(crate) fn init(ruby: &Ruby) -> Result<(), Error> { + let eppo_client = ruby.define_module("EppoClient")?; + + let configuration = eppo_client.define_class("Configuration", magnus::class::object())?; + configuration.define_singleton_method("new", function!(Configuration::new, 1))?; + configuration.define_method( + "flags_configuration", + method!(Configuration::flags_configuration, 0), + )?; + configuration.define_method( + "bandits_configuration", + method!(Configuration::bandits_configuration, 0), + )?; + + Ok(()) +} + +#[derive(Debug, Clone)] +#[magnus::wrap(class = "EppoClient::Configuration", free_immediately)] +pub struct Configuration { + inner: Arc, +} + +impl Configuration { + fn new(ruby: &Ruby, kw: RHash) -> Result { + let args = get_kwargs(kw, &["flags_configuration"], &["bandits_configuration"])?; + let (flags_configuration,): (RString,) = args.required; + let (bandits_configuration,): (Option>,) = args.optional; + let rest: RHash = args.splat; + if !rest.is_empty() { + return Err(Error::new( + ruby.exception_arg_error(), + format!("unexpected keyword arguments: {:?}", rest), + )); + } + + let inner = { + let _gc_lock = GcLock::new(ruby); + + Arc::new(CoreConfiguration::from_server_response( + UniversalFlagConfig::from_json( + SDK_METADATA, + unsafe { + // SAFETY: we have disabled GC, so the memory can't be modified concurrently. + flags_configuration.as_slice() + } + .to_vec(), + ) + .map_err(|err| { + Error::new( + ruby.exception_arg_error(), + format!("failed to parse flags_configuration: {err:?}"), + ) + })?, + bandits_configuration + .flatten() + .map(|bandits| { + serde_json::from_slice(unsafe { + // SAFETY: we have disabled GC, so the memory can't be modified concurrently. + bandits.as_slice() + }) + }) + .transpose() + .map_err(|err| { + Error::new( + ruby.exception_arg_error(), + format!("failed to parse bandits_configuration: {err:?}"), + ) + })?, + )) + }; + + Ok(Configuration { inner }) + } + + fn flags_configuration(ruby: &Ruby, rb_self: &Self) -> Result { + Ok(ruby.str_from_slice(rb_self.inner.flags.to_json())) + } + + fn bandits_configuration(ruby: &Ruby, rb_self: &Self) -> Result, Error> { + let Some(bandits) = &rb_self.inner.bandits else { + return Ok(None) + }; + let vec = serde_json::to_vec(bandits).map_err(|err| { + // this should never happen + Error::new( + ruby.exception_runtime_error(), + format!("failed to serialize bandits configuration: {err:?}"), + ) + })?; + Ok(Some(ruby.str_from_slice(&vec))) + } +} + +impl From> for Configuration { + fn from(inner: Arc) -> Configuration { + Configuration { inner } + } +} + +impl From for Arc { + fn from(value: Configuration) -> Arc { + value.inner + } +} diff --git a/ruby-sdk/ext/eppo_client/src/gc_lock.rs b/ruby-sdk/ext/eppo_client/src/gc_lock.rs new file mode 100644 index 00000000..2949c194 --- /dev/null +++ b/ruby-sdk/ext/eppo_client/src/gc_lock.rs @@ -0,0 +1,25 @@ +use magnus::Ruby; + +pub struct GcLock<'a> { + ruby: &'a Ruby, + /// Holds `true` if GC was already disabled before acquiring the lock (so it doesn't need to be + /// re-enabled). + gc_was_disabled: bool, +} + +impl<'a> GcLock<'a> { + pub fn new(ruby: &'a Ruby) -> GcLock<'a> { + GcLock { + ruby, + gc_was_disabled: ruby.gc_disable(), + } + } +} + +impl<'a> Drop for GcLock<'a> { + fn drop(&mut self) { + if !self.gc_was_disabled { + self.ruby.gc_enable(); + } + } +} diff --git a/ruby-sdk/ext/eppo_client/src/lib.rs b/ruby-sdk/ext/eppo_client/src/lib.rs index 8974a09c..a0a88f69 100644 --- a/ruby-sdk/ext/eppo_client/src/lib.rs +++ b/ruby-sdk/ext/eppo_client/src/lib.rs @@ -1,12 +1,20 @@ mod client; +mod configuration; +mod gc_lock; +use eppo_core::SdkMetadata; use magnus::{function, method, prelude::*, Error, Object, Ruby}; use crate::client::Client; +pub(crate) const SDK_METADATA: SdkMetadata = SdkMetadata { + name: "ruby", + version: env!("CARGO_PKG_VERSION"), +}; + #[magnus::init] fn init(ruby: &Ruby) -> Result<(), Error> { - env_logger::Builder::from_env(env_logger::Env::new().default_filter_or("eppo")).init(); + env_logger::Builder::from_env(env_logger::Env::new().default_filter_or("eppo=debug")).init(); let eppo_client = ruby.define_module("EppoClient")?; let core = eppo_client.define_module("Core")?; @@ -23,12 +31,24 @@ fn init(ruby: &Ruby) -> Result<(), Error> { "get_bandit_action_details", method!(Client::get_bandit_action_details, 5), )?; + core_client.define_method("configuration", method!(Client::get_configuration, 0))?; + core_client.define_method("configuration=", method!(Client::set_configuration, 1))?; core_client.define_method("shutdown", method!(Client::shutdown, 0))?; core.const_set( "DEFAULT_BASE_URL", eppo_core::configuration_fetcher::DEFAULT_BASE_URL, )?; + core.const_set( + "DEFAULT_POLL_INTERVAL_SECONDS", + eppo_core::poller_thread::PollerThreadConfig::DEFAULT_POLL_INTERVAL.as_secs(), + )?; + core.const_set( + "DEFAULT_POLL_JITTER_SECONDS", + eppo_core::poller_thread::PollerThreadConfig::DEFAULT_POLL_JITTER.as_secs(), + )?; + + configuration::init(ruby)?; Ok(()) } diff --git a/ruby-sdk/lib/eppo_client/client.rb b/ruby-sdk/lib/eppo_client/client.rb index 859ea058..fd453b1f 100644 --- a/ruby-sdk/lib/eppo_client/client.rb +++ b/ruby-sdk/lib/eppo_client/client.rb @@ -24,6 +24,14 @@ def init(config) @core = EppoClient::Core::Client.new(config) end + def configuration + @core.configuration + end + + def configuration=(configuration) + @core.configuration = configuration + end + def shutdown @core.shutdown end diff --git a/ruby-sdk/lib/eppo_client/config.rb b/ruby-sdk/lib/eppo_client/config.rb index ebb172a6..6ee8dca5 100644 --- a/ruby-sdk/lib/eppo_client/config.rb +++ b/ruby-sdk/lib/eppo_client/config.rb @@ -6,12 +6,14 @@ module EppoClient # The class for configuring the Eppo client singleton class Config - attr_reader :api_key, :assignment_logger, :base_url + attr_reader :api_key, :assignment_logger, :base_url, :poll_interval_seconds, :poll_jitter_seconds - def initialize(api_key, assignment_logger: AssignmentLogger.new, base_url: EppoClient::Core::DEFAULT_BASE_URL) + def initialize(api_key, assignment_logger: AssignmentLogger.new, base_url: EppoClient::Core::DEFAULT_BASE_URL, poll_interval_seconds: EppoClient::Core::DEFAULT_POLL_INTERVAL_SECONDS, poll_jitter_seconds: EppoClient::Core::DEFAULT_POLL_JITTER_SECONDS, initial_configuration: nil) @api_key = api_key @assignment_logger = assignment_logger @base_url = base_url + @poll_interval_seconds = poll_interval_seconds + @poll_jitter_seconds = poll_jitter_seconds end def validate diff --git a/ruby-sdk/lib/eppo_client/version.rb b/ruby-sdk/lib/eppo_client/version.rb index eaaf60c6..bcee6d39 100644 --- a/ruby-sdk/lib/eppo_client/version.rb +++ b/ruby-sdk/lib/eppo_client/version.rb @@ -2,5 +2,5 @@ # TODO: this version and ext/eppo_client/Cargo.toml should be in sync module EppoClient - VERSION = "3.1.2" + VERSION = "3.2.0" end diff --git a/ruby-sdk/spec/configuration_spec.rb b/ruby-sdk/spec/configuration_spec.rb new file mode 100644 index 00000000..eed6a557 --- /dev/null +++ b/ruby-sdk/spec/configuration_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +# require 'eppo_client' + +RSpec.describe EppoClient::Configuration do + flags_config = File.read "../sdk-test-data/ufc/flags-v1.json" + + describe "new()" do + it "initializes from flags_configuration" do + EppoClient::Configuration.new(flags_configuration: flags_config) + end + + it "initializes from flags_configuration and bandits_configuration" do + flags_config = File.read "../sdk-test-data/ufc/bandit-flags-v1.json" + bandits_config = File.read "../sdk-test-data/ufc/bandit-models-v1.json" + EppoClient::Configuration.new(flags_configuration: flags_config, bandits_configuration: bandits_config) + end + + it "requires flags_configuration to be a keyword" do + expect { + EppoClient::Configuration.new(flags_config) + }.to raise_error TypeError + end + + it "throws on parse error" do + expect { + EppoClient::Configuration.new(flags_configuration: '{"invalid": "configuration"}') + }.to raise_error ArgumentError + end + + it "accepts explicit nil as bandits_configuration" do + EppoClient::Configuration.new(flags_configuration: flags_config, bandits_configuration: nil) + end + end + + describe "flags_configuration()" do + it "returns configuration" do + configuration = EppoClient::Configuration.new(flags_configuration: flags_config) + + flags = configuration.flags_configuration + + expect(flags).to be_a String + end + end + + describe "bandits_configuration()" do + it "returns configuration" do + flags_config = File.read "../sdk-test-data/ufc/bandit-flags-v1.json" + bandits_config = File.read "../sdk-test-data/ufc/bandit-models-v1.json" + configuration = EppoClient::Configuration.new(flags_configuration: flags_config, bandits_configuration: bandits_config) + + bandits = configuration.bandits_configuration + + expect(bandits).to be_a String + end + + it "returns nil when there's no bandits" do + configuration = EppoClient::Configuration.new(flags_configuration: flags_config) + + bandits = configuration.bandits_configuration + + expect(bandits).to be_nil + end + end + + it "can be reinstantiated from own configuration" do + config1 = EppoClient::Configuration.new(flags_configuration: flags_config) + + config2 = EppoClient::Configuration.new(flags_configuration: config1.flags_configuration, bandits_configuration: config1.bandits_configuration) + + expect(config1.flags_configuration).to eq(config2.flags_configuration) + expect(config1.bandits_configuration).to eq(config2.bandits_configuration) + end + + it "can be reinstantiated from own configuration (with bandits)" do + flags_config = File.read "../sdk-test-data/ufc/bandit-flags-v1.json" + bandits_config = File.read "../sdk-test-data/ufc/bandit-models-v1.json" + config1 = EppoClient::Configuration.new(flags_configuration: flags_config, bandits_configuration: bandits_config) + + config2 = EppoClient::Configuration.new(flags_configuration: config1.flags_configuration, bandits_configuration: config1.bandits_configuration) + + expect(config1.flags_configuration).to eq(config2.flags_configuration) + # JSON parsing is an internal detail and is not a public + # guarantee. We're using it here because serialization order of + # bandits is not guaranteed. + expect(JSON.parse(config1.bandits_configuration)).to eq(JSON.parse(config2.bandits_configuration)) + end +end diff --git a/ruby-sdk/spec/eppo_client_spec.rb b/ruby-sdk/spec/eppo_client_spec.rb index 2903c13f..70d1f4ce 100644 --- a/ruby-sdk/spec/eppo_client_spec.rb +++ b/ruby-sdk/spec/eppo_client_spec.rb @@ -7,6 +7,36 @@ expect(EppoClient::VERSION).not_to be nil end + describe "configuration()" do + it "allows getting configuration" do + init_client_for "ufc" + + configuration = EppoClient::Client.instance.configuration + + expect(configuration).not_to be_nil + end + + it "returns nil when not initialized" do + init_client_for "offline" + + configuration = EppoClient::Client.instance.configuration + + expect(configuration).to be_nil + end + end + + describe "configuration=()" do + it "allows setting configuration on offline client" do + init_client_for "offline" + + configuration = EppoClient::Configuration.new(flags_configuration: File.read("../sdk-test-data/ufc/flags-v1.json")) + + EppoClient::Client.instance.configuration = configuration + + expect(EppoClient::Client.instance.configuration).not_to be_nil + end + end + describe "UFC flag evaluation", :flags do before :all do init_client_for "ufc" diff --git a/ruby-sdk/spec/spec_helper.rb b/ruby-sdk/spec/spec_helper.rb index 3fd2f0f9..8bc22926 100644 --- a/ruby-sdk/spec/spec_helper.rb +++ b/ruby-sdk/spec/spec_helper.rb @@ -3,11 +3,15 @@ require "eppo_client" def init_client_for(test_name) + if test_name == "offline" then + EppoClient::Client.instance.init(EppoClient::Config.new("test-api-key", poll_interval_seconds: nil)) + else config = EppoClient::Config.new("test-api-key", base_url: "http://127.0.0.1:8378/#{test_name}/api") EppoClient::Client.instance.init(config) # Sleep to allow the client to fetch config sleep(0.050) + end end RSpec.configure do |config|