diff --git a/src/main/java/io/harness/cf/client/api/MetricsProcessor.java b/src/main/java/io/harness/cf/client/api/MetricsProcessor.java index 00546cba..90185b34 100644 --- a/src/main/java/io/harness/cf/client/api/MetricsProcessor.java +++ b/src/main/java/io/harness/cf/client/api/MetricsProcessor.java @@ -31,6 +31,9 @@ class MetricsProcessor extends AbstractScheduledService { private static final String SDK_LANGUAGE = "SDK_LANGUAGE"; private static final String SDK_VERSION = "SDK_VERSION"; + private static final Target globalTarget = + Target.builder().name(GLOBAL_TARGET_NAME).identifier(GLOBAL_TARGET).build(); + private final Connector connector; private final BaseConfig config; private final AtomicLongMap frequencyMap; @@ -70,11 +73,12 @@ public synchronized void pushToQueue(Target target, String featureName, Variatio uniqueTargetSet.add(target); - if (config.isGlobalTargetEnabled()) { - target.setIdentifier(GLOBAL_TARGET); + Target metricTarget = globalTarget; + if (!config.isGlobalTargetEnabled()) { + metricTarget = target; } - frequencyMap.incrementAndGet(new MetricEvent(featureName, target, variation)); + frequencyMap.incrementAndGet(new MetricEvent(featureName, metricTarget, variation)); } /** This method sends the metrics data to the analytics server and resets the cache */ @@ -117,10 +121,8 @@ protected Metrics prepareSummaryMetricsBody(Map data, Set(), new ArrayList<>()); final Map summaryMetricsData = new HashMap<>(); - addTargetData( - metrics, Target.builder().name(GLOBAL_TARGET_NAME).identifier(GLOBAL_TARGET).build()); - targets.forEach(target -> addTargetData(metrics, target)); + data.forEach( (target, count) -> summaryMetricsData.put( diff --git a/src/test/java/io/harness/cf/client/api/MetricsProcessorTest.java b/src/test/java/io/harness/cf/client/api/MetricsProcessorTest.java index b30e69e4..e8644cdb 100644 --- a/src/test/java/io/harness/cf/client/api/MetricsProcessorTest.java +++ b/src/test/java/io/harness/cf/client/api/MetricsProcessorTest.java @@ -1,22 +1,22 @@ package io.harness.cf.client.api; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.when; import com.google.common.collect.Maps; import io.harness.cf.client.connector.Connector; +import io.harness.cf.client.connector.ConnectorException; import io.harness.cf.client.dto.Target; -import io.harness.cf.model.FeatureConfig; -import io.harness.cf.model.Metrics; -import io.harness.cf.model.Variation; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; +import io.harness.cf.model.*; +import java.util.*; import java.util.concurrent.*; import lombok.NonNull; import lombok.extern.slf4j.Slf4j; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.*; @Slf4j @@ -141,7 +141,7 @@ public void onMetricsFailure() { } @Test - public void testPrepareSummaryMetricsBody() { + void testPrepareSummaryMetricsBody() { Target target = Target.builder().identifier("harness").build(); FeatureConfig feature = FeatureConfig.builder().feature("bool-flag").build(); Variation variation = Variation.builder().identifier("true").value("true").build(); @@ -150,12 +150,69 @@ public void testPrepareSummaryMetricsBody() { Set uniqueTargets = new HashSet<>(); uniqueTargets.add(target); - Map map = Maps.newHashMap(); - map.put(event, 6L); + Map freqMap = Maps.newHashMap(); + freqMap.put(event, 6L); - Metrics metrics = metricsProcessor.prepareSummaryMetricsBody(map, uniqueTargets); + Metrics metrics = metricsProcessor.prepareSummaryMetricsBody(freqMap, uniqueTargets); - assert metrics.getTargetData() != null; - assert metrics.getTargetData().get(1).getIdentifier().equals(target.getIdentifier()); + assertNotNull(metrics); + assertNotNull(metrics.getTargetData()); + assertNotNull(metrics.getMetricsData()); + + assertEquals(target.getIdentifier(), metrics.getTargetData().get(0).getIdentifier()); + assertEquals(6, metrics.getMetricsData().get(0).getCount()); + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void shouldPostCorrectMetrics_WhenGlobalTargetEnabledOrDisabled(boolean globalTargetEnabled) + throws ConnectorException { + final Connector mockConnector = Mockito.mock(Connector.class); + final BaseConfig mockConfig = Mockito.mock(BaseConfig.class); + final ArgumentCaptor metricsArgumentCaptor = ArgumentCaptor.forClass(Metrics.class); + + when(mockConfig.isGlobalTargetEnabled()).thenReturn(globalTargetEnabled); + when(mockConfig.getBufferSize()).thenReturn(10); + doNothing().when(mockConnector).postMetrics(metricsArgumentCaptor.capture()); + + final MetricsProcessor processor = + new MetricsProcessor(mockConnector, mockConfig, Mockito.mock(MetricsCallback.class)); + + final Target target = Target.builder().identifier("target123").build(); + final Variation variation = Variation.builder().identifier("true").value("true").build(); + processor.pushToQueue(target, "feature1", variation); + processor.pushToQueue(target, "feature1", variation); + processor.pushToQueue(target, "feature2", variation); + processor.pushToQueue(target, "feature2", variation); + processor.pushToQueue(target, "feature3", variation); + processor.pushToQueue(target, "feature3", variation); + processor.runOneIteration(); // Mimic scheduled job + + final Metrics sentMetrics = metricsArgumentCaptor.getValue(); + + assertNotNull(sentMetrics.getMetricsData()); + assertNotNull(sentMetrics.getTargetData()); + + assertEquals(1, sentMetrics.getTargetData().size()); + assertEquals("target123", sentMetrics.getTargetData().get(0).getIdentifier()); + + assertEquals(3, sentMetrics.getMetricsData().size()); + for (MetricsData md : sentMetrics.getMetricsData()) { + final Map map = keyValueArrayToMap(md.getAttributes()); + assertEquals(2, md.getCount()); + if (globalTargetEnabled) { + assertEquals("__global__cf_target", map.get("target")); + } else { + assertEquals("target123", map.get("target")); + } + } + } + + private Map keyValueArrayToMap(List keyValueList) { + final Map map = new HashMap<>(); + for (KeyValue kv : keyValueList) { + map.put(kv.getKey(), kv.getValue()); + } + return map; } }