fix: move extra attribute retrieval to after hook

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

Author: danjuv

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

@@ -54,6 +54,9 @@ def after(
             attributes[Attributes.OTEL_PROVIDER_NAME] = (
                 hook_context.provider_metadata.name
             )
+        attributes = attributes | attributes_from_dimensions(
+            self.extra_attributes, details.flag_metadata
+        )
         self.evaluation_success_total.add(1, attributes)
 
     def error(
@@ -82,9 +85,6 @@ def finally_after(
             attributes[Attributes.OTEL_PROVIDER_NAME] = (
                 hook_context.provider_metadata.name
             )
-        attributes = attributes | attributes_from_dimensions(
-            self.extra_attributes, details.flag_metadata
-        )
         self.evaluation_active_count.add(-1, attributes)
 
 

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

@@ -95,33 +95,42 @@ def test_metric_after(mock_get_meter):
     mock_counters["feature_flag.evaluation.active_count"].add.assert_not_called()
 
 
-def test_metric_error(mock_get_meter):
+def test_metric_after_with_extra_dimensions(mock_get_meter):
     _, mock_counters = mock_get_meter
-    hook = MetricsHook()
+    hook = MetricsHook(extra_attributes=["scope", "test"])
     hook_context = HookContext(
         flag_key="flag_key",
         flag_type=FlagType.BOOLEAN,
         default_value=False,
         evaluation_context=EvaluationContext(),
         provider_metadata=Metadata(name="test-provider"),
     )
-    hook.error(hook_context, Exception("test error"), hints={})
-    mock_counters["feature_flag.evaluation.error_total"].add.assert_called_once_with(
+    details = FlagEvaluationDetails(
+        flag_key="flag_key",
+        value=True,
+        variant="enabled",
+        reason=Reason.TARGETING_MATCH,
+        flag_metadata={"scope": "application", "test": True},
+        error_code=None,
+        error_message=None,
+    )
+    hook.after(hook_context, details, hints={})
+    mock_counters["feature_flag.evaluation.success_total"].add.assert_called_once_with(
         1,
         {
             "feature_flag.key": "flag_key",
+            "feature_flag.result.reason": "targeting_match",
+            "feature_flag.result.variant": "enabled",
             "feature_flag.provider.name": "test-provider",
-            "exception": "test error",
+            "scope": "application",
+            "test": True,
         },
     )
-    mock_counters["feature_flag.evaluation.success_total"].add.assert_not_called()
-    mock_counters["feature_flag.evaluation.request_total"].add.assert_not_called()
-    mock_counters["feature_flag.evaluation.active_count"].add.assert_not_called()
 
 
-def test_metric_finally_after(mock_get_meter):
+def test_metric_after_with_extra_dimensions_missing_attribute(mock_get_meter):
     _, mock_counters = mock_get_meter
-    hook = MetricsHook()
+    hook = MetricsHook(extra_attributes=["scope", "test"])
     hook_context = HookContext(
         flag_key="flag_key",
         flag_type=FlagType.BOOLEAN,
@@ -134,54 +143,48 @@ def test_metric_finally_after(mock_get_meter):
         value=True,
         variant="enabled",
         reason=Reason.TARGETING_MATCH,
-        error_code=None,
-        error_message=None,
+        flag_metadata={"test": True},
     )
-    hook.finally_after(hook_context, details, hints={})
-    mock_counters["feature_flag.evaluation.active_count"].add.assert_called_once_with(
-        -1,
+    hook.after(hook_context, details, hints={})
+    mock_counters["feature_flag.evaluation.success_total"].add.assert_called_once_with(
+        1,
         {
             "feature_flag.key": "flag_key",
+            "feature_flag.result.reason": "targeting_match",
+            "feature_flag.result.variant": "enabled",
             "feature_flag.provider.name": "test-provider",
+            "test": True,
         },
     )
-    mock_counters["feature_flag.evaluation.success_total"].add.assert_not_called()
-    mock_counters["feature_flag.evaluation.request_total"].add.assert_not_called()
-    mock_counters["feature_flag.evaluation.error_total"].add.assert_not_called()
 
 
-def test_metric_finally_after_with_extra_dimensions(mock_get_meter):
+def test_metric_error(mock_get_meter):
     _, mock_counters = mock_get_meter
-    hook = MetricsHook(extra_attributes=["scope", "test"])
+    hook = MetricsHook()
     hook_context = HookContext(
         flag_key="flag_key",
         flag_type=FlagType.BOOLEAN,
         default_value=False,
         evaluation_context=EvaluationContext(),
         provider_metadata=Metadata(name="test-provider"),
     )
-    details = FlagEvaluationDetails(
-        flag_key="flag_key",
-        value=True,
-        variant="enabled",
-        reason=Reason.TARGETING_MATCH,
-        flag_metadata={"scope": "application", "test": True},
-    )
-    hook.finally_after(hook_context, details, hints={})
-    mock_counters["feature_flag.evaluation.active_count"].add.assert_called_once_with(
-        -1,
+    hook.error(hook_context, Exception("test error"), hints={})
+    mock_counters["feature_flag.evaluation.error_total"].add.assert_called_once_with(
+        1,
         {
             "feature_flag.key": "flag_key",
             "feature_flag.provider.name": "test-provider",
-            "scope": "application",
-            "test": True,
+            "exception": "test error",
         },
     )
+    mock_counters["feature_flag.evaluation.success_total"].add.assert_not_called()
+    mock_counters["feature_flag.evaluation.request_total"].add.assert_not_called()
+    mock_counters["feature_flag.evaluation.active_count"].add.assert_not_called()
 
 
-def test_metric_finally_after_with_extra_dimensions_missing_attribute(mock_get_meter):
+def test_metric_finally_after(mock_get_meter):
     _, mock_counters = mock_get_meter
-    hook = MetricsHook(extra_attributes=["scope", "test"])
+    hook = MetricsHook()
     hook_context = HookContext(
         flag_key="flag_key",
         flag_type=FlagType.BOOLEAN,
@@ -194,14 +197,17 @@ def test_metric_finally_after_with_extra_dimensions_missing_attribute(mock_get_m
         value=True,
         variant="enabled",
         reason=Reason.TARGETING_MATCH,
-        flag_metadata={"test": True},
+        error_code=None,
+        error_message=None,
     )
     hook.finally_after(hook_context, details, hints={})
     mock_counters["feature_flag.evaluation.active_count"].add.assert_called_once_with(
         -1,
         {
             "feature_flag.key": "flag_key",
             "feature_flag.provider.name": "test-provider",
-            "test": True,
         },
     )
+    mock_counters["feature_flag.evaluation.success_total"].add.assert_not_called()
+    mock_counters["feature_flag.evaluation.request_total"].add.assert_not_called()
+    mock_counters["feature_flag.evaluation.error_total"].add.assert_not_called()