From a38f5ae975a95fe78ba8bdc372667dbe0043dd90 Mon Sep 17 00:00:00 2001 From: thiyagu06 Date: Sun, 18 Dec 2022 16:02:14 +0530 Subject: [PATCH 1/3] feat!: update otel hook to be spec compliant Signed-off-by: thiyagu06 --- hooks/README.md | 24 +++- hooks/open-telemetry/pom.xml | 23 ++++ .../contrib/hooks/otel/OpenTelemetryHook.java | 80 ++++++++++--- .../hooks/otel/OpenTelemetryHookTest.java | 113 +++++++++++++++++- pom.xml | 7 ++ 5 files changed, 225 insertions(+), 22 deletions(-) diff --git a/hooks/README.md b/hooks/README.md index e13d30199..270888b77 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -1,3 +1,25 @@ # OpenFeature Java Hooks -Hooks are a mechanism whereby application developers can add arbitrary behavior to flag evaluation. They operate similarly to middleware in many web frameworks. Please see the [spec](https://github.com/open-feature/spec/blob/main/specification/sections/04-hooks.md) for more details. +The OpenTelemetry hook for OpenFeature provides +a [spec compliant] (https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/feature-flags.md) +way to automatically add a feature flag +evaluation to a span as a span event. This can be used to determine the impact a feature has on a request, +enabling enhanced observability use cases, such as A/B testing or progressive feature releases. + +## Installation + +``` + dev.openfeature.contrib.hooks + otel + 0.4.0 +``` + +## Usage + +OpenFeature provider various ways to register hooks. The location that a hook is registered affects when the hook is +run. It's recommended to register the `OpenTelemetryHook` globally in most situations, but it's possible to only enable +the hook on specific clients. You should **never** register the `OpenTelemetryHook` globally and on a client. + +``` + OpenFeatureAPI.getInstance().addHooks(new OpenTelemetryHook()); +``` \ No newline at end of file diff --git a/hooks/open-telemetry/pom.xml b/hooks/open-telemetry/pom.xml index 2cfd9194b..1e121ffa5 100644 --- a/hooks/open-telemetry/pom.xml +++ b/hooks/open-telemetry/pom.xml @@ -26,6 +26,29 @@ + + com.fasterxml.jackson.core + jackson-databind + 2.14.1 + + + + io.opentelemetry + opentelemetry-sdk + + + + + + io.opentelemetry + opentelemetry-bom + 1.20.1 + pom + import + + + + \ No newline at end of file diff --git a/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java b/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java index 917bb37fd..59e83b9c5 100644 --- a/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java +++ b/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java @@ -1,29 +1,79 @@ package dev.openfeature.contrib.hooks.otel; -import dev.openfeature.sdk.Client; -import dev.openfeature.sdk.NoOpProvider; -import dev.openfeature.sdk.OpenFeatureAPI; +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.Hook; +import dev.openfeature.sdk.HookContext; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.trace.Span; +import lombok.SneakyThrows; -/** - * A placeholder. +import java.util.Map; + +/** + * The OpenTelemetry hook provides a way to automatically add a feature flag evaluation to a span as a span event. + * Refer to OpenTelemetry */ -public class OpenTelemetryHook { - - /** +public class OpenTelemetryHook implements Hook { + + private final AttributeKey flagKeyAttributeKey = AttributeKey.stringKey("feature_flag.flag_key"); + + private final AttributeKey providerNameAttributeKey = AttributeKey.stringKey("feature_flag.provider_name"); + + private final AttributeKey variantAttributeKey = AttributeKey.stringKey("feature_flag.variant"); + + private static final String EVENT_NAME = "feature_flag"; + + private final ObjectMapper objectMapper = new ObjectMapper(); + + /** * Create a new OpenTelemetryHook instance. */ public OpenTelemetryHook() { } - /** - * A test method... + /** + * Records the event in the current span after the successful flag evaluation. + * + * @param ctx Information about the particular flag evaluation + * @param details Information about how the flag was resolved, including any resolved values. + * @param hints An immutable mapping of data for users to communicate to the hooks. + */ + @SneakyThrows + @Override + public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { + Span currentSpan = Span.current(); + String value = details.getValue() != null ? getValue(details.getValue()) : ""; + if (currentSpan != null) { + Attributes attributes = Attributes.of( + flagKeyAttributeKey, ctx.getFlagKey(), + providerNameAttributeKey, ctx.getProviderMetadata().getName(), + variantAttributeKey, value); + currentSpan.addEvent(EVENT_NAME, attributes); + } + } + + /** + * Records the error details in the current span after the flag evaluation has processed abnormally. * - * @return {boolean} + * @param ctx Information about the particular flag evaluation + * @param error The exception that was thrown. + * @param hints An immutable mapping of data for users to communicate to the hooks. */ - public static boolean test() { - OpenFeatureAPI.getInstance().setProvider(new NoOpProvider()); - Client client = OpenFeatureAPI.getInstance().getClient(); - return client.getBooleanValue("test2", true); + @Override + public void error(HookContext ctx, Exception error, Map hints) { + Span currentSpan = Span.current(); + if (currentSpan != null) { + Attributes attributes = Attributes.of( + flagKeyAttributeKey, ctx.getFlagKey(), + providerNameAttributeKey, ctx.getProviderMetadata().getName()); + currentSpan.recordException(error, attributes); + } } + @SneakyThrows + private String getValue(Object value) { + return objectMapper.writeValueAsString(value); + } } diff --git a/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java b/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java index 339128e4b..c87c922c7 100644 --- a/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java +++ b/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java @@ -1,15 +1,116 @@ package dev.openfeature.contrib.hooks.otel; -import static org.assertj.core.api.Assertions.assertThat; - +import dev.openfeature.sdk.FlagEvaluationDetails; +import dev.openfeature.sdk.FlagValueType; +import dev.openfeature.sdk.HookContext; +import dev.openfeature.sdk.MutableContext; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.api.trace.Span; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; +import org.mockito.internal.matchers.Any; +import org.mockito.junit.jupiter.MockitoExtension; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +@ExtendWith(MockitoExtension.class) class OpenTelemetryHookTest { + private OpenTelemetryHook openTelemetryHook = new OpenTelemetryHook(); + + private final AttributeKey flagKeyAttributeKey = AttributeKey.stringKey("feature_flag.flag_key"); + + private final AttributeKey providerNameAttributeKey = AttributeKey.stringKey("feature_flag.provider_name"); + + private final AttributeKey variantAttributeKey = AttributeKey.stringKey("feature_flag.variant"); + + private static MockedStatic mockedSpan; + + @Mock private Span span; + + private HookContext hookContext = HookContext.builder() + .flagKey("test_key") + .type(FlagValueType.STRING) + .providerMetadata(() -> "test provider") + .ctx(new MutableContext()) + .defaultValue("default") + .build(); + + @BeforeAll + public static void init() { + mockedSpan = mockStatic(Span.class); + } + + @AfterAll + public static void close() { + mockedSpan.close(); + } + + @Test + @DisplayName("should add an event in span during after method execution") + void should_add_event_in_span_during_after_method_execution() { + FlagEvaluationDetails details = FlagEvaluationDetails.builder() + .variant("test_variant") + .value("variant passed") + .build(); + mockedSpan.when(Span::current).thenReturn(span); + openTelemetryHook.after(hookContext, details, null); + Attributes expectedAttr = Attributes.of(flagKeyAttributeKey, "test_key", + providerNameAttributeKey, "test provider", + variantAttributeKey, "\"variant passed\""); + verify(span).addEvent("feature_flag", expectedAttr); + } + @Test - @DisplayName("a simple test.") - void test() { - assertThat(OpenTelemetryHook.test()).isEqualTo(true); + @DisplayName("should not call addEvent because there is no active span") + void should_not_call_add_event_when_no_active_span() { + HookContext hookContext = HookContext.builder() + .flagKey("test_key") + .type(FlagValueType.STRING) + .providerMetadata(() -> "test provider") + .ctx(new MutableContext()) + .defaultValue("default") + .build(); + FlagEvaluationDetails details = FlagEvaluationDetails.builder() + .variant(null) + .value("variant passed") + .build(); + mockedSpan.when(Span::current).thenReturn(null); + openTelemetryHook.after(hookContext, details, null); + verifyNoInteractions(span); } -} + + @Test + @DisplayName("should record an exception in span during error method execution") + void should_record_exception_in_span_during_error_method_execution() { + RuntimeException runtimeException = new RuntimeException("could not resolve the flag"); + mockedSpan.when(Span::current).thenReturn(span); + openTelemetryHook.error(hookContext, runtimeException, null); + Attributes expectedAttr = Attributes.of(flagKeyAttributeKey, "test_key", + providerNameAttributeKey, "test provider"); + verify(span).recordException(runtimeException, expectedAttr); + } + + @Test + @DisplayName("should not call recordException because there is no active span") + void should_not_call_record_exception_when_no_active_span() { + RuntimeException runtimeException = new RuntimeException("could not resolve the flag"); + mockedSpan.when(Span::current).thenReturn(null); + openTelemetryHook.error(hookContext, runtimeException, null); + verifyNoInteractions(span); + } + +} \ No newline at end of file diff --git a/pom.xml b/pom.xml index caacd7225..64a946c6c 100644 --- a/pom.xml +++ b/pom.xml @@ -131,6 +131,13 @@ test + + org.mockito + mockito-junit-jupiter + 4.10.0 + test + + org.mockito mockito-inline From 9858e9c20ede3edeeb636b866f3612ddfe302d20 Mon Sep 17 00:00:00 2001 From: thiyagu06 Date: Thu, 22 Dec 2022 19:54:01 +0530 Subject: [PATCH 2/3] removed jackson dependency and fixed review comments on adding version info in README.md Signed-off-by: thiyagu06 --- hooks/README.md | 13 ++++++++----- hooks/open-telemetry/pom.xml | 6 ------ .../contrib/hooks/otel/OpenTelemetryHook.java | 12 +----------- .../contrib/hooks/otel/OpenTelemetryHookTest.java | 6 +++--- release-please-config.json | 3 ++- 5 files changed, 14 insertions(+), 26 deletions(-) diff --git a/hooks/README.md b/hooks/README.md index 270888b77..2c8af2117 100644 --- a/hooks/README.md +++ b/hooks/README.md @@ -7,12 +7,15 @@ evaluation to a span as a span event. This can be used to determine the impact a enabling enhanced observability use cases, such as A/B testing or progressive feature releases. ## Installation - -``` - dev.openfeature.contrib.hooks - otel - 0.4.0 + +```xml + + dev.openfeature.contrib.hooks + otel + 0.4.0 + ``` + ## Usage diff --git a/hooks/open-telemetry/pom.xml b/hooks/open-telemetry/pom.xml index 1e121ffa5..eec100e4a 100644 --- a/hooks/open-telemetry/pom.xml +++ b/hooks/open-telemetry/pom.xml @@ -26,12 +26,6 @@ - - com.fasterxml.jackson.core - jackson-databind - 2.14.1 - - io.opentelemetry opentelemetry-sdk diff --git a/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java b/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java index 59e83b9c5..c66a361ff 100644 --- a/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java +++ b/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java @@ -1,13 +1,11 @@ package dev.openfeature.contrib.hooks.otel; -import com.fasterxml.jackson.databind.ObjectMapper; import dev.openfeature.sdk.FlagEvaluationDetails; import dev.openfeature.sdk.Hook; import dev.openfeature.sdk.HookContext; import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.trace.Span; -import lombok.SneakyThrows; import java.util.Map; @@ -25,8 +23,6 @@ public class OpenTelemetryHook implements Hook { private static final String EVENT_NAME = "feature_flag"; - private final ObjectMapper objectMapper = new ObjectMapper(); - /** * Create a new OpenTelemetryHook instance. */ @@ -40,11 +36,10 @@ public OpenTelemetryHook() { * @param details Information about how the flag was resolved, including any resolved values. * @param hints An immutable mapping of data for users to communicate to the hooks. */ - @SneakyThrows @Override public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { Span currentSpan = Span.current(); - String value = details.getValue() != null ? getValue(details.getValue()) : ""; + String value = details.getValue() != null ? details.getValue().toString() : ""; if (currentSpan != null) { Attributes attributes = Attributes.of( flagKeyAttributeKey, ctx.getFlagKey(), @@ -71,9 +66,4 @@ public void error(HookContext ctx, Exception error, Map hints) { currentSpan.recordException(error, attributes); } } - - @SneakyThrows - private String getValue(Object value) { - return objectMapper.writeValueAsString(value); - } } diff --git a/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java b/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java index c87c922c7..24064cbbc 100644 --- a/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java +++ b/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java @@ -64,13 +64,13 @@ public static void close() { void should_add_event_in_span_during_after_method_execution() { FlagEvaluationDetails details = FlagEvaluationDetails.builder() .variant("test_variant") - .value("variant passed") + .value("variant_value") .build(); mockedSpan.when(Span::current).thenReturn(span); openTelemetryHook.after(hookContext, details, null); Attributes expectedAttr = Attributes.of(flagKeyAttributeKey, "test_key", providerNameAttributeKey, "test provider", - variantAttributeKey, "\"variant passed\""); + variantAttributeKey, "variant_value"); verify(span).addEvent("feature_flag", expectedAttr); } @@ -86,7 +86,7 @@ void should_not_call_add_event_when_no_active_span() { .build(); FlagEvaluationDetails details = FlagEvaluationDetails.builder() .variant(null) - .value("variant passed") + .value("variant_value") .build(); mockedSpan.when(Span::current).thenReturn(null); openTelemetryHook.after(hookContext, details, null); diff --git a/release-please-config.json b/release-please-config.json index 4f7efc30d..6aba5d1e0 100644 --- a/release-please-config.json +++ b/release-please-config.json @@ -29,7 +29,8 @@ "bump-patch-for-minor-pre-major": true, "versioning": "default", "extra-files": [ - "pom.xml" + "pom.xml", + "README.md" ] } } From 9b82ca6c17e10f3c8d485f7d93fd2932d5da17da Mon Sep 17 00:00:00 2001 From: thiyagu06 Date: Thu, 22 Dec 2022 20:25:45 +0530 Subject: [PATCH 3/3] use variant value in the span attribute. Signed-off-by: thiyagu06 --- .../contrib/hooks/otel/OpenTelemetryHook.java | 16 ++++++++-------- .../hooks/otel/OpenTelemetryHookTest.java | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java b/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java index c66a361ff..fe82b7fba 100644 --- a/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java +++ b/hooks/open-telemetry/src/main/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHook.java @@ -15,13 +15,13 @@ */ public class OpenTelemetryHook implements Hook { - private final AttributeKey flagKeyAttributeKey = AttributeKey.stringKey("feature_flag.flag_key"); + private static final String EVENT_NAME = "feature_flag"; - private final AttributeKey providerNameAttributeKey = AttributeKey.stringKey("feature_flag.provider_name"); + private final AttributeKey flagKeyAttributeKey = AttributeKey.stringKey(EVENT_NAME + ".flag_key"); - private final AttributeKey variantAttributeKey = AttributeKey.stringKey("feature_flag.variant"); + private final AttributeKey providerNameAttributeKey = AttributeKey.stringKey(EVENT_NAME + ".provider_name"); - private static final String EVENT_NAME = "feature_flag"; + private final AttributeKey variantAttributeKey = AttributeKey.stringKey(EVENT_NAME + ".variant"); /** * Create a new OpenTelemetryHook instance. @@ -32,19 +32,19 @@ public OpenTelemetryHook() { /** * Records the event in the current span after the successful flag evaluation. * - * @param ctx Information about the particular flag evaluation + * @param ctx Information about the particular flag evaluation * @param details Information about how the flag was resolved, including any resolved values. - * @param hints An immutable mapping of data for users to communicate to the hooks. + * @param hints An immutable mapping of data for users to communicate to the hooks. */ @Override public void after(HookContext ctx, FlagEvaluationDetails details, Map hints) { Span currentSpan = Span.current(); - String value = details.getValue() != null ? details.getValue().toString() : ""; if (currentSpan != null) { + String variant = details.getVariant() != null ? details.getVariant() : String.valueOf(details.getValue()); Attributes attributes = Attributes.of( flagKeyAttributeKey, ctx.getFlagKey(), providerNameAttributeKey, ctx.getProviderMetadata().getName(), - variantAttributeKey, value); + variantAttributeKey, variant); currentSpan.addEvent(EVENT_NAME, attributes); } } diff --git a/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java b/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java index 24064cbbc..4792f2c32 100644 --- a/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java +++ b/hooks/open-telemetry/src/test/java/dev/openfeature/contrib/hooks/otel/OpenTelemetryHookTest.java @@ -68,6 +68,20 @@ void should_add_event_in_span_during_after_method_execution() { .build(); mockedSpan.when(Span::current).thenReturn(span); openTelemetryHook.after(hookContext, details, null); + Attributes expectedAttr = Attributes.of(flagKeyAttributeKey, "test_key", + providerNameAttributeKey, "test provider", + variantAttributeKey, "test_variant"); + verify(span).addEvent("feature_flag", expectedAttr); + } + + @Test + @DisplayName("attribute should fallback to value field when variant is null") + void attribute_should_fallback_to_value_field_when_variant_is_null() { + FlagEvaluationDetails details = FlagEvaluationDetails.builder() + .value("variant_value") + .build(); + mockedSpan.when(Span::current).thenReturn(span); + openTelemetryHook.after(hookContext, details, null); Attributes expectedAttr = Attributes.of(flagKeyAttributeKey, "test_key", providerNameAttributeKey, "test provider", variantAttributeKey, "variant_value");