fix: Align TracingHook with spec (#342)

* refactor: move after logic to finally_after so evaluation span is created on failure

Signed-off-by: Danju Visvanathan <danju.visvanathan@gmail.com>

* feat: optionally disable exceptions on evaluation errors

Signed-off-by: Danju Visvanathan <danju.visvanathan@gmail.com>

* fix: add provider metadata to hook exception

Signed-off-by: Danju Visvanathan <danju.visvanathan@gmail.com>

* chore: given/when/then comments

Signed-off-by: Danju Visvanathan <danju.visvanathan@gmail.com>

* fixup: formatting

Signed-off-by: Todd Baert <todd.baert@dynatrace.com>

---------

Signed-off-by: Danju Visvanathan <danju.visvanathan@gmail.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>

Author: danjuv

hooks/openfeature-hooks-opentelemetry/src/openfeature/contrib/hook/opentelemetry/__init__.py

@@ -21,7 +21,10 @@ class EventAttributes:
 
 
 class TracingHook(Hook):
-    def after(
+    def __init__(self, exclude_exceptions: bool = False):
+        self.exclude_exceptions = exclude_exceptions
+
+    def finally_after(
         self,
         hook_context: HookContext,
         details: FlagEvaluationDetails,
@@ -60,5 +63,15 @@ def after(
     def error(
         self, hook_context: HookContext, exception: Exception, hints: HookHints
     ) -> None:
+        if self.exclude_exceptions:
+            return
+        attributes = {
+            EventAttributes.KEY: hook_context.flag_key,
+            EventAttributes.RESULT_VALUE: json.dumps(hook_context.default_value),
+        }
+        if hook_context.provider_metadata:
+            attributes[EventAttributes.PROVIDER_NAME] = (
+                hook_context.provider_metadata.name
+            )
         current_span = trace.get_current_span()
-        current_span.record_exception(exception)
+        current_span.record_exception(exception, attributes)

hooks/openfeature-hooks-opentelemetry/tests/test_otel.py

@@ -17,7 +17,7 @@ def mock_get_current_span(monkeypatch):
     monkeypatch.setattr(trace, "get_current_span", Mock())
 
 
-def test_after(mock_get_current_span):
+def test_finally_after(mock_get_current_span):
     # Given
     hook = TracingHook()
     hook_context = HookContext(
@@ -40,7 +40,7 @@ def test_after(mock_get_current_span):
     trace.get_current_span.return_value = mock_span
 
     # When
-    hook.after(hook_context, details, hints={})
+    hook.finally_after(hook_context, details, hints={})
 
     # Then
     mock_span.add_event.assert_called_once_with(
@@ -79,7 +79,7 @@ def test_after_evaluation_error(mock_get_current_span):
     trace.get_current_span.return_value = mock_span
 
     # When
-    hook.after(hook_context, details, hints={})
+    hook.finally_after(hook_context, details, hints={})
 
     # Then
     mock_span.add_event.assert_called_once_with(
@@ -97,6 +97,33 @@ def test_after_evaluation_error(mock_get_current_span):
 def test_error(mock_get_current_span):
     # Given
     hook = TracingHook()
+    hook_context = HookContext(
+        flag_key="flag_key",
+        flag_type=FlagType.BOOLEAN,
+        default_value=False,
+        evaluation_context=EvaluationContext(),
+        provider_metadata=Metadata(name="test-provider"),
+    )
+    exception = Exception()
+    attributes = {
+        "feature_flag.key": "flag_key",
+        "feature_flag.result.value": "false",
+        "feature_flag.provider.name": "test-provider",
+    }
+
+    mock_span = Mock(spec=Span)
+    trace.get_current_span.return_value = mock_span
+
+    # When
+    hook.error(hook_context, exception, hints={})
+
+    # Then
+    mock_span.record_exception.assert_called_once_with(exception, attributes)
+
+
+def test_error_exclude_exceptions(mock_get_current_span):
+    # Given
+    hook = TracingHook(exclude_exceptions=True)
     hook_context = HookContext(
         flag_key="flag_key",
         flag_type=FlagType.BOOLEAN,
@@ -110,6 +137,29 @@ def test_error(mock_get_current_span):
 
     # When
     hook.error(hook_context, exception, hints={})
+    # Then
+    mock_span.record_exception.assert_not_called()
+
+
+def test_error_no_provider_metadata(mock_get_current_span):
+    # Given
+    hook = TracingHook()
+    hook_context = HookContext(
+        flag_key="flag_key",
+        flag_type=FlagType.BOOLEAN,
+        default_value=False,
+        evaluation_context=EvaluationContext(),
+    )
+    exception = Exception()
+    attributes = {
+        "feature_flag.key": "flag_key",
+        "feature_flag.result.value": "false",
+    }
 
+    mock_span = Mock(spec=Span)
+    trace.get_current_span.return_value = mock_span
+
+    # When
+    hook.error(hook_context, exception, hints={})
     # Then
-    mock_span.record_exception.assert_called_once_with(exception)
+    mock_span.record_exception.assert_called_once_with(exception, attributes)