Skip to content

Commit

Permalink
Merge pull request #314 from asserts/radha/fix-scrape-config-metrics-npe
Browse files Browse the repository at this point in the history 
Fix NPE in ScrapeConfigExporter
  • Loading branch information
@jradhakrishnan
jradhakrishnan authored Aug 29, 2023
2 parents e890d69 + 9b1d6a6 commit 28b1db4
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 29 deletions.
42 changes: 27 additions & 15 deletions src/main/java/ai/asserts/aws/exporter/ScrapeConfigExporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
package ai.asserts.aws.exporter;

import ai.asserts.aws.ScrapeConfigProvider;
import ai.asserts.aws.SimpleTenantTask;
import ai.asserts.aws.TaskExecutorUtil;
import ai.asserts.aws.account.AccountProvider;
import com.google.common.collect.ImmutableMap;
import io.prometheus.client.Collector;
Expand All @@ -17,6 +19,8 @@

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Future;
import java.util.stream.Collectors;

import static ai.asserts.aws.MetricNameUtil.SCRAPE_NAMESPACE_LABEL;

Expand All @@ -28,6 +32,7 @@ public class ScrapeConfigExporter extends Collector implements InitializingBean
private final ScrapeConfigProvider scrapeConfigProvider;
private final MetricSampleBuilder sampleBuilder;
private final CollectorRegistry collectorRegistry;
private final TaskExecutorUtil taskExecutorUtil;

@Override
public void afterPropertiesSet() {
Expand All @@ -36,23 +41,30 @@ public void afterPropertiesSet() {

@Override
public List<MetricFamilySamples> collect() {
List<Sample> allSamples = new ArrayList<>();
List<MetricFamilySamples> metricFamilySamples = new ArrayList<>();
try {
List<Sample> intervalSamples = new ArrayList<>();
accountProvider.getAccounts().forEach(awsAccount ->
scrapeConfigProvider.getScrapeConfig(awsAccount.getTenant())
.getNamespaces()
.forEach(namespaceConfig ->
scrapeConfigProvider.getStandardNamespace(namespaceConfig.getName())
.flatMap(cwNamespace -> sampleBuilder.buildSingleSample(
"aws_exporter_scrape_interval",
ImmutableMap.of(SCRAPE_NAMESPACE_LABEL,
cwNamespace.getNormalizedNamespace()),
namespaceConfig.getEffectiveScrapeInterval() * 1.0D))
.ifPresent(intervalSamples::add)));

if (intervalSamples.size() > 0) {
sampleBuilder.buildFamily(intervalSamples).ifPresent(metricFamilySamples::add);
List<Future<List<Sample>>> futures = accountProvider.getAccounts().stream().map(awsAccount ->
taskExecutorUtil.executeAccountTask(awsAccount, new SimpleTenantTask<List<Sample>>() {
@Override
public List<Sample> call() {
List<Sample> intervalSamples = new ArrayList<>();
scrapeConfigProvider.getScrapeConfig(awsAccount.getTenant())
.getNamespaces()
.forEach(namespaceConfig ->
scrapeConfigProvider.getStandardNamespace(namespaceConfig.getName())
.flatMap(cwNamespace -> sampleBuilder.buildSingleSample(
"aws_exporter_scrape_interval",
ImmutableMap.of(SCRAPE_NAMESPACE_LABEL,
cwNamespace.getNormalizedNamespace()),
namespaceConfig.getEffectiveScrapeInterval() * 1.0D))
.ifPresent(intervalSamples::add));
return intervalSamples;
}
})).collect(Collectors.toList());
taskExecutorUtil.awaitAll(futures, allSamples::addAll);
if (allSamples.size() > 0) {
sampleBuilder.buildFamily(allSamples).ifPresent(metricFamilySamples::add);
}
} catch (Exception e) {
log.error("Failed to build metric samples", e);
Expand Down
50 changes: 36 additions & 14 deletions src/test/java/ai/asserts/aws/exporter/ScrapeConfigExporterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,35 +5,44 @@
package ai.asserts.aws.exporter;

import ai.asserts.aws.ScrapeConfigProvider;
import ai.asserts.aws.TaskExecutorUtil;
import ai.asserts.aws.TenantTask;
import ai.asserts.aws.account.AWSAccount;
import ai.asserts.aws.account.AccountProvider;
import ai.asserts.aws.config.NamespaceConfig;
import ai.asserts.aws.config.ScrapeConfig;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import io.prometheus.client.Collector;
import io.prometheus.client.Collector.MetricFamilySamples.Sample;
import io.prometheus.client.CollectorRegistry;
import org.easymock.Capture;
import org.easymock.EasyMockSupport;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import java.util.List;
import java.util.Optional;
import java.util.concurrent.Future;
import java.util.function.Consumer;

import static ai.asserts.aws.MetricNameUtil.SCRAPE_NAMESPACE_LABEL;
import static ai.asserts.aws.model.CWNamespace.lambda;
import static org.easymock.EasyMock.capture;
import static org.easymock.EasyMock.eq;
import static org.easymock.EasyMock.expect;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

public class ScrapeConfigExporterTest extends EasyMockSupport {
private ScrapeConfigProvider scrapeConfigProvider;
private ScrapeConfig scrapeConfig;
private NamespaceConfig namespaceConfig;
private MetricSampleBuilder metricSampleBuilder;
private Collector.MetricFamilySamples.Sample sample;
private Collector.MetricFamilySamples metricFamilySamples;
private Sample sample;
private CollectorRegistry collectorRegistry;
private AccountProvider accountProvider;
private TaskExecutorUtil taskExecutorUtil;
private Future<List<Sample>> mockFuture;
private ScrapeConfigExporter testClass;

@BeforeEach
Expand All @@ -43,12 +52,13 @@ public void setup() {
namespaceConfig = mock(NamespaceConfig.class);
namespaceConfig = mock(NamespaceConfig.class);
metricSampleBuilder = mock(MetricSampleBuilder.class);
sample = mock(Collector.MetricFamilySamples.Sample.class);
metricFamilySamples = mock(Collector.MetricFamilySamples.class);
sample = mock(Sample.class);
collectorRegistry = mock(CollectorRegistry.class);
accountProvider = mock(AccountProvider.class);
taskExecutorUtil = mock(TaskExecutorUtil.class);
mockFuture = mock(Future.class);
testClass = new ScrapeConfigExporter(accountProvider, scrapeConfigProvider, metricSampleBuilder,
collectorRegistry);
collectorRegistry, taskExecutorUtil);
}

@Test
Expand All @@ -60,12 +70,16 @@ public void afterPropertiesSet() {
}

@Test
public void collect() {
public void collect() throws Exception {
AWSAccount awsAccount = AWSAccount.builder()
.tenant("tenant")
.build();
expect(accountProvider.getAccounts()).andReturn(ImmutableSet.of(
AWSAccount.builder()
.tenant("tenant")
.build()
awsAccount
));
Capture<TenantTask<List<Sample>>> c1 = Capture.newInstance();
Capture<Consumer<List<Sample>>> c2 = Capture.newInstance();
expect(taskExecutorUtil.executeAccountTask(eq(awsAccount), capture(c1))).andReturn(mockFuture);
expect(scrapeConfigProvider.getScrapeConfig("tenant")).andReturn(scrapeConfig);
expect(scrapeConfig.getNamespaces()).andReturn(ImmutableList.of(namespaceConfig));
expect(namespaceConfig.getName()).andReturn("ns1");
Expand All @@ -75,10 +89,18 @@ public void collect() {
expect(metricSampleBuilder.buildSingleSample("aws_exporter_scrape_interval",
ImmutableMap.of(SCRAPE_NAMESPACE_LABEL, "AWS/Lambda"), 61.0D)).andReturn(Optional.of(sample));

expect(metricSampleBuilder.buildFamily(ImmutableList.of(sample))).andReturn(Optional.of(metricFamilySamples));

taskExecutorUtil.awaitAll(eq(ImmutableList.of(mockFuture)), capture(c2));
replayAll();
assertEquals(ImmutableList.of(metricFamilySamples), testClass.collect());

testClass.collect();

assertNotNull(c2.getValue());
c2.getValue().accept(ImmutableList.of(sample));

assertNotNull(c1.getValue());
c1.getValue().call();


verifyAll();
}
}

0 comments on commit 28b1db4

Please sign in to comment.