feat: add user facing impact metrics #342

sjaanus · 2025-12-19T13:42:47Z

Add user facing impact metrics methods.

sjaanus · 2025-12-19T13:43:30Z

src/main/java/io/getunleash/DefaultUnleash.java

+    private Variant resolveVariant(
+            String toggleName, UnleashContext enhancedContext, Variant defaultValue) {
+        Optional<FlatResponse<VariantDef>> response =
+                Optional.ofNullable(this.featureRepository.getVariant(toggleName, enhancedContext));
+        Optional<VariantDef> variantDef = response.map(r -> r.value);
+        return YggdrasilAdapters.adapt(variantDef, defaultValue);
+    }


We need private resolve variant method that does not dispatch impression events.

sjaanus · 2025-12-19T13:45:56Z

src/main/java/io/getunleash/impactmetrics/ImpactMetricRegistryAndDataSource.java

@@ -0,0 +1,4 @@
+package io.getunleash.impactmetrics;
+
+public interface ImpactMetricRegistryAndDataSource


We needed type definition that includes ImpactMetricRegistry and ImpactMetricsDataSource, since in Java we have config that accepts this. This is new to Java, not in Node.

sjaanus · 2025-12-19T13:53:33Z

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

+
+    public void defineCounter(String name, String help) {
+        if (name == null || name.isEmpty() || help == null || help.isEmpty()) {
+            LOGGER.warn("Counter name or help cannot be empty: {}, {}", name, help);


We do not have similar event system, like we have in node. We have more about SDK lifecycle, FeatureEvaluated or UnleashReady. In Java we just use logger.

sjaanus · 2025-12-19T13:54:54Z

src/test/java/io/getunleash/impactmetrics/MetricsAPITest.java

+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+
+public class MetricsAPITest {


These tests are 1 to 1 taken from metric-client test in Node. The naming in Node is poor, because the metric-client is actually deprecated, so I kept it as MetricsAPI test as this is more correct.

Copilot

Pull request overview

This PR adds user-facing impact metrics functionality to the Unleash SDK, enabling users to define and track counters, gauges, and histograms associated with feature flags.

Introduces a new MetricsAPI class that provides methods to define and update metrics (counters, gauges, histograms)
Adds supporting classes for metric context (StaticContext, MetricFlagContext, VariantResolver)
Refactors the impact metrics type hierarchy by introducing ImpactMetricRegistryAndDataSource interface

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/main/java/io/getunleash/impactmetrics/MetricsAPI.java	New API class for defining and updating user-facing metrics with feature flag context
src/main/java/io/getunleash/impactmetrics/StaticContext.java	New value object holding static application context (app name, environment)
src/main/java/io/getunleash/impactmetrics/MetricFlagContext.java	New class for passing feature flag context to metric operations
src/main/java/io/getunleash/impactmetrics/VariantResolver.java	New interface for resolving variant information for feature flags
src/main/java/io/getunleash/impactmetrics/ImpactMetricRegistryAndDataSource.java	New composite interface combining ImpactMetricRegistry and ImpactMetricsDataSource
src/main/java/io/getunleash/impactmetrics/ImpactMetricRegistry.java	Extended with getter methods for retrieving registered metrics by name
src/main/java/io/getunleash/impactmetrics/InMemoryMetricRegistry.java	Updated to implement new composite interface and getter methods
src/main/java/io/getunleash/DefaultUnleash.java	Integrates MetricsAPI and refactors variant resolution logic
src/main/java/io/getunleash/util/UnleashConfig.java	Updated type references from ImpactMetricsDataSource to ImpactMetricRegistryAndDataSource
src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java	Updated type references from ImpactMetricsDataSource to ImpactMetricRegistryAndDataSource
src/test/java/io/getunleash/impactmetrics/MetricsAPITest.java	Comprehensive test suite for MetricsAPI functionality
src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java	Updated mock types to use new ImpactMetricRegistryAndDataSource interface

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-19T14:14:22Z

src/main/java/io/getunleash/DefaultUnleash.java

        Optional<FlatResponse<VariantDef>> response =
                Optional.ofNullable(this.featureRepository.getVariant(toggleName, enhancedContext));


The variant is being resolved twice for the same toggle and context. Line 155 calls resolveVariant() which internally calls featureRepository.getVariant(), and then line 158 calls it again. The second call is only used to check for impression data. This duplicate call could be avoided by modifying resolveVariant() to return both the variant and the response object, or by restructuring the code to reuse the response from the first call.

Copilot · 2025-12-19T14:14:22Z

src/main/java/io/getunleash/DefaultUnleash.java

+    public final MetricsAPI impactMetrics;



The impactMetrics field is declared as public, which exposes internal implementation details and breaks encapsulation. Consider making this field private and providing a public getter method instead, or exposing it through the Unleash interface if it's intended to be part of the public API.

Suggested change

public final MetricsAPI impactMetrics;

private final MetricsAPI impactMetrics;

public MetricsAPI getImpactMetrics() {

return impactMetrics;

}

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

coveralls · 2025-12-19T15:35:48Z

Pull Request Test Coverage Report for Build 20457737290

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

116 of 131 (88.55%) changed or added relevant lines in 7 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+1.0%) to 80.178%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/main/java/io/getunleash/impactmetrics/MetricsAPI.java	60	75	80.0%

Totals
Change from base Build 20339792970:	1.0%
Covered Lines:	3313
Relevant Lines:	3978

💛 - Coveralls

sjaanus · 2025-12-22T13:36:50Z

src/main/java/io/getunleash/impactmetrics/VariantResolver.java

+import io.getunleash.variant.Variant;
+
+public interface VariantResolver {
+    Variant forceGetVariant(String flagName, UnleashContext context);


getVariantForImpactMetrics

sjaanus · 2025-12-22T13:37:13Z

src/main/java/io/getunleash/DefaultUnleash.java

        return variant;
    }

+    private Variant getVariantForMetrics(String toggleName, UnleashContext context) {


getVariantForImpactMetrics

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

src/main/java/io/getunleash/impactmetrics/ImpactMetricRegistry.java

src/main/java/io/getunleash/impactmetrics/ImpactMetricRegistryAndDataSource.java

sjaanus · 2025-12-23T08:50:31Z

src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java

    @Test
    public void should_not_include_impact_metrics_field_when_empty() throws YggdrasilError {
-        ImpactMetricsDataSource registry = mock(ImpactMetricsDataSource.class);
+        ImpactMetricRegistryAndDataSource registry = mock(ImpactMetricRegistryAndDataSource.class);


Check if we can use these together. Registry might be always datasource.

kwasniew

Pair reviewed

Copilot

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 10 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-23T09:04:52Z

src/main/java/io/getunleash/Unleash.java

    default void shutdown() {}

    MoreOperations more();
+


The new public method getImpactMetrics() lacks documentation. This is a user-facing API method that should include JavaDoc explaining its purpose, what MetricsAPI provides, and how it should be used. This is especially important as this is a new feature being introduced.

Suggested change

/**

* Returns the impact metrics API associated with this {@link Unleash} instance.

* <p>

* The {@link MetricsAPI} provides access to functionality for working with impact metrics

* related to feature toggle evaluations, such as recording or retrieving metrics data

* produced by this client.

* </p>

* <p>

* Typical usage is to obtain a reference from an {@code Unleash} instance and then use the

* returned {@link MetricsAPI} to integrate impact metrics with your own monitoring,

* reporting, or analysis tooling.

* </p>

*

* @return the {@link MetricsAPI} for interacting with impact metrics.

*/

Copilot · 2025-12-23T09:04:52Z

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

+        List<String> labelNames = List.of("featureName", "appName", "environment");
+        metricRegistry.gauge(new MetricOptions(name, help, labelNames));


The label names definition uses "featureName" as a single label, but the actual labels map uses individual feature flag names as keys (e.g., "featureX", "featureY" from tests). This creates a mismatch between the declared label schema and the actual labels being used. The label names should either match the actual feature flag names being used, or the implementation should use a consistent "featureName" label key with the flag name as the value.

Copilot · 2025-12-23T09:04:52Z

src/main/java/io/getunleash/DefaultUnleash.java

+    @Override
+    public MetricsAPI getImpactMetrics() {
+        return impactMetrics;
+    }


The new getImpactMetrics() method lacks test coverage. Since comprehensive tests exist for other methods in this file (e.g., more() method is tested), the new user-facing API should also have tests to verify it returns a properly initialized MetricsAPI instance.

Copilot · 2025-12-23T09:04:53Z

src/main/java/io/getunleash/impactmetrics/StaticContext.java

+public class StaticContext {
+    private final String appName;
+    private final String environment;
+
+    public StaticContext(String appName, String environment) {
+        this.appName = appName;
+        this.environment = environment;


The constructor does not validate that appName and environment are non-null. Since these values are used in MetricsAPI to populate labels and are retrieved via getter methods, null values could cause NullPointerException when used. Consider adding null checks or using @nullable annotations if null values are intentionally allowed.

Suggested change

public class StaticContext {

private final String appName;

private final String environment;

public StaticContext(String appName, String environment) {

this.appName = appName;

this.environment = environment;

import java.util.Objects;

public class StaticContext {

private final String appName;

private final String environment;

public StaticContext(String appName, String environment) {

this.appName = Objects.requireNonNull(appName, "appName must not be null");

this.environment = Objects.requireNonNull(environment, "environment must not be null");

Copilot · 2025-12-23T09:04:53Z

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

+    public void defineCounter(String name, String help) {
+        if (name == null || name.isEmpty() || help == null || help.isEmpty()) {
+            LOGGER.warn("Counter name or help cannot be empty: {}, {}", name, help);
+            return;
+        }
+        List<String> labelNames = List.of("featureName", "appName", "environment");
+        metricRegistry.counter(new MetricOptions(name, help, labelNames));
+    }
+
+    public void defineGauge(String name, String help) {
+        if (name == null || name.isEmpty() || help == null || help.isEmpty()) {
+            LOGGER.warn("Gauge name or help cannot be empty: {}, {}", name, help);
+            return;
+        }
+        List<String> labelNames = List.of("featureName", "appName", "environment");
+        metricRegistry.gauge(new MetricOptions(name, help, labelNames));
+    }
+
+    public void defineHistogram(String name, String help, @Nullable List<Double> buckets) {
+        if (name == null || name.isEmpty() || help == null || help.isEmpty()) {
+            LOGGER.warn("Histogram name or help cannot be empty: {}, {}", name, help);
+            return;
+        }
+        List<String> labelNames = List.of("featureName", "appName", "environment");
+        List<Double> bucketList = buckets != null ? buckets : new ArrayList<>();
+        metricRegistry.histogram(new BucketMetricOptions(name, help, labelNames, bucketList));
+    }
+
+    private Map<String, String> getFlagLabels(@Nullable MetricFlagContext flagContext) {
+        Map<String, String> flagLabels = new HashMap<>();
+        if (flagContext != null) {
+            for (String flag : flagContext.getFlagNames()) {
+                Variant variant = variantResolver.forceGetVariant(flag, flagContext.getContext());
+
+                if (variant.isEnabled()) {
+                    flagLabels.put(flag, variant.getName());
+                } else if (variant.isFeatureEnabled()) {
+                    flagLabels.put(flag, "enabled");
+                } else {
+                    flagLabels.put(flag, "disabled");
+                }
+            }
+        }
+        return flagLabels;
+    }
+
+    public void incrementCounter(
+            String name, @Nullable Long value, @Nullable MetricFlagContext flagContext) {
+        Counter counter = metricRegistry.getCounter(name);
+        if (counter == null) {
+            LOGGER.warn("Counter {} not defined, this counter will not be incremented.", name);
+            return;
+        }
+
+        Map<String, String> flagLabels = getFlagLabels(flagContext);
+        Map<String, String> labels = new HashMap<>(flagLabels);
+        labels.put("appName", staticContext.getAppName());
+        labels.put("environment", staticContext.getEnvironment());
+
+        counter.inc(value != null ? value : 1L, labels);
+    }
+
+    public void updateGauge(String name, long value, @Nullable MetricFlagContext flagContext) {
+        Gauge gauge = metricRegistry.getGauge(name);
+        if (gauge == null) {
+            LOGGER.warn("Gauge {} not defined, this gauge will not be updated.", name);
+            return;
+        }
+
+        Map<String, String> flagLabels = getFlagLabels(flagContext);
+        Map<String, String> labels = new HashMap<>(flagLabels);
+        labels.put("appName", staticContext.getAppName());
+        labels.put("environment", staticContext.getEnvironment());
+
+        gauge.set(value, labels);
+    }
+
+    public void observeHistogram(
+            String name, double value, @Nullable MetricFlagContext flagContext) {
+        Histogram histogram = metricRegistry.getHistogram(name);
+        if (histogram == null) {
+            LOGGER.warn("Histogram {} not defined, this histogram will not be updated.", name);
+            return;
+        }
+
+        Map<String, String> flagLabels = getFlagLabels(flagContext);
+        Map<String, String> labels = new HashMap<>(flagLabels);
+        labels.put("appName", staticContext.getAppName());
+        labels.put("environment", staticContext.getEnvironment());
+
+        histogram.observe(value, labels);
+    }


The public methods defineCounter, defineGauge, defineHistogram, incrementCounter, updateGauge, and observeHistogram lack JavaDoc documentation. These are user-facing API methods that should document their purpose, parameters, behavior when metrics are not defined, and any side effects or warnings.

Copilot · 2025-12-23T09:04:53Z

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

+        List<String> labelNames = List.of("featureName", "appName", "environment");
+        metricRegistry.counter(new MetricOptions(name, help, labelNames));


The label names definition uses "featureName" as a single label, but the actual labels map uses individual feature flag names as keys (e.g., "featureX", "featureY" from tests). This creates a mismatch between the declared label schema and the actual labels being used. The label names should either match the actual feature flag names being used, or the implementation should use a consistent "featureName" label key with the flag name as the value.

Copilot · 2025-12-23T09:04:54Z

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

+        List<String> labelNames = List.of("featureName", "appName", "environment");
+        List<Double> bucketList = buckets != null ? buckets : new ArrayList<>();


The label names definition uses "featureName" as a single label, but the actual labels map uses individual feature flag names as keys (e.g., "featureX", "featureY" from tests). This creates a mismatch between the declared label schema and the actual labels being used. The label names should either match the actual feature flag names being used, or the implementation should use a consistent "featureName" label key with the flag name as the value.

Copilot · 2025-12-23T09:04:54Z

src/main/java/io/getunleash/FakeUnleash.java

+    @Override
+    public MetricsAPI getImpactMetrics() {
+        return impactMetrics;
+    }


The new getImpactMetrics() method lacks test coverage. Since comprehensive tests exist for other methods in this file (e.g., more() method is tested), the new user-facing API should also have tests to verify it returns a properly initialized MetricsAPI instance with the correct configuration.

Copilot · 2025-12-23T09:04:54Z

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java

+                Variant variant = variantResolver.forceGetVariant(flag, flagContext.getContext());
+
+                if (variant.isEnabled()) {
+                    flagLabels.put(flag, variant.getName());
+                } else if (variant.isFeatureEnabled()) {
+                    flagLabels.put(flag, "enabled");
+                } else {
+                    flagLabels.put(flag, "disabled");
+                }


The variantResolver.forceGetVariant() call does not check if the returned Variant is null before calling methods on it. While the current implementations seem to always return a non-null Variant, the interface contract doesn't explicitly guarantee this. Consider adding a null check or annotating the VariantResolver interface method with @nonnull to make the contract explicit.

Copilot · 2025-12-23T09:04:54Z

src/main/java/io/getunleash/impactmetrics/MetricFlagContext.java

+
+public class MetricFlagContext {
+    private final List<String> flagNames;
+    private final UnleashContext context;
+
+    public MetricFlagContext(List<String> flagNames, UnleashContext context) {
+        this.flagNames = flagNames;
+        this.context = context;


The constructor does not validate that flagNames and context are non-null. Since both fields are accessed without null checks in MetricsAPI.getFlagLabels(), null values will cause NullPointerException. Consider adding validation or using @nullable annotations if null values are intentionally allowed.

Suggested change

public class MetricFlagContext {

private final List<String> flagNames;

private final UnleashContext context;

public MetricFlagContext(List<String> flagNames, UnleashContext context) {

this.flagNames = flagNames;

this.context = context;

import java.util.Objects;

public class MetricFlagContext {

private final List<String> flagNames;

private final UnleashContext context;

public MetricFlagContext(List<String> flagNames, UnleashContext context) {

this.flagNames = Objects.requireNonNull(flagNames, "flagNames must not be null");

this.context = Objects.requireNonNull(context, "context must not be null");

feat: add user facing impact metrics

1126ac3

sjaanus requested a review from chriswk December 19, 2025 13:42

thomasheartman added this to Issues and PRs Dec 19, 2025

github-project-automation bot moved this to New in Issues and PRs Dec 19, 2025

sjaanus commented Dec 19, 2025

View reviewed changes

sjaanus requested review from Copilot and kwasniew and removed request for chriswk and kwasniew December 19, 2025 14:09

Copilot started reviewing on behalf of sjaanus December 19, 2025 14:09 View session

Copilot AI reviewed Dec 19, 2025

View reviewed changes

sjaanus added 2 commits December 22, 2025 13:13

Fix double call to repo

c5f3148

Hide public api

6cac282

sjaanus commented Dec 22, 2025

View reviewed changes

sjaanus commented Dec 23, 2025

View reviewed changes

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java Show resolved Hide resolved

sjaanus commented Dec 23, 2025

View reviewed changes

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java Show resolved Hide resolved

sjaanus commented Dec 23, 2025

View reviewed changes

src/main/java/io/getunleash/impactmetrics/MetricsAPI.java Show resolved Hide resolved

sjaanus commented Dec 23, 2025

View reviewed changes

src/main/java/io/getunleash/impactmetrics/ImpactMetricRegistry.java Show resolved Hide resolved

sjaanus commented Dec 23, 2025

View reviewed changes

src/main/java/io/getunleash/impactmetrics/ImpactMetricRegistryAndDataSource.java Show resolved Hide resolved

sjaanus commented Dec 23, 2025

View reviewed changes

kwasniew requested a review from Copilot December 23, 2025 09:00

kwasniew self-assigned this Dec 23, 2025

kwasniew self-requested a review December 23, 2025 09:00

kwasniew approved these changes Dec 23, 2025

View reviewed changes

Copilot started reviewing on behalf of kwasniew December 23, 2025 09:00 View session

github-project-automation bot moved this from New to Approved PRs in Issues and PRs Dec 23, 2025

Copilot AI reviewed Dec 23, 2025

View reviewed changes

sjaanus added 2 commits December 23, 2025 11:42

Fix

26df7fd

Fix

9ff061b

sjaanus merged commit 04c42e3 into main Dec 23, 2025
9 checks passed

sjaanus deleted the metric-api branch December 23, 2025 10:12

github-project-automation bot moved this from Approved PRs to Done in Issues and PRs Dec 23, 2025

		@@ -0,0 +1,4 @@
		package io.getunleash.impactmetrics;

		public interface ImpactMetricRegistryAndDataSource

		Optional<FlatResponse<VariantDef>> response =
		Optional.ofNullable(this.featureRepository.getVariant(toggleName, enhancedContext));

+    /**
+     * Returns the impact metrics API associated with this {@link Unleash} instance.
+     * <p>
+     * The {@link MetricsAPI} provides access to functionality for working with impact metrics
+     * related to feature toggle evaluations, such as recording or retrieving metrics data
+     * produced by this client.
+     * </p>
+     * <p>
+     * Typical usage is to obtain a reference from an {@code Unleash} instance and then use the
+     * returned {@link MetricsAPI} to integrate impact metrics with your own monitoring,
+     * reporting, or analysis tooling.
+     * </p>
+     *
+     * @return the {@link MetricsAPI} for interacting with impact metrics.
+     */

		List<String> labelNames = List.of("featureName", "appName", "environment");
		metricRegistry.gauge(new MetricOptions(name, help, labelNames));

		List<String> labelNames = List.of("featureName", "appName", "environment");
		metricRegistry.counter(new MetricOptions(name, help, labelNames));

		List<String> labelNames = List.of("featureName", "appName", "environment");
		List<Double> bucketList = buckets != null ? buckets : new ArrayList<>();

feat: add user facing impact metrics #342

feat: add user facing impact metrics #342

Uh oh!

Conversation

sjaanus commented Dec 19, 2025

Uh oh!

sjaanus Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

sjaanus Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

sjaanus Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

sjaanus Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 20457737290

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

sjaanus Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

sjaanus Dec 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sjaanus Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

kwasniew left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

coveralls commented Dec 19, 2025 •

edited

Loading