Skip to content

Commit 587e2e4

Browse files
fix(ruby): omit nil feature_flag.context.id for invalid contexts (#641)
## What When a flag is evaluated with an **invalid context**, the observability plugin's evaluation hook sets `feature_flag.context.id` to `nil`. OpenTelemetry rejects nil attribute values, so it logs an error on every such evaluation — twice (once on the `evaluation` span, once on the `feature_flag` event): ``` OpenTelemetry error: invalid span attribute value type NilClass for key 'feature_flag.context.id' on span 'evaluation' OpenTelemetry error: invalid event attribute value type NilClass for key 'feature_flag.context.id' on span 'evaluation' ``` This guards on the key itself via `series_context.context&.fully_qualified_key` and `.compact`s the span/event attribute hashes so nil values are dropped centrally — before they reach OTel — for this and any future optional attribute. ## Root cause The old guard only checked the context object was non-nil: ```ruby context = series_context.context if context attrs[FEATURE_FLAG_CONTEXT_ID] = context.fully_qualified_key end ``` But an invalid context (e.g. an empty/missing `key` or a non-string `kind`) is a *present* `LDContext` object whose `fully_qualified_key` is `nil` (`create_invalid_context` → `new(nil, nil, nil, …)` in the server SDK). So we handed OTel a nil value. The fix omits the attribute entirely when the key is unavailable. ## Impact Cosmetic — telemetry still exports and lands normally (routing is by the `launchdarkly.project_id` resource attribute, not this per-span attribute). But it's noisy in customer logs, and the nil is also a signal that those specific evaluations are running against an invalid context (and therefore falling back to the flag's default value). ## Reported by Surfaced by a customer integrating the Ruby plugin on Rails after their upgrade — last remaining log noise once auto-instrumentation was working. ## Test plan - Adds a regression test that captures the OTel error channel and reproduces the exact log lines without the fix. - `bundle exec rake test` → 115 runs, 338 assertions, 0 failures. ## Follow-up (not in this PR) `launchdarkly-server-sdk-otel`'s `tracing_hook.rb` has the same nil-context pattern (`evaluation_series_context.context.fully_qualified_key` with no guard). Separate gem; worth a matching fix if/when that surfaces. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Telemetry-only change that omits optional span/event attributes; no evaluation, auth, or export routing behavior changes. > > **Overview** > Stops OpenTelemetry from logging **invalid attribute value type NilClass** on every flag evaluation when the context is invalid but still a non-nil `LDContext` (e.g. non-string `kind`), where `fully_qualified_key` is nil. > > The evaluation hook no longer gates on `context` alone; it resolves the id via **`context_id_for`** (`series_context.context&.fully_qualified_key`) and **`.compact`s** span and `feature_flag` event attribute hashes in **`build_before_attributes`** and **`add_feature_flag_event`** so nil optional fields are omitted before they reach OTel. > > Adds a regression test that captures the OTel logger and asserts `feature_flag.context.id` is absent on the span and event for invalid contexts. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 56253e5. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Claude <noreply@anthropic.com>
1 parent c2ba28b commit 587e2e4

2 files changed

Lines changed: 61 additions & 10 deletions

File tree

sdk/@launchdarkly/observability-ruby/lib/launchdarkly_observability/hook.rb

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,23 @@ def build_before_attributes(series_context)
9191
FEATURE_FLAG_PROVIDER_NAME => 'LaunchDarkly'
9292
}
9393

94-
context = series_context.context
95-
if context
96-
attrs[FEATURE_FLAG_CONTEXT_ID] = context.fully_qualified_key
97-
end
94+
attrs[FEATURE_FLAG_CONTEXT_ID] = context_id_for(series_context)
9895

99-
attrs
96+
# OpenTelemetry rejects nil attribute values and logs an error for each
97+
# one. Drop nils centrally so individual call sites cannot reintroduce the
98+
# "invalid attribute value type NilClass" noise (e.g. an invalid context
99+
# whose fully_qualified_key is nil).
100+
attrs.compact
101+
end
102+
103+
# Returns the fully-qualified context key, or nil if unavailable.
104+
#
105+
# An invalid context (e.g. a non-string context kind) is still a non-nil
106+
# object but yields a nil fully_qualified_key. OpenTelemetry rejects nil
107+
# attribute values ("invalid attribute value type NilClass"), so callers
108+
# must omit the attribute entirely when this returns nil.
109+
def context_id_for(series_context)
110+
series_context.context&.fully_qualified_key
100111
end
101112

102113
def serialize_value(value)
@@ -127,10 +138,7 @@ def add_feature_flag_event(span, series_context, detail)
127138
FEATURE_FLAG_PROVIDER_NAME => 'LaunchDarkly'
128139
}
129140

130-
context = series_context.context
131-
if context
132-
event_attributes[FEATURE_FLAG_CONTEXT_ID] = context.fully_qualified_key
133-
end
141+
event_attributes[FEATURE_FLAG_CONTEXT_ID] = context_id_for(series_context)
134142

135143
if detail.variation_index
136144
event_attributes[FEATURE_FLAG_RESULT_VARIANT] = detail.variation_index.to_s
@@ -146,7 +154,9 @@ def add_feature_flag_event(span, series_context, detail)
146154
add_reason_event_details(event_attributes, reason)
147155
end
148156

149-
span.add_event(FEATURE_FLAG_EVENT_NAME, attributes: event_attributes)
157+
# See build_before_attributes: drop nil values so a missing context id (or
158+
# any future optional attribute) cannot emit a nil-attribute OTel error.
159+
span.add_event(FEATURE_FLAG_EVENT_NAME, attributes: event_attributes.compact)
150160
end
151161

152162
def add_reason_event_details(event_attributes, reason)

sdk/@launchdarkly/observability-ruby/test/hook_test.rb

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,47 @@ def test_context_id_uses_fully_qualified_key_for_multi_kind
192192
assert_equal 'org:org-1:user:user-1', span.attributes['feature_flag.context.id']
193193
end
194194

195+
# Regression: an invalid context (e.g. a non-string kind) is a non-nil
196+
# object whose fully_qualified_key is nil. Setting a nil OTel attribute
197+
# raised "OpenTelemetry error: invalid attribute value type NilClass for
198+
# key 'feature_flag.context.id'" on every evaluation. The attribute must be
199+
# omitted instead.
200+
def test_invalid_context_omits_context_id_attribute
201+
invalid_context = LaunchDarkly::LDContext.create({ key: 'k', kind: 123 })
202+
refute invalid_context.valid?
203+
assert_nil invalid_context.fully_qualified_key
204+
205+
series_context = LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext.new(
206+
'my-flag', invalid_context, false, :variation
207+
)
208+
209+
# Capture OpenTelemetry's error channel. Setting an attribute to nil does not
210+
# raise and does not set the attribute (OTel drops it) — the only observable
211+
# symptom is an "invalid attribute value type NilClass" error logged here,
212+
# which is exactly the noise the customer saw on every evaluation.
213+
captured = StringIO.new
214+
original_logger = OpenTelemetry.logger
215+
OpenTelemetry.logger = ::Logger.new(captured)
216+
begin
217+
data = @hook.before_evaluation(series_context, {})
218+
@hook.after_evaluation(series_context, data, create_evaluation_detail)
219+
ensure
220+
OpenTelemetry.logger = original_logger
221+
end
222+
223+
refute_match(/invalid .*attribute value type NilClass/, captured.string,
224+
'must not emit an OTel nil-attribute error for an invalid context')
225+
refute_match(/feature_flag\.context\.id/, captured.string)
226+
227+
span = @exporter.finished_spans.first
228+
refute_nil span
229+
refute span.attributes.key?('feature_flag.context.id')
230+
231+
flag_event = span.events.find { |e| e.name == 'feature_flag' }
232+
refute_nil flag_event
233+
refute flag_event.attributes.key?('feature_flag.context.id')
234+
end
235+
195236
def test_nil_context_does_not_raise_and_still_records_event
196237
series_context = LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext.new(
197238
'my-flag', nil, false, :variation

0 commit comments

Comments
 (0)