From a0921b203be14b61d3ac0e277389e75020502f98 Mon Sep 17 00:00:00 2001 From: jradhakrishnan Date: Fri, 14 Jul 2023 20:03:45 +0530 Subject: [PATCH] Emit ECS Meta metrics even when no prometheus instrumentation [ch16154] * Build `StaticConfig` even when no prometheus targets * Always emit meta metrics * Build scrape target only when Prometheus instrumentation is present * Remove unused config fields * Reorder methods in `ECSTaskUtil` based on access --- .../ai/asserts/aws/config/ScrapeConfig.java | 15 -- .../exporter/ECSServiceDiscoveryExporter.java | 3 + .../ai/asserts/aws/exporter/ECSTaskUtil.java | 145 ++++++++++-------- .../ECSServiceDiscoveryExporterTest.java | 31 +++- .../asserts/aws/exporter/ECSTaskUtilTest.java | 20 ++- 5 files changed, 122 insertions(+), 92 deletions(-) diff --git a/config/src/main/java/ai/asserts/aws/config/ScrapeConfig.java b/config/src/main/java/ai/asserts/aws/config/ScrapeConfig.java index 6803d0b3..c5f31cfd 100644 --- a/config/src/main/java/ai/asserts/aws/config/ScrapeConfig.java +++ b/config/src/main/java/ai/asserts/aws/config/ScrapeConfig.java @@ -72,18 +72,6 @@ public class ScrapeConfig { @Builder.Default private Map primaryExporterByAccount = new TreeMap<>(); - @Builder.Default - private Integer listMetricsResultCacheTTLMinutes = 10; - - @Builder.Default - private Integer listFunctionsResultCacheTTLMinutes = 5; - - @Builder.Default - private Integer getResourcesResultCacheTTLMinutes = 5; - - @Builder.Default - private Integer numTaskThreads = 5; - @Builder.Default private AuthConfig authConfig = new AuthConfig(); @@ -93,9 +81,6 @@ public class ScrapeConfig { @Builder.Default private Integer delay = 0; - @Builder.Default - private Integer logScrapeDelaySeconds = 15; - private TagExportConfig tagExportConfig; private String alertForwardUrl; diff --git a/src/main/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporter.java b/src/main/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporter.java index 349c113b..f0df4767 100644 --- a/src/main/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporter.java +++ b/src/main/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporter.java @@ -201,6 +201,9 @@ void writeFile(ScrapeConfig scrapeConfig, List targets, String fil @VisibleForTesting boolean shouldScrapeTargets(ScrapeConfig scrapeConfig, StaticConfig config) { + if( config.getTargets().isEmpty() ) { + return false; + } String targetVpc = config.getLabels().getVpcId(); String targetSubnet = config.getLabels().getSubnetId(); boolean vpcOK = scrapeConfig.isDiscoverECSTasksAcrossVPCs() || diff --git a/src/main/java/ai/asserts/aws/exporter/ECSTaskUtil.java b/src/main/java/ai/asserts/aws/exporter/ECSTaskUtil.java index b9815dd0..d8d827f4 100644 --- a/src/main/java/ai/asserts/aws/exporter/ECSTaskUtil.java +++ b/src/main/java/ai/asserts/aws/exporter/ECSTaskUtil.java @@ -4,8 +4,8 @@ */ package ai.asserts.aws.exporter; -import ai.asserts.aws.AWSClientProvider; import ai.asserts.aws.AWSApiCallRateLimiter; +import ai.asserts.aws.AWSClientProvider; import ai.asserts.aws.TagUtil; import ai.asserts.aws.TaskExecutorUtil; import ai.asserts.aws.account.AWSAccount; @@ -75,7 +75,8 @@ public class ECSTaskUtil { public static final String PROMETHEUS_METRIC_PATH_DOCKER_LABEL = "PROMETHEUS_EXPORTER_PATH"; - public ECSTaskUtil(AWSClientProvider awsClientProvider, ResourceMapper resourceMapper, AWSApiCallRateLimiter rateLimiter, + public ECSTaskUtil(AWSClientProvider awsClientProvider, ResourceMapper resourceMapper, + AWSApiCallRateLimiter rateLimiter, TagUtil tagUtil, TaskExecutorUtil taskExecutorUtil) { this.awsClientProvider = awsClientProvider; this.resourceMapper = resourceMapper; @@ -86,17 +87,6 @@ public ECSTaskUtil(AWSClientProvider awsClientProvider, ResourceMapper resourceM envName = getInstallEnvName(); } - @VisibleForTesting - String getInstallEnvName() { - final String envName; - if (System.getenv(INSTALLED_ENV_NAME) != null) { - envName = System.getenv(INSTALLED_ENV_NAME); - } else { - envName = null; - } - return envName; - } - public boolean hasAllInfo(Task task) { return "RUNNING".equals(task.lastStatus()) && task.hasAttachments() && task.attachments() .stream() @@ -142,6 +132,8 @@ public List buildScrapeTargets(AWSAccount account, ScrapeConfig sc Optional portFromLabel = getDockerLabel(cD, PROMETHEUS_PORT_DOCKER_LABEL); labelsBuilder.availabilityZone(task.availabilityZone()); String jobName = cD.name(); + labelsBuilder.container(jobName); + labelsBuilder.job(jobName); if (pathFromLabel.isPresent() && portFromLabel.isPresent()) { log.debug("Found prometheus port={}, path={} from docker labels for container {}/{}", portFromLabel.get(), @@ -149,26 +141,17 @@ public List buildScrapeTargets(AWSAccount account, ScrapeConfig sc taskDefinition.taskDefinitionArn(), cD.name()); Labels labels = labelsBuilder - .job(jobName) .metricsPath(pathFromLabel.get()) - .container(cD.name()) .build(); - labels.populateMapEntries(); - labels.putAll(tagLabels); - StaticConfig staticConfig = targetsByLabel.computeIfAbsent( - labels, k -> StaticConfig.builder().labels(labels).build()); + StaticConfig staticConfig = buildStaticConfig(tagLabels, targetsByLabel, cD, labels); staticConfig.getTargets().add(format("%s:%s", ipAddress, portFromLabel.get())); - if (cD.logConfiguration() != null) { - LogConfig logConfig = LogConfig.builder() - .logDriver(cD.logConfiguration().logDriver().toString()) - .options(cD.logConfiguration().options()).build(); - staticConfig.getLogConfigs().add(logConfig); - } } else { log.warn("Docker labels for prometheus port and path not found for container {}/{}", taskDefinition.taskDefinitionArn(), cD.name()); + Labels labels = labelsBuilder.build(); + buildStaticConfig(tagLabels, targetsByLabel, cD, labels); } }); } else { @@ -187,6 +170,58 @@ public List buildScrapeTargets(AWSAccount account, ScrapeConfig sc return targets; } + public SubnetDetails getSubnetDetails(Resource taskResource) { + EcsClient ecsClient = awsClientProvider.getECSClient(taskResource.getRegion(), AWSAccount.builder() + .accountId(taskResource.getAccount()) + .build()); + DescribeTasksResponse response = rateLimiter.doWithRateLimit("EcsClient/describeTasks", + ImmutableSortedMap.of( + SCRAPE_ACCOUNT_ID_LABEL, taskResource.getAccount(), + SCRAPE_REGION_LABEL, taskResource.getRegion(), + SCRAPE_OPERATION_LABEL, "EcsClient/describeTasks"), + () -> ecsClient.describeTasks(DescribeTasksRequest.builder() + .cluster(taskResource.getChildOf().getName()) + .tasks(taskResource.getArn()) + .build())); + if (response.hasTasks()) { + return response.tasks().get(0).attachments().stream() + .filter(attachment -> attachment.type().equals("ElasticNetworkInterface")) + .findFirst() + .flatMap(attachment -> attachment.details().stream() + .filter(kv -> kv.name().equals("subnetId")).findFirst()) + .map(kv -> { + AtomicReference vpcId = new AtomicReference<>(""); + AtomicReference subnetId = new AtomicReference<>(""); + subnetId.set(kv.value()); + vpcId.set(subnetIdMap.computeIfAbsent(subnetId.get(), kk -> + getVpcId(taskResource, subnetId))); + return SubnetDetails.builder() + .vpcId(vpcId.get()) + .subnetId(subnetId.get()) + .build(); + }).orElse(null); + } + log.warn("Failed to find description for {}", taskResource); + return null; + } + + @VisibleForTesting + String getEnv(AWSAccount account) { + return account.getName() != null ? account.getName() : envName != null ? envName : account.getAccountId(); + } + + @VisibleForTesting + String getInstallEnvName() { + final String envName; + if (System.getenv(INSTALLED_ENV_NAME) != null) { + envName = System.getenv(INSTALLED_ENV_NAME); + } else { + envName = null; + } + return envName; + } + + @SuppressWarnings("OptionalUsedAsFieldOrParameterType") private LabelsBuilder getLabelsBuilder(AWSAccount account, Resource cluster, Optional service, Task task) { Resource taskDefResource = resourceMapper.map(task.taskDefinitionArn()) @@ -232,8 +267,19 @@ private LabelsBuilder getLabelsBuilder(AWSAccount account, Resource cluster, Opt return labelsBuilder; } - private String getEnv(AWSAccount account) { - return envName != null ? envName : account.getName() != null ? account.getName() : account.getAccountId(); + private StaticConfig buildStaticConfig(Map tagLabels, Map targetsByLabel, + ContainerDefinition cD, Labels labels) { + labels.populateMapEntries(); + labels.putAll(tagLabels); + StaticConfig staticConfig = targetsByLabel.computeIfAbsent( + labels, k -> StaticConfig.builder().labels(labels).build()); + if (cD.logConfiguration() != null) { + LogConfig logConfig = LogConfig.builder() + .logDriver(cD.logConfiguration().logDriver().toString()) + .options(cD.logConfiguration().options()).build(); + staticConfig.getLogConfigs().add(logConfig); + } + return staticConfig; } private String getIPAddress(Task task) { @@ -249,39 +295,11 @@ private String getIPAddress(Task task) { return ipAddress; } - public SubnetDetails getSubnetDetails(Resource taskResource) { - EcsClient ecsClient = awsClientProvider.getECSClient(taskResource.getRegion(), AWSAccount.builder() - .accountId(taskResource.getAccount()) - .build()); - DescribeTasksResponse response = rateLimiter.doWithRateLimit("EcsClient/describeTasks", - ImmutableSortedMap.of( - SCRAPE_ACCOUNT_ID_LABEL, taskResource.getAccount(), - SCRAPE_REGION_LABEL, taskResource.getRegion(), - SCRAPE_OPERATION_LABEL, "EcsClient/describeTasks"), - () -> ecsClient.describeTasks(DescribeTasksRequest.builder() - .cluster(taskResource.getChildOf().getName()) - .tasks(taskResource.getArn()) - .build())); - if (response.hasTasks()) { - return response.tasks().get(0).attachments().stream() - .filter(attachment -> attachment.type().equals("ElasticNetworkInterface")) - .findFirst() - .flatMap(attachment -> attachment.details().stream() - .filter(kv -> kv.name().equals("subnetId")).findFirst()) - .map(kv -> { - AtomicReference vpcId = new AtomicReference<>(""); - AtomicReference subnetId = new AtomicReference<>(""); - subnetId.set(kv.value()); - vpcId.set(subnetIdMap.computeIfAbsent(subnetId.get(), kk -> - getVpcId(taskResource, subnetId))); - return SubnetDetails.builder() - .vpcId(vpcId.get()) - .subnetId(subnetId.get()) - .build(); - }).orElse(null); - } - log.warn("Failed to find description for {}", taskResource); - return null; + private Optional getDockerLabel(ContainerDefinition container, String labelName) { + return container.dockerLabels().entrySet().stream() + .filter(entry -> entry.getKey().equals(labelName)) + .map(Map.Entry::getValue) + .findFirst(); } private SubnetDetails getSubnetDetails(Task task, Resource taskResource) { @@ -321,11 +339,4 @@ private String getVpcId(Resource taskResource, AtomicReference subnetId) r.subnets().stream().findFirst().ifPresent(subnet -> id.set(subnet.vpcId())); return id.get(); } - - Optional getDockerLabel(ContainerDefinition container, String labelName) { - return container.dockerLabels().entrySet().stream() - .filter(entry -> entry.getKey().equals(labelName)) - .map(Map.Entry::getValue) - .findFirst(); - } } diff --git a/src/test/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporterTest.java b/src/test/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporterTest.java index ee867cb9..2749c3b2 100644 --- a/src/test/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporterTest.java +++ b/src/test/java/ai/asserts/aws/exporter/ECSServiceDiscoveryExporterTest.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.databind.ObjectWriter; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import org.easymock.EasyMockSupport; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -238,36 +239,49 @@ void identifySubnetsToScrape() { .vpcId("vpc-1") .subnetId("subnet-1") .build()); - assertTrue(testClass.shouldScrapeTargets(scrapeConfig, StaticConfig.builder() + StaticConfig staticConfig = StaticConfig.builder() .labels(Labels.builder() .vpcId("vpc-1") .subnetId("subnet-1") .build()) - .build())); + .build(); + + // Without target + assertFalse(testClass.shouldScrapeTargets(scrapeConfig, staticConfig)); + + // With target + staticConfig.getTargets().add("1.2.3.4:8080"); + assertTrue(testClass.shouldScrapeTargets(scrapeConfig, staticConfig)); // Same VPC, different subnet. But subnet configured to be scraped - assertTrue(testClass.shouldScrapeTargets(scrapeConfig, StaticConfig.builder() + staticConfig = StaticConfig.builder() .labels(Labels.builder() .vpcId("vpc-1") .subnetId("subnet-2") .build()) - .build())); + .build(); + staticConfig.getTargets().add("1.2.3.4:8080"); + assertTrue(testClass.shouldScrapeTargets(scrapeConfig, staticConfig)); // Same VPC, different subnet. But subnet not configured to be scraped - assertFalse(testClass.shouldScrapeTargets(scrapeConfig, StaticConfig.builder() + staticConfig = StaticConfig.builder() .labels(Labels.builder() .vpcId("vpc-1") .subnetId("subnet-3") .build()) - .build())); + .build(); + staticConfig.getTargets().add("1.2.3.4:8080"); + assertFalse(testClass.shouldScrapeTargets(scrapeConfig, staticConfig)); // Different VPC - assertFalse(testClass.shouldScrapeTargets(scrapeConfig, StaticConfig.builder() + staticConfig = StaticConfig.builder() .labels(Labels.builder() .vpcId("vpc-2") .subnetId("subnet-1") .build()) - .build())); + .build(); + staticConfig.getTargets().add("1.2.3.4:8080"); + assertFalse(testClass.shouldScrapeTargets(scrapeConfig, staticConfig)); verifyAll(); } @@ -283,6 +297,7 @@ public void run() throws Exception { expect(ecsTaskProvider.getScrapeTargets()).andReturn(ImmutableList.of(mockStaticConfig, mockStaticConfig)); expect(scrapeConfig.isLogECSTargets()).andReturn(true); + expect(mockStaticConfig.getTargets()).andReturn(ImmutableSet.of("1.2.3.4:8080")).anyTimes(); expect(mockStaticConfig.getLabels()).andReturn(mockLabels).anyTimes(); expect(mockLabels.getVpcId()).andReturn("vpc-id").anyTimes(); expect(mockLabels.getSubnetId()).andReturn("subnet-id").anyTimes(); diff --git a/src/test/java/ai/asserts/aws/exporter/ECSTaskUtilTest.java b/src/test/java/ai/asserts/aws/exporter/ECSTaskUtilTest.java index 4d4dd392..8fffd21c 100644 --- a/src/test/java/ai/asserts/aws/exporter/ECSTaskUtilTest.java +++ b/src/test/java/ai/asserts/aws/exporter/ECSTaskUtilTest.java @@ -4,8 +4,8 @@ */ package ai.asserts.aws.exporter; -import ai.asserts.aws.AWSClientProvider; import ai.asserts.aws.AWSApiCallRateLimiter; +import ai.asserts.aws.AWSClientProvider; import ai.asserts.aws.TagUtil; import ai.asserts.aws.TaskExecutorUtil; import ai.asserts.aws.TestTaskThreadPool; @@ -66,6 +66,7 @@ public class ECSTaskUtilTest extends EasyMockSupport { private TagUtil tagUtil; private ScrapeConfig scrapeConfig; private AWSAccount account; + private String defaultEnvName; @BeforeEach public void setup() { @@ -85,9 +86,15 @@ public void setup() { Ec2Client ec2Client = mock(Ec2Client.class); + defaultEnvName = "dev"; testClass = new ECSTaskUtil(awsClientProvider, resourceMapper, rateLimiter, tagUtil, - taskExecutorUtil); + taskExecutorUtil) { + @Override + String getInstallEnvName() { + return defaultEnvName; + } + }; expect(awsClientProvider.getEc2Client(anyString(), anyObject())).andReturn(ec2Client).anyTimes(); expect(ec2Client.describeSubnets(DescribeSubnetsRequest.builder() @@ -144,6 +151,15 @@ public void hasAllInfo_true() { verifyAll(); } + @Test + public void getEnvName() { + assertEquals("prod", testClass.getEnv(AWSAccount.builder() + .name("prod") + .build())); + assertEquals(defaultEnvName, testClass.getEnv(AWSAccount.builder() + .build())); + } + @Test public void containerWithDockerLabels() { expect(resourceMapper.map("task-def-arn")).andReturn(Optional.of(taskDef));