Skip to content

Commit f829d07

Browse files
fix: Release stopped event processors and keep identifiers out of logs
- Stop running event processors at exit from one registry that a stopped processor leaves, instead of an at_exit block per start that kept every processor in memory. - Drop the identifier from the duplicate exposure debug log. - Give the accurate reason when get_experiment_flag skips an exposure because flags come from the environment document, which carries no experiment data. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent f803880 commit f829d07

4 files changed

Lines changed: 68 additions & 4 deletions

File tree

‎lib/flagsmith.rb‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,8 @@ def get_value_for_identity(feature_name, user_id = nil, default: nil)
220220

221221
# Get a flag for an identity, taking the same traits as get_identity_flags, and record the
222222
# identity's exposure to the variant it was served when it is in the feature's running experiment.
223-
# Requires enable_events.
223+
# Requires enable_events. Exposure needs remote evaluation: the environment document used by local
224+
# evaluation and offline mode carries no experiment data, so no exposure is recorded in those modes.
224225
def get_experiment_flag(feature_name, identifier, **traits)
225226
require_events!
226227
flag = get_identity_flags(identifier, **traits)[feature_name]
@@ -281,6 +282,7 @@ def require_events!
281282
def exposure_skip_reason(flag)
282283
return 'feature not found' unless flag.is_a?(Flagsmith::Flags::Flag)
283284
return 'feature is disabled' unless flag.enabled?
285+
return 'experiments need remote evaluation, not local evaluation or offline mode' if environment
284286
return 'identity is not in a running experiment' unless flag.experiment&.in_experiment?
285287

286288
'flag has no variant' if flag.variant.nil?

‎lib/flagsmith/sdk/event_processor.rb‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,25 @@
33
require 'set'
44

55
module Flagsmith
6+
# Started event processors, stopped together at exit. A stopped processor leaves, so it is not kept in memory.
7+
module RunningEventProcessors
8+
@processors = Set.new
9+
@mutex = Mutex.new
10+
at_exit { to_a.each(&:stop) }
11+
12+
def self.add(processor)
13+
@mutex.synchronize { @processors.add(processor) }
14+
end
15+
16+
def self.delete(processor)
17+
@mutex.synchronize { @processors.delete(processor) }
18+
end
19+
20+
def self.to_a
21+
@mutex.synchronize { @processors.to_a }
22+
end
23+
end
24+
625
# Buffers tracking and exposure events, and sends them to the Flagsmith events API in batches.
726
#
827
# Events are sent every +flush_interval_seconds+, as soon as +max_buffer_items+ are buffered,
@@ -58,12 +77,13 @@ def start
5877
return if @interval
5978

6079
@interval = set_interval(@flush_interval_seconds) { flush }
61-
at_exit { stop }
80+
RunningEventProcessors.add(self)
6281
end
6382

6483
def stop
6584
@flush_mutex.synchronize { clear_interval(@interval) } if @interval # never interrupts a batch being sent
6685
@interval = nil
86+
RunningEventProcessors.delete(self)
6787
flush
6888
end
6989

@@ -122,7 +142,7 @@ def duplicate_exposure?(event)
122142
return false unless event[:event] == FLAG_EXPOSURE_EVENT
123143
return false if @buffered_exposures.add?(event.values_at(:feature_name, :identifier, :value))
124144

125-
@logger.debug "Skipping duplicate #{FLAG_EXPOSURE_EVENT} event for feature #{event[:feature_name]}, identifier #{event[:identifier]}"
145+
@logger.debug "Skipping duplicate #{FLAG_EXPOSURE_EVENT} event for feature #{event[:feature_name]}"
126146
true
127147
end
128148

‎spec/sdk/event_processor_spec.rb‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ def respond_to_events_with(status)
9999
# Then
100100
expect(sent_batches[0][:body]['events'].map { |event| event['identifier'] }).to eq(%w[user_123 user_456])
101101
expect(logger).to have_received(:debug)
102-
.with('Skipping duplicate $flag_exposure event for feature banner, identifier user_123').once
102+
.with('Skipping duplicate $flag_exposure event for feature banner').once
103103
end
104104

105105
it 'sends an exposure again in a later flush interval' do
@@ -208,4 +208,24 @@ def respond_to_events_with(status)
208208
# Then
209209
expect(sent_batches.length).to eq(1)
210210
end
211+
212+
it 'is stopped at exit while it is running' do
213+
# Given / When
214+
subject.start
215+
216+
# Then
217+
expect(Flagsmith::RunningEventProcessors.to_a).to include(subject)
218+
subject.stop
219+
end
220+
221+
it 'is not kept for exit once stopped' do
222+
# Given
223+
subject.start
224+
225+
# When
226+
subject.stop
227+
228+
# Then
229+
expect(Flagsmith::RunningEventProcessors.to_a).not_to include(subject)
230+
end
211231
end

‎spec/sdk/experiments_spec.rb‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,28 @@
134134
end
135135
end
136136

137+
context 'when the flags come from the environment document' do
138+
subject do
139+
described_class.new(
140+
environment_key: 'ser.environment-key', enable_events: true, logger: logger, offline_mode: true,
141+
offline_handler: Flagsmith::OfflineHandlers::LocalFileHandler.new('spec/sdk/fixtures/environment.json')
142+
)
143+
end
144+
145+
it 'returns the identity flag and records no exposure' do
146+
# Given / When
147+
flag = subject.get_experiment_flag('some_feature', 'user_123')
148+
149+
# Then
150+
expect(flag.value).to eq('some-value')
151+
expect(subject.event_processor).not_to have_received(:track_exposure_event)
152+
expect(logger).to have_received(:debug).with(
153+
'Not sending $flag_exposure for feature some_feature: ' \
154+
'experiments need remote evaluation, not local evaluation or offline mode.'
155+
)
156+
end
157+
end
158+
137159
context 'when events are not enabled' do
138160
let(:enable_events) { false }
139161

0 commit comments

Comments
 (0)