diff --git a/ldotel/testing/test_tracing.py b/ldotel/testing/test_tracing.py index 73ba4e0..a96eb4c 100644 --- a/ldotel/testing/test_tracing.py +++ b/ldotel/testing/test_tracing.py @@ -1,3 +1,6 @@ +from dataclasses import dataclass +from typing import Any, Optional + import pytest from ldclient import Config, Context, LDClient from ldclient.evaluation import EvaluationDetail @@ -13,6 +16,31 @@ from ldotel.tracing import Hook, HookOptions +@dataclass +class _SeriesContextWithoutEnvironmentId: + """ + The shape of ``EvaluationSeriesContext`` in SDK versions which do not + report the environment ID to hooks. + """ + + key: str + context: Context + default_value: Any + method: str + + +def _series_context(environment_id: Optional[str]) -> EvaluationSeriesContext: + series_context = EvaluationSeriesContext( + key='boolean', + context=Context.create('org-key', 'org'), + default_value=False, + method='variation', + ) + setattr(series_context, 'environment_id', environment_id) + + return series_context + + @pytest.fixture def td() -> TestData: td = TestData.data_source() @@ -63,7 +91,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 @@ -81,7 +109,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 @@ -112,7 +140,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 @@ -146,7 +174,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 @@ -174,7 +202,7 @@ 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 @@ -182,7 +210,7 @@ def test_hook_makes_its_span_active(self, client: LDClient, exporter: SpanExport 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 @@ -215,8 +243,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): @@ -251,3 +279,110 @@ 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 + + def test_records_set_id_from_series_context(self, exporter: SpanExporter, tracer: Tracer): + series_context = _series_context(environment_id='series-environment-id') + + hook = Hook() + with tracer.start_as_current_span("test_records_set_id_from_series_context"): + data = hook.before_evaluation(series_context, {}) # type: ignore + hook.after_evaluation(series_context, data, EvaluationDetail(value=True, variation_index=0, reason={})) # type: ignore + + event = exporter.get_finished_spans()[0].events[0] # type: ignore[attr-defined] + assert event.attributes['feature_flag.set.id'] == 'series-environment-id' + + def test_configured_environment_id_takes_precedence_over_series_context(self, exporter: SpanExporter, tracer: Tracer): + series_context = _series_context(environment_id='series-environment-id') + + hook = Hook(HookOptions(environment_id='configured-environment-id')) + with tracer.start_as_current_span("test_configured_environment_id_takes_precedence_over_series_context"): + data = hook.before_evaluation(series_context, {}) # type: ignore + hook.after_evaluation(series_context, data, EvaluationDetail(value=True, variation_index=0, reason={})) # type: ignore + + event = exporter.get_finished_spans()[0].events[0] # type: ignore[attr-defined] + assert event.attributes['feature_flag.set.id'] == 'configured-environment-id' + + @pytest.mark.parametrize("environment_id", [None, '']) + def test_omits_set_id_when_series_context_environment_id_is_unusable(self, environment_id, exporter: SpanExporter, tracer: Tracer): + series_context = _series_context(environment_id=environment_id) + + hook = Hook() + with tracer.start_as_current_span("test_omits_set_id_when_series_context_environment_id_is_unusable"): + data = hook.before_evaluation(series_context, {}) # type: ignore + hook.after_evaluation(series_context, data, EvaluationDetail(value=True, variation_index=0, reason={})) # type: ignore + + event = exporter.get_finished_spans()[0].events[0] # type: ignore[attr-defined] + assert 'feature_flag.set.id' not in event.attributes + + def test_omits_set_id_when_series_context_has_no_environment_id_attribute(self, exporter: SpanExporter, tracer: Tracer): + series_context = _SeriesContextWithoutEnvironmentId( + key='boolean', + context=Context.create('org-key', 'org'), + default_value=False, + method='variation', + ) + + hook = Hook() + with tracer.start_as_current_span("test_omits_set_id_when_series_context_has_no_environment_id_attribute"): + data = hook.before_evaluation(series_context, {}) # type: ignore + hook.after_evaluation(series_context, data, EvaluationDetail(value=True, variation_index=0, reason={})) # type: ignore + + 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_invalid_environment_id(self, environment_id, td: TestData, exporter: SpanExporter, tracer: Tracer): + config = Config('sdk-key', update_processor_class=td, send_events=False) + client = LDClient(config=config) + client.add_hook(Hook(HookOptions(environment_id=environment_id))) + + with tracer.start_as_current_span("test_ignores_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 diff --git a/ldotel/tracing.py b/ldotel/tracing.py index e23969d..89918da 100644 --- a/ldotel/tracing.py +++ b/ldotel/tracing.py @@ -1,6 +1,7 @@ import json import warnings from dataclasses import dataclass +from typing import Dict, Optional from ldclient.evaluation import EvaluationDetail from ldclient.hook import EvaluationSeriesContext @@ -9,6 +10,7 @@ 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 @dataclass @@ -39,11 +41,32 @@ 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. + + SDK versions which report the environment ID to hooks do so automatically, + so this option is only required for SDK versions which do not. When both + are available, this option takes precedence. + + The value must be a non-empty string. Any other value is ignored, which is + equivalent to not specifying an environment ID at all. + """ + + +def _valid_environment_id(environment_id: Optional[str]) -> Optional[str]: + if isinstance(environment_id, str) and environment_id != '': + return environment_id + + return None + class Hook(LDHook): def __init__(self, options: HookOptions = HookOptions()): self.__tracer = trace.get_tracer_provider().get_tracer("launchdarkly") self.__options = options + self.__environment_id = _valid_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. " @@ -105,17 +128,21 @@ 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', } + environment_id = self.__environment_id or _valid_environment_id(getattr(series_context, 'environment_id', None)) + if environment_id is not None: + attributes['feature_flag.set.id'] = 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)