diff --git a/CHANGELOG.md b/CHANGELOG.md index 831c43cb..3b81056b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,18 @@ All notable changes to the LaunchDarkly Ruby SDK will be documented in this file. This project adheres to [Semantic Versioning](http://semver.org). +## [5.1.0] - 2018-08-27 +### Added: +- The new `LDClient` method `all_flags_state()` should be used instead of `all_flags()` if you are passing flag data to the front end for use with the JavaScript SDK. It preserves some flag metadata that the front end requires in order to send analytics events correctly. Versions 2.5.0 and above of the JavaScript SDK are able to use this metadata, but the output of `all_flags_state()` will still work with older versions. +- The `all_flags_state()` method also allows you to select only client-side-enabled flags to pass to the front end, by using the option `client_side_only: true`. + +### Changed: +- Unexpected exceptions are now logged at `ERROR` level, and exception stacktraces at `DEBUG` level. Previously, both were being logged at `WARN` level. + +### Deprecated: +- `LDClient.all_flags()` + + ## [5.0.1] - 2018-07-02 ### Fixed: Fixed a regression in version 5.0.0 that could prevent the client from reconnecting if the stream connection was dropped by the server. diff --git a/CODEOWNERS b/CODEOWNERS index 44429ee1..8b137891 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @ashanbrown + diff --git a/lib/ldclient-rb.rb b/lib/ldclient-rb.rb index ce9d0307..7264b220 100644 --- a/lib/ldclient-rb.rb +++ b/lib/ldclient-rb.rb @@ -1,6 +1,7 @@ require "ldclient-rb/version" require "ldclient-rb/util" require "ldclient-rb/evaluation" +require "ldclient-rb/flags_state" require "ldclient-rb/ldclient" require "ldclient-rb/cache_store" require "ldclient-rb/expiring_cache" diff --git a/lib/ldclient-rb/events.rb b/lib/ldclient-rb/events.rb index 0c9a0ece..202fc235 100644 --- a/lib/ldclient-rb/events.rb +++ b/lib/ldclient-rb/events.rb @@ -142,7 +142,7 @@ def main_loop(queue, buffer, flush_workers) message.completed end rescue => e - @config.logger.warn { "[LDClient] Unexpected error in event processor: #{e.inspect}. \nTrace: #{e.backtrace}" } + Util.log_exception(@config.logger, "Unexpected error in event processor", e) end end end @@ -226,7 +226,7 @@ def trigger_flush(buffer, flush_workers) resp = EventPayloadSendTask.new.run(@sdk_key, @config, @client, payload, @formatter) handle_response(resp) if !resp.nil? rescue => e - @config.logger.warn { "[LDClient] Unexpected error in event processor: #{e.inspect}. \nTrace: #{e.backtrace}" } + Util.log_exception(@config.logger, "Unexpected error in event processor", e) end end buffer.clear if success # Reset our internal state, these events now belong to the flush worker diff --git a/lib/ldclient-rb/flags_state.rb b/lib/ldclient-rb/flags_state.rb new file mode 100644 index 00000000..09f88975 --- /dev/null +++ b/lib/ldclient-rb/flags_state.rb @@ -0,0 +1,66 @@ +require 'json' + +module LaunchDarkly + # + # A snapshot of the state of all feature flags with regard to a specific user, generated by + # calling the client's all_flags_state method. Serializing this object to JSON using + # JSON.generate (or the to_json method) will produce the appropriate data structure for + # bootstrapping the LaunchDarkly JavaScript client. + # + class FeatureFlagsState + def initialize(valid) + @flag_values = {} + @flag_metadata = {} + @valid = valid + end + + # Used internally to build the state map. + def add_flag(flag, value, variation) + key = flag[:key] + @flag_values[key] = value + meta = { version: flag[:version], trackEvents: flag[:trackEvents] } + meta[:variation] = variation if !variation.nil? + meta[:debugEventsUntilDate] = flag[:debugEventsUntilDate] if flag[:debugEventsUntilDate] + @flag_metadata[key] = meta + end + + # Returns true if this object contains a valid snapshot of feature flag state, or false if the + # state could not be computed (for instance, because the client was offline or there was no user). + def valid? + @valid + end + + # Returns the value of an individual feature flag at the time the state was recorded. + # Returns nil if the flag returned the default value, or if there was no such flag. + def flag_value(key) + @flag_values[key] + end + + # Returns a map of flag keys to flag values. If a flag would have evaluated to the default value, + # its value will be nil. + # + # Do not use this method if you are passing data to the front end to "bootstrap" the JavaScript client. + # Instead, use as_json. + def values_map + @flag_values + end + + # Returns a hash that can be used as a JSON representation of the entire state map, in the format + # used by the LaunchDarkly JavaScript SDK. Use this method if you are passing data to the front end + # in order to "bootstrap" the JavaScript client. + # + # Do not rely on the exact shape of this data, as it may change in future to support the needs of + # the JavaScript client. + def as_json(*) # parameter is unused, but may be passed if we're using the json gem + ret = @flag_values.clone + ret['$flagsState'] = @flag_metadata + ret['$valid'] = @valid + ret + end + + # Same as as_json, but converts the JSON structure into a string. + def to_json(*a) + as_json.to_json(a) + end + end +end diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb index 5c0e872d..e9873679 100644 --- a/lib/ldclient-rb/ldclient.rb +++ b/lib/ldclient-rb/ldclient.rb @@ -162,7 +162,7 @@ def variation(key, user, default) @event_processor.add_event(make_feature_event(feature, user, res[:variation], value, default)) return value rescue => exn - @config.logger.warn { "[LDClient] Error evaluating feature flag: #{exn.inspect}. \nTrace: #{exn.backtrace}" } + Util.log_exception(@config.logger, "Error evaluating feature flag", exn) @event_processor.add_event(make_feature_event(feature, user, nil, default, default)) return default end @@ -193,26 +193,61 @@ def track(event_name, user, data) end # - # Returns all feature flag values for the given user + # Returns all feature flag values for the given user. This method is deprecated - please use + # {#all_flags_state} instead. Current versions of the client-side SDK will not generate analytics + # events correctly if you pass the result of all_flags. + # + # @param user [Hash] The end user requesting the feature flags + # @return [Hash] a hash of feature flag keys to values # def all_flags(user) - sanitize_user(user) - return Hash.new if @config.offline? + all_flags_state(user).values_map + end - unless user - @config.logger.error { "[LDClient] Must specify user in all_flags" } - return Hash.new + # + # Returns a FeatureFlagsState object that encapsulates the state of all feature flags for a given user, + # including the flag values and also metadata that can be used on the front end. This method does not + # send analytics events back to LaunchDarkly. + # + # @param user [Hash] The end user requesting the feature flags + # @param options={} [Hash] Optional parameters to control how the state is generated + # @option options [Boolean] :client_side_only (false) True if only flags marked for use with the + # client-side SDK should be included in the state. By default, all flags are included. + # @return [FeatureFlagsState] a FeatureFlagsState object which can be serialized to JSON + # + def all_flags_state(user, options={}) + return FeatureFlagsState.new(false) if @config.offline? + + unless user && !user[:key].nil? + @config.logger.error { "[LDClient] User and user key must be specified in all_flags_state" } + return FeatureFlagsState.new(false) end + sanitize_user(user) + begin features = @store.all(FEATURES) - - # TODO rescue if necessary - Hash[features.map{ |k, f| [k, evaluate(f, user, @store, @config.logger)[:value]] }] rescue => exn - @config.logger.warn { "[LDClient] Error evaluating all flags: #{exn.inspect}. \nTrace: #{exn.backtrace}" } - return Hash.new + Util.log_exception(@config.logger, "Unable to read flags for all_flags_state", exn) + return FeatureFlagsState.new(false) end + + state = FeatureFlagsState.new(true) + client_only = options[:client_side_only] || false + features.each do |k, f| + if client_only && !f[:clientSide] + next + end + begin + result = evaluate(f, user, @store, @config.logger) + state.add_flag(f, result[:value], result[:variation]) + rescue => exn + Util.log_exception(@config.logger, "Error evaluating flag \"#{k}\" in all_flags_state", exn) + state.add_flag(f, nil, nil) + end + end + + state end # @@ -226,12 +261,6 @@ def close @store.stop end - def log_exception(caller, exn) - error_traceback = "#{exn.inspect} #{exn}\n\t#{exn.backtrace.join("\n\t")}" - error = "[LDClient] Unexpected exception in #{caller}: #{error_traceback}" - @config.logger.error { error } - end - def sanitize_user(user) if user[:key] user[:key] = user[:key].to_s @@ -252,7 +281,7 @@ def make_feature_event(flag, user, variation, value, default) } end - private :evaluate, :log_exception, :sanitize_user, :make_feature_event + private :evaluate, :sanitize_user, :make_feature_event end # diff --git a/lib/ldclient-rb/util.rb b/lib/ldclient-rb/util.rb index 6ba70dbc..707ba3ce 100644 --- a/lib/ldclient-rb/util.rb +++ b/lib/ldclient-rb/util.rb @@ -1,6 +1,11 @@ module LaunchDarkly module Util + def self.log_exception(logger, message, exc) + logger.error { "[LDClient] #{message}: #{exc.inspect}" } + logger.debug { "[LDClient] Exception trace: #{exc.backtrace}" } + end + def self.http_error_recoverable?(status) if status >= 400 && status < 500 status == 400 || status == 408 || status == 429 diff --git a/lib/ldclient-rb/version.rb b/lib/ldclient-rb/version.rb index e6bfa967..1d54cc52 100644 --- a/lib/ldclient-rb/version.rb +++ b/lib/ldclient-rb/version.rb @@ -1,3 +1,3 @@ module LaunchDarkly - VERSION = "5.0.1" + VERSION = "5.1.0" end diff --git a/spec/flags_state_spec.rb b/spec/flags_state_spec.rb new file mode 100644 index 00000000..3d21029b --- /dev/null +++ b/spec/flags_state_spec.rb @@ -0,0 +1,82 @@ +require "spec_helper" +require "json" + +describe LaunchDarkly::FeatureFlagsState do + subject { LaunchDarkly::FeatureFlagsState } + + it "can get flag value" do + state = subject.new(true) + flag = { key: 'key' } + state.add_flag(flag, 'value', 1) + + expect(state.flag_value('key')).to eq 'value' + end + + it "returns nil for unknown flag" do + state = subject.new(true) + + expect(state.flag_value('key')).to be nil + end + + it "can be converted to values map" do + state = subject.new(true) + flag1 = { key: 'key1' } + flag2 = { key: 'key2' } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + + expect(state.values_map).to eq({ 'key1' => 'value1', 'key2' => 'value2' }) + end + + it "can be converted to JSON structure" do + state = subject.new(true) + flag1 = { key: "key1", version: 100, offVariation: 0, variations: [ 'value1' ], trackEvents: false } + flag2 = { key: "key2", version: 200, offVariation: 1, variations: [ 'x', 'value2' ], trackEvents: true, debugEventsUntilDate: 1000 } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + + result = state.as_json + expect(result).to eq({ + 'key1' => 'value1', + 'key2' => 'value2', + '$flagsState' => { + 'key1' => { + :variation => 0, + :version => 100, + :trackEvents => false + }, + 'key2' => { + :variation => 1, + :version => 200, + :trackEvents => true, + :debugEventsUntilDate => 1000 + } + }, + '$valid' => true + }) + end + + it "can be converted to JSON string" do + state = subject.new(true) + flag1 = { key: "key1", version: 100, offVariation: 0, variations: [ 'value1' ], trackEvents: false } + flag2 = { key: "key2", version: 200, offVariation: 1, variations: [ 'x', 'value2' ], trackEvents: true, debugEventsUntilDate: 1000 } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + + object = state.as_json + str = state.to_json + expect(object.to_json).to eq(str) + end + + it "uses our custom serializer with JSON.generate" do + state = subject.new(true) + flag1 = { key: "key1", version: 100, offVariation: 0, variations: [ 'value1' ], trackEvents: false } + flag2 = { key: "key2", version: 200, offVariation: 1, variations: [ 'x', 'value2' ], trackEvents: true, debugEventsUntilDate: 1000 } + state.add_flag(flag1, 'value1', 0) + state.add_flag(flag2, 'value2', 1) + + stringFromToJson = state.to_json + stringFromGenerate = JSON.generate(state) + expect(stringFromGenerate).to eq(stringFromToJson) + end +end diff --git a/spec/ldclient_spec.rb b/spec/ldclient_spec.rb index 8e4b5eb5..ae76a678 100644 --- a/spec/ldclient_spec.rb +++ b/spec/ldclient_spec.rb @@ -99,6 +99,114 @@ def event_processor end end + describe '#all_flags' do + let(:flag1) { { key: "key1", offVariation: 0, variations: [ 'value1' ] } } + let(:flag2) { { key: "key2", offVariation: 0, variations: [ 'value2' ] } } + + it "returns flag values" do + config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + result = client.all_flags({ key: 'userkey' }) + expect(result).to eq({ 'key1' => 'value1', 'key2' => 'value2' }) + end + + it "returns empty map for nil user" do + config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + result = client.all_flags(nil) + expect(result).to eq({}) + end + + it "returns empty map for nil user key" do + config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + result = client.all_flags({}) + expect(result).to eq({}) + end + + it "returns empty map if offline" do + offline_config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + result = offline_client.all_flags(nil) + expect(result).to eq({}) + end + end + + describe '#all_flags_state' do + let(:flag1) { { key: "key1", version: 100, offVariation: 0, variations: [ 'value1' ], trackEvents: false } } + let(:flag2) { { key: "key2", version: 200, offVariation: 1, variations: [ 'x', 'value2' ], trackEvents: true, debugEventsUntilDate: 1000 } } + + it "returns flags state" do + config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + state = client.all_flags_state({ key: 'userkey' }) + expect(state.valid?).to be true + + values = state.values_map + expect(values).to eq({ 'key1' => 'value1', 'key2' => 'value2' }) + + result = state.as_json + expect(result).to eq({ + 'key1' => 'value1', + 'key2' => 'value2', + '$flagsState' => { + 'key1' => { + :variation => 0, + :version => 100, + :trackEvents => false + }, + 'key2' => { + :variation => 1, + :version => 200, + :trackEvents => true, + :debugEventsUntilDate => 1000 + } + }, + '$valid' => true + }) + end + + it "can be filtered for only client-side flags" do + flag1 = { key: "server-side-1", offVariation: 0, variations: [ 'a' ], clientSide: false } + flag2 = { key: "server-side-2", offVariation: 0, variations: [ 'b' ], clientSide: false } + flag3 = { key: "client-side-1", offVariation: 0, variations: [ 'value1' ], clientSide: true } + flag4 = { key: "client-side-2", offVariation: 0, variations: [ 'value2' ], clientSide: true } + config.feature_store.init({ LaunchDarkly::FEATURES => { + flag1[:key] => flag1, flag2[:key] => flag2, flag3[:key] => flag3, flag4[:key] => flag4 + }}) + + state = client.all_flags_state({ key: 'userkey' }, client_side_only: true) + expect(state.valid?).to be true + + values = state.values_map + expect(values).to eq({ 'client-side-1' => 'value1', 'client-side-2' => 'value2' }) + end + + it "returns empty state for nil user" do + config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + state = client.all_flags_state(nil) + expect(state.valid?).to be false + expect(state.values_map).to eq({}) + end + + it "returns empty state for nil user key" do + config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + state = client.all_flags_state({}) + expect(state.valid?).to be false + expect(state.values_map).to eq({}) + end + + it "returns empty state if offline" do + offline_config.feature_store.init({ LaunchDarkly::FEATURES => { 'key1' => flag1, 'key2' => flag2 } }) + + state = offline_client.all_flags_state({ key: 'userkey' }) + expect(state.valid?).to be false + expect(state.values_map).to eq({}) + end + end + describe '#secure_mode_hash' do it "will return the expected value for a known message and secret" do result = client.secure_mode_hash({key: :Message}) @@ -130,17 +238,6 @@ def event_processor end end - describe '#log_exception' do - it "log error data" do - expect(client.instance_variable_get(:@config).logger).to receive(:error) - begin - raise StandardError.new 'asdf' - rescue StandardError => exn - client.send(:log_exception, 'caller', exn) - end - end - end - describe 'with send_events: false' do let(:config) { LaunchDarkly::Config.new({offline: true, send_events: false, update_processor: update_processor}) } let(:client) { subject.new("secret", config) } diff --git a/spec/util_spec.rb b/spec/util_spec.rb new file mode 100644 index 00000000..50a72f76 --- /dev/null +++ b/spec/util_spec.rb @@ -0,0 +1,17 @@ +require "spec_helper" + +describe LaunchDarkly::Util do + describe 'log_exception' do + let(:logger) { double() } + + it "logs error data" do + expect(logger).to receive(:error) + expect(logger).to receive(:debug) + begin + raise StandardError.new 'asdf' + rescue StandardError => exn + LaunchDarkly::Util.log_exception(logger, "message", exn) + end + end + end +end