Skip to content

fix: conform feature_flag span event to the OTEL spec #54

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Vadman97 wants to merge 1 commit into
base: main
Choose a base branch
from
+110 −11
Open

fix: conform feature_flag span event to the OTEL spec #54

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 70 additions & 8 deletions ldotel/testing/test_tracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import logging

import pytest
from ldclient import Config, Context, LDClient
from ldclient.evaluation import EvaluationDetail
Expand Down Expand Up @@ -63,7 +65,7 @@ def test_records_basic_span_event(self, client: LDClient, exporter: SpanExporter
assert event.attributes['feature_flag.key'] == 'boolean'
assert event.attributes['feature_flag.provider.name'] == 'LaunchDarkly'
assert event.attributes['feature_flag.context.id'] == 'org:org-key'
assert event.attributes['feature_flag.result.variationIndex'] == '0'
assert event.attributes['feature_flag.result.variationIndex'] == 0
assert 'feature_flag.result.value' not in event.attributes
assert 'feature_flag.result.reason.inExperiment' not in event.attributes

Expand All @@ -81,7 +83,7 @@ def test_can_include_variant(self, client: LDClient, exporter: SpanExporter, tra
assert event.attributes['feature_flag.key'] == 'boolean'
assert event.attributes['feature_flag.provider.name'] == 'LaunchDarkly'
assert event.attributes['feature_flag.context.id'] == 'org:org-key'
assert event.attributes['feature_flag.result.variationIndex'] == '0'
assert event.attributes['feature_flag.result.variationIndex'] == 0
assert event.attributes['feature_flag.result.value'] == 'true'
assert 'feature_flag.result.reason.inExperiment' not in event.attributes

Expand Down Expand Up @@ -112,7 +114,7 @@ def test_can_include_value_types(self, flag_key, variations, variation_index, ex
assert event.attributes['feature_flag.key'] == flag_key
assert event.attributes['feature_flag.provider.name'] == 'LaunchDarkly'
assert event.attributes['feature_flag.context.id'] == 'org:org-key'
assert event.attributes['feature_flag.result.variationIndex'] == str(variation_index)
assert event.attributes['feature_flag.result.variationIndex'] == variation_index
assert event.attributes['feature_flag.result.value'] == json.dumps(expected_value)
assert 'feature_flag.result.reason.inExperiment' not in event.attributes

Expand Down Expand Up @@ -146,7 +148,7 @@ def test_add_span_leaves_events_on_top_level_span(self, client: LDClient, export
assert event.attributes['feature_flag.key'] == 'boolean'
assert event.attributes['feature_flag.provider.name'] == 'LaunchDarkly'
assert event.attributes['feature_flag.context.id'] == 'org:org-key'
assert event.attributes['feature_flag.result.variationIndex'] == '0'
assert event.attributes['feature_flag.result.variationIndex'] == 0
assert 'feature_flag.result.value' not in event.attributes
assert 'feature_flag.result.reason.inExperiment' not in event.attributes

Expand Down Expand Up @@ -174,15 +176,15 @@ def test_hook_makes_its_span_active(self, client: LDClient, exporter: SpanExport
assert middle.events[0].attributes['feature_flag.key'] == 'boolean'
assert middle.events[0].attributes['feature_flag.provider.name'] == 'LaunchDarkly'
assert middle.events[0].attributes['feature_flag.context.id'] == 'org:org-key'
assert middle.events[0].attributes['feature_flag.result.variationIndex'] == '0'
assert middle.events[0].attributes['feature_flag.result.variationIndex'] == 0
assert 'feature_flag.result.value' not in middle.events[0].attributes
assert 'feature_flag.result.reason.inExperiment' not in middle.events[0].attributes

assert top.events[0].name == 'feature_flag'
assert top.events[0].attributes['feature_flag.key'] == 'boolean'
assert top.events[0].attributes['feature_flag.provider.name'] == 'LaunchDarkly'
assert top.events[0].attributes['feature_flag.context.id'] == 'org:org-key'
assert top.events[0].attributes['feature_flag.result.variationIndex'] == '0'
assert top.events[0].attributes['feature_flag.result.variationIndex'] == 0
assert 'feature_flag.result.value' not in top.events[0].attributes
assert 'feature_flag.result.reason.inExperiment' not in top.events[0].attributes

Expand Down Expand Up @@ -215,8 +217,8 @@ def test_records_in_experiment_attribute(self, exporter: SpanExporter, tracer: T
assert event.attributes['feature_flag.key'] == 'experiment-flag'
assert event.attributes['feature_flag.provider.name'] == 'LaunchDarkly'
assert event.attributes['feature_flag.context.id'] == 'org:org-key'
assert event.attributes['feature_flag.result.variationIndex'] == '1'
assert event.attributes['feature_flag.result.reason.inExperiment'] == 'true'
assert event.attributes['feature_flag.result.variationIndex'] == 1
assert event.attributes['feature_flag.result.reason.inExperiment'] is True
assert 'feature_flag.result.value' not in event.attributes

def test_does_not_include_variation_index_when_none(self, exporter: SpanExporter, tracer: Tracer):
Expand Down Expand Up @@ -251,3 +253,63 @@ def test_does_not_include_variation_index_when_none(self, exporter: SpanExporter
assert 'feature_flag.result.variationIndex' not in event.attributes
assert 'feature_flag.result.reason.inExperiment' not in event.attributes
assert 'feature_flag.result.value' not in event.attributes

def test_records_attributes_with_specified_types(self, exporter: SpanExporter, tracer: Tracer):
"""
The OTEL spec types variationIndex as an int and inExperiment as a
boolean. Guard against them regressing to strings, which would break
consumers that match on the typed value.
"""
series_context = EvaluationSeriesContext(
key='experiment-flag',
context=Context.create('org-key', 'org'),
default_value=False,
method='variation',
)
detail = EvaluationDetail(value=True, variation_index=1, reason={"inExperiment": True})

hook = Hook()
with tracer.start_as_current_span("test_records_attributes_with_specified_types"):
data = hook.before_evaluation(series_context, {}) # type: ignore
hook.after_evaluation(series_context, data, detail) # type: ignore

event = exporter.get_finished_spans()[0].events[0] # type: ignore[attr-defined]

variation_index = event.attributes['feature_flag.result.variationIndex']
assert isinstance(variation_index, int) and not isinstance(variation_index, bool)
assert variation_index == 1

in_experiment = event.attributes['feature_flag.result.reason.inExperiment']
assert isinstance(in_experiment, bool)
assert in_experiment is True

def test_records_set_id_when_environment_id_configured(self, client: LDClient, exporter: SpanExporter, tracer: Tracer):
client.add_hook(Hook(HookOptions(environment_id='my-environment-id')))
with tracer.start_as_current_span("test_records_set_id_when_environment_id_configured"):
client.variation('boolean', Context.create('org-key', 'org'), False)

event = exporter.get_finished_spans()[0].events[0] # type: ignore[attr-defined]
assert event.attributes['feature_flag.set.id'] == 'my-environment-id'

def test_omits_set_id_when_environment_id_not_configured(self, client: LDClient, exporter: SpanExporter, tracer: Tracer):
client.add_hook(Hook())
with tracer.start_as_current_span("test_omits_set_id_when_environment_id_not_configured"):
client.variation('boolean', Context.create('org-key', 'org'), False)

event = exporter.get_finished_spans()[0].events[0] # type: ignore[attr-defined]
assert 'feature_flag.set.id' not in event.attributes

@pytest.mark.parametrize("environment_id", ['', 0, False, []])
def test_ignores_and_logs_invalid_environment_id(self, environment_id, td: TestData, exporter: SpanExporter, tracer: Tracer, caplog):
config = Config('sdk-key', update_processor_class=td, send_events=False)
client = LDClient(config=config)

with caplog.at_level(logging.WARNING, logger='ldclient.otel'):
client.add_hook(Hook(HookOptions(environment_id=environment_id)))

with tracer.start_as_current_span("test_ignores_and_logs_invalid_environment_id"):
client.variation('boolean', Context.create('org-key', 'org'), False)

event = exporter.get_finished_spans()[0].events[0] # type: ignore[attr-defined]
assert 'feature_flag.set.id' not in event.attributes
assert any(record.levelname == 'WARNING' for record in caplog.records)
43 changes: 40 additions & 3 deletions ldotel/tracing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import json
import logging
import warnings
from dataclasses import dataclass
from typing import Dict, Optional

from ldclient.evaluation import EvaluationDetail
from ldclient.hook import EvaluationSeriesContext
Expand All @@ -9,6 +11,9 @@
from opentelemetry import trace
from opentelemetry.context import attach, detach
from opentelemetry.trace import Span, get_current_span, set_span_in_context
from opentelemetry.util.types import AttributeValue

log = logging.getLogger('ldclient.otel')


@dataclass
Expand Down Expand Up @@ -39,11 +44,40 @@ class HookOptions:
span events.
"""

environment_id: Optional[str] = None
"""
If set, then the tracing hook will add the environment ID to span events as
the ``feature_flag.set.id`` attribute.

The value must be a non-empty string. Any other value is ignored, and a
warning is logged, which is equivalent to not specifying an environment ID
at all.
"""


def _validate_environment_id(environment_id: Optional[str]) -> Optional[str]:
"""
Validate a configured environment ID, returning it only when it is a
non-empty string. An invalid value is logged and treated as unset.
"""
if environment_id is None:
return None

if not isinstance(environment_id, str) or environment_id == '':
log.warning(
'The environment ID provided to the LaunchDarkly tracing hook must be a non-empty string. '
'The feature_flag.set.id attribute will not be added to span events.'
)
return None

return environment_id


class Hook(LDHook):
def __init__(self, options: HookOptions = HookOptions()):
self.__tracer = trace.get_tracer_provider().get_tracer("launchdarkly")
self.__options = options
self.__environment_id = _validate_environment_id(options.environment_id)
if self.__options.include_variant:
warnings.warn(
"The 'include_variant' option is deprecated and will be removed in a future version. "
Expand Down Expand Up @@ -105,17 +139,20 @@ def after_evaluation(self, series_context: EvaluationSeriesContext, data: dict,
if span is None:
return data

attributes = {
attributes: Dict[str, AttributeValue] = {
'feature_flag.context.id': series_context.context.fully_qualified_key,
'feature_flag.key': series_context.key,
'feature_flag.provider.name': 'LaunchDarkly',
}

if self.__environment_id is not None:
attributes['feature_flag.set.id'] = self.__environment_id

if detail.variation_index is not None:
attributes['feature_flag.result.variationIndex'] = str(detail.variation_index)
attributes['feature_flag.result.variationIndex'] = detail.variation_index

if detail.reason.get('inExperiment'):
attributes['feature_flag.result.reason.inExperiment'] = 'true'
attributes['feature_flag.result.reason.inExperiment'] = True

if self.__options.include_value or self.__options.include_variant:
attributes['feature_flag.result.value'] = json.dumps(detail.value)
Expand Down
Loading