Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/main/java/io/getunleash/metric/UnleashMetricServiceImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import io.getunleash.engine.MetricsBucket;
import io.getunleash.engine.UnleashEngine;
import io.getunleash.impactmetrics.CollectedMetric;
import io.getunleash.impactmetrics.ImpactMetricsDataSource;
import io.getunleash.util.Throttler;
import io.getunleash.util.UnleashConfig;
import io.getunleash.util.UnleashScheduledExecutor;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.util.List;
import java.util.Set;

public class UnleashMetricServiceImpl implements UnleashMetricService {
Expand All @@ -19,6 +22,8 @@ public class UnleashMetricServiceImpl implements UnleashMetricService {

private final Throttler throttler;

private final ImpactMetricsDataSource impactMetricsRegistry;

public UnleashMetricServiceImpl(
UnleashConfig unleashConfig, UnleashScheduledExecutor executor, UnleashEngine engine) {
this(
Expand All @@ -42,6 +47,7 @@ public UnleashMetricServiceImpl(
300,
unleashConfig.getUnleashURLs().getClientMetricsURL());
this.engine = engine;
this.impactMetricsRegistry = unleashConfig.getImpactMetricsRegistry();
long metricsInterval = unleashConfig.getSendMetricsInterval();

executor.setInterval(sendMetrics(), metricsInterval, metricsInterval);
Expand All @@ -59,13 +65,21 @@ private Runnable sendMetrics() {
if (throttler.performAction()) {
MetricsBucket bucket = this.engine.getMetrics();

ClientMetrics metrics = new ClientMetrics(unleashConfig, bucket);
List<CollectedMetric> impactMetrics = impactMetricsRegistry.collect();
List<CollectedMetric> impactMetricsOrNull =
impactMetrics.isEmpty() ? null : impactMetrics;

ClientMetrics metrics =
new ClientMetrics(unleashConfig, bucket, impactMetricsOrNull);
Comment on lines +69 to +73
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is design from node SDK, that if we do not have any metrics, we do not add the empty array to payload.

int statusCode = metricSender.sendMetrics(metrics);
if (statusCode >= 200 && statusCode < 400) {
throttler.decrementFailureCountAndResetSkips();
}
if (statusCode >= 400) {
throttler.handleHttpErrorCodes(statusCode);
if (impactMetricsOrNull != null) {
impactMetricsRegistry.restore(impactMetricsOrNull);
}
}

} else {
Expand Down
9 changes: 5 additions & 4 deletions src/main/java/io/getunleash/util/UnleashConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import io.getunleash.event.NoOpSubscriber;
import io.getunleash.event.UnleashSubscriber;
import io.getunleash.impactmetrics.ImpactMetricsDataSource;
import io.getunleash.impactmetrics.InMemoryMetricRegistry;
import io.getunleash.lang.Nullable;
import io.getunleash.metric.DefaultHttpMetricsSender;
import io.getunleash.repository.HttpFeatureFetcher;
Expand Down Expand Up @@ -74,7 +75,7 @@ public class UnleashConfig {
@Nullable private final ToggleBootstrapProvider toggleBootstrapProvider;
@Nullable private final Proxy proxy;
@Nullable private final Consumer<UnleashException> startupExceptionHandler;
@Nullable private final ImpactMetricsDataSource impactMetricsRegistry;
private final ImpactMetricsDataSource impactMetricsRegistry;

private UnleashConfig(
@Nullable URI unleashAPI,
Expand Down Expand Up @@ -109,7 +110,7 @@ private UnleashConfig(
@Nullable Proxy proxy,
@Nullable Authenticator proxyAuthenticator,
@Nullable Consumer<UnleashException> startupExceptionHandler,
@Nullable ImpactMetricsDataSource impactMetricsRegistry) {
ImpactMetricsDataSource impactMetricsRegistry) {

if (appName == null) {
throw new IllegalStateException("You are required to specify the unleash appName");
Expand Down Expand Up @@ -356,7 +357,6 @@ public Proxy getProxy() {
return proxy;
}

@Nullable
public ImpactMetricsDataSource getImpactMetricsRegistry() {
return impactMetricsRegistry;
}
Expand Down Expand Up @@ -766,7 +766,8 @@ public UnleashConfig build() {
proxy,
proxyAuthenticator,
startupExceptionHandler,
impactMetricsRegistry);
Optional.ofNullable(impactMetricsRegistry)
.orElseGet(InMemoryMetricRegistry::new));
}

public String getDefaultSdkVersion() {
Expand Down
59 changes: 0 additions & 59 deletions src/test/java/io/getunleash/impactmetrics/MetricTypeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,6 @@

import static org.assertj.core.api.Assertions.assertThat;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import io.getunleash.util.HistogramBucketSerializer;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -303,60 +300,4 @@ private NumericMetricSample sample(long value) {
private NumericMetricSample sample(Map<String, String> labels, long value) {
return new NumericMetricSample(labels, value);
}

@Test
public void should_serialize_histogram_with_positive_infinity_as_plus_inf_string() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving this to http level, so we can catch also if serializer was not mounted.

InMemoryMetricRegistry registry = new InMemoryMetricRegistry();
Histogram histogram =
registry.histogram(
new BucketMetricOptions(
"serialization_test", "test serialization", List.of(1.0, 5.0)));

histogram.observe(10.0, Map.of("env", "prod"));

List<CollectedMetric> metrics = registry.collect();
assertThat(metrics).hasSize(1);

Gson gson =
new GsonBuilder()
.registerTypeAdapter(HistogramBucket.class, new HistogramBucketSerializer())
.create();

String json = gson.toJson(metrics.get(0));

assertThat(json).contains("\"le\":\"+Inf\"");
assertThat(json).contains("\"le\":1.0");
assertThat(json).contains("\"le\":5.0");
}

@Test
public void should_serialize_histogram_buckets_correctly() {
InMemoryMetricRegistry registry = new InMemoryMetricRegistry();
Histogram histogram =
registry.histogram(
new BucketMetricOptions(
"bucket_serialization", "test buckets", List.of(0.5, 2.0)));

histogram.observe(0.3);
histogram.observe(1.5);
histogram.observe(10.0);

List<CollectedMetric> metrics = registry.collect();
assertThat(metrics).hasSize(1);

BucketMetricSample sample = (BucketMetricSample) metrics.get(0).getSamples().get(0);
assertThat(sample.getBuckets()).hasSize(3);

Gson gson =
new GsonBuilder()
.registerTypeAdapter(HistogramBucket.class, new HistogramBucketSerializer())
.create();

String json = gson.toJson(sample);

assertThat(json).contains("\"le\":0.5");
assertThat(json).contains("\"le\":2.0");
assertThat(json).contains("\"le\":\"+Inf\"");
assertThat(json).contains("\"count\":");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,17 @@

import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
import io.getunleash.engine.MetricsBucket;
import io.getunleash.impactmetrics.BucketMetricOptions;
import io.getunleash.impactmetrics.CollectedMetric;
import io.getunleash.impactmetrics.Histogram;
import io.getunleash.impactmetrics.InMemoryMetricRegistry;
import io.getunleash.util.UnleashConfig;
import java.net.URI;
import java.net.URISyntaxException;
import java.time.Instant;
import java.time.LocalDateTime;
import java.util.HashSet;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

Expand Down Expand Up @@ -108,4 +113,44 @@ public void should_handle_service_failure_when_sending_metrics() throws URISynta
.withHeader(UNLEASH_INTERVAL, matching(String.valueOf(metricsInterval)))
.withHeader("UNLEASH-APPNAME", matching("test-app")));
}

@Test
public void should_send_impact_metrics_with_histogram_and_plus_inf_bucket()
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the java test style, we verify that +Inf is being sent.

throws URISyntaxException {
stubFor(
post(urlEqualTo("/client/metrics"))
.withHeader("UNLEASH-APPNAME", matching("test-app"))
.willReturn(aResponse().withStatus(200)));

URI uri = new URI("http://localhost:" + serverMock.getPort());
UnleashConfig config = UnleashConfig.builder().appName("test-app").unleashAPI(uri).build();

InMemoryMetricRegistry registry = new InMemoryMetricRegistry();
Histogram histogram =
registry.histogram(
new BucketMetricOptions(
"test_histogram", "testing histogram", List.of(1.0, 5.0)));

histogram.observe(0.5);
histogram.observe(10.0);

List<CollectedMetric> impactMetrics = registry.collect();

DefaultHttpMetricsSender sender = new DefaultHttpMetricsSender(config);
MetricsBucket bucket = new MetricsBucket(Instant.now(), Instant.now(), null);
ClientMetrics metrics = new ClientMetrics(config, bucket, impactMetrics);
sender.sendMetrics(metrics);

verify(
postRequestedFor(urlMatching("/client/metrics"))
.withRequestBody(matching(".*\"impactMetrics\".*"))
.withRequestBody(matching(".*\"name\"\\s*:\\s*\"test_histogram\".*"))
.withRequestBody(matching(".*\"type\"\\s*:\\s*\"HISTOGRAM\".*"))
.withRequestBody(matching(".*\"le\"\\s*:\\s*\"\\+Inf\".*"))
.withHeader(
UNLEASH_INTERVAL, matching(config.getSendMetricsIntervalMillis()))
.withHeader(
UNLEASH_CONNECTION_ID_HEADER, matching(config.getConnectionId()))
.withHeader("UNLEASH-APPNAME", matching("test-app")));
}
}
127 changes: 127 additions & 0 deletions src/test/java/io/getunleash/metric/UnleashMetricServiceImplTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@
import io.getunleash.engine.UnleashEngine;
import io.getunleash.engine.YggdrasilError;
import io.getunleash.engine.YggdrasilInvalidInputException;
import io.getunleash.impactmetrics.CollectedMetric;
import io.getunleash.impactmetrics.ImpactMetricsDataSource;
import io.getunleash.util.UnleashConfig;
import io.getunleash.util.UnleashScheduledExecutor;
import java.time.LocalDateTime;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
Expand Down Expand Up @@ -359,6 +363,129 @@ public void url_not_found_immediately_increases_interval_to_max() throws Yggdras
assertThat(unleashMetricService.getSkips()).isEqualTo(0);
}

@Test
public void should_include_impact_metrics_in_clientmetrics_payload() throws YggdrasilError {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are same as in node SDK.

UnleashConfig config =
UnleashConfig.builder()
.appName("test")
.sendMetricsInterval(10)
.unleashAPI("http://unleash.com")
.impactMetricsRegistry(mock(ImpactMetricsDataSource.class))
.build();

UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class);
DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class);
UnleashEngine engine = new UnleashEngine();
ImpactMetricsDataSource registry = mock(ImpactMetricsDataSource.class);

CollectedMetric sample =
new CollectedMetric(
"feature_toggle_used",
"tracks toggle usage",
io.getunleash.impactmetrics.MetricType.COUNTER,
Collections.emptyList());
when(registry.collect()).thenReturn(List.of(sample));

UnleashConfig configWithRegistry =
UnleashConfig.builder()
.appName("test")
.sendMetricsInterval(10)
.unleashAPI("http://unleash.com")
.impactMetricsRegistry(registry)
.build();

UnleashMetricServiceImpl unleashMetricService =
new UnleashMetricServiceImpl(configWithRegistry, sender, executor, engine);

ArgumentCaptor<Runnable> sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class);
verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong());

when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(200);

sendMetricsCallback.getValue().run();

ArgumentCaptor<ClientMetrics> cmCaptor = ArgumentCaptor.forClass(ClientMetrics.class);
verify(sender).sendMetrics(cmCaptor.capture());

ClientMetrics metricsSent = cmCaptor.getValue();
assertThat(metricsSent.getImpactMetrics()).isNotNull();
assertThat(metricsSent.getImpactMetrics()).hasSize(1);
assertThat(metricsSent.getImpactMetrics().get(0).getName())
.isEqualTo("feature_toggle_used");
}

@Test
public void should_restore_impact_metrics_on_failure() throws YggdrasilError {
ImpactMetricsDataSource registry = mock(ImpactMetricsDataSource.class);

CollectedMetric sample =
new CollectedMetric(
"feature_toggle_used",
"tracks toggle usage",
io.getunleash.impactmetrics.MetricType.COUNTER,
Collections.emptyList());
when(registry.collect()).thenReturn(List.of(sample));

UnleashConfig config =
UnleashConfig.builder()
.appName("test")
.sendMetricsInterval(10)
.unleashAPI("http://unleash.com")
.impactMetricsRegistry(registry)
.build();

UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class);
DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class);
UnleashEngine engine = new UnleashEngine();

UnleashMetricServiceImpl unleashMetricService =
new UnleashMetricServiceImpl(config, sender, executor, engine);

ArgumentCaptor<Runnable> sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class);
verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong());

when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(500);

sendMetricsCallback.getValue().run();

verify(registry, times(1)).restore(List.of(sample));
}

@Test
public void should_not_include_impact_metrics_field_when_empty() throws YggdrasilError {
ImpactMetricsDataSource registry = mock(ImpactMetricsDataSource.class);
when(registry.collect()).thenReturn(Collections.emptyList());

UnleashConfig config =
UnleashConfig.builder()
.appName("test")
.sendMetricsInterval(10)
.unleashAPI("http://unleash.com")
.impactMetricsRegistry(registry)
.build();

UnleashScheduledExecutor executor = mock(UnleashScheduledExecutor.class);
DefaultHttpMetricsSender sender = mock(DefaultHttpMetricsSender.class);
UnleashEngine engine = new UnleashEngine();

UnleashMetricServiceImpl unleashMetricService =
new UnleashMetricServiceImpl(config, sender, executor, engine);

ArgumentCaptor<Runnable> sendMetricsCallback = ArgumentCaptor.forClass(Runnable.class);
verify(executor).setInterval(sendMetricsCallback.capture(), anyLong(), anyLong());

when(sender.sendMetrics(any(ClientMetrics.class))).thenReturn(200);

sendMetricsCallback.getValue().run();

ArgumentCaptor<ClientMetrics> cmCaptor = ArgumentCaptor.forClass(ClientMetrics.class);
verify(sender).sendMetrics(cmCaptor.capture());

ClientMetrics metricsSent = cmCaptor.getValue();
assertThat(metricsSent.getImpactMetrics()).isNull();
verify(registry, times(1)).collect();
}

@Test
public void should_add_new_metrics_data_to_bucket() {
UnleashConfig config =
Expand Down
Loading