Merge pull request #10876 from vespa-engine/olaa/metering-by-zone

Add zone dimension to metering data

4 files changed, 55 insertions, 28 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java
index a378bcb63bd..5ee6df9f034 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/resource/ResourceSnapshot.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.api.integration.resource;
 
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
 
 import java.time.Instant;
@@ -17,14 +18,16 @@ public class ResourceSnapshot {
     private final ApplicationId applicationId;
     private final ResourceAllocation resourceAllocation;
     private final Instant timestamp;
+    private final ZoneId zoneId;
 
-    public ResourceSnapshot(ApplicationId applicationId, double cpuCores, double memoryGb, double diskGb, Instant timestamp) {
+    public ResourceSnapshot(ApplicationId applicationId, double cpuCores, double memoryGb, double diskGb, Instant timestamp, ZoneId zoneId) {
         this.applicationId = applicationId;
         this.resourceAllocation = new ResourceAllocation(cpuCores, memoryGb, diskGb);
         this.timestamp = timestamp;
+        this.zoneId = zoneId;
     }
 
-    public static ResourceSnapshot from(List<Node> nodes, Instant timestamp) {
+    public static ResourceSnapshot from(List<Node> nodes, Instant timestamp, ZoneId zoneId) {
         Set<ApplicationId> applicationIds = nodes.stream()
                                                  .filter(node -> node.owner().isPresent())
                                                  .map(node -> node.owner().get())
@@ -37,7 +40,8 @@ public class ResourceSnapshot {
                 nodes.stream().mapToDouble(Node::vcpu).sum(),
                 nodes.stream().mapToDouble(Node::memoryGb).sum(),
                 nodes.stream().mapToDouble(Node::diskGb).sum(),
-                timestamp
+                timestamp,
+                zoneId
         );
     }
 
@@ -61,4 +65,8 @@ public class ResourceSnapshot {
         return timestamp;
     }
 
+    public ZoneId getZoneId() {
+        return zoneId;
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
index c700ddac51c..0e14b61c5c5 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainer.java
@@ -3,6 +3,8 @@ package com.yahoo.vespa.hosted.controller.maintenance;
 
 import com.yahoo.config.provision.CloudName;
 import com.yahoo.config.provision.SystemName;
+import com.yahoo.config.provision.zone.ZoneApi;
+import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.vespa.hosted.controller.Controller;
 import com.yahoo.vespa.hosted.controller.api.integration.configserver.Node;
@@ -15,6 +17,7 @@ import java.time.Duration;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 /**
@@ -47,33 +50,44 @@ public class ResourceMeterMaintainer extends Maintainer {
 
     @Override
     protected void maintain() {
-        Collection<ResourceSnapshot> resourceSnapshots = getResourceSnapshots(allocatedNodes());
+
+        Collection<ResourceSnapshot> resourceSnapshots = getAllResourceSnapshots();
         meteringClient.consume(resourceSnapshots);
 
         metric.set(METERING_LAST_REPORTED, clock.millis() / 1000, metric.createContext(Collections.emptyMap()));
         // total metered resource usage, for alerting on drastic changes
         metric.set(METERING_TOTAL_REPORTED,
-                   resourceSnapshots.stream().mapToDouble(r -> r.getCpuCores() + r.getMemoryGb() + r.getDiskGb()).sum(),
+                   resourceSnapshots.stream()
+                           .mapToDouble(r -> r.getCpuCores() + r.getMemoryGb() + r.getDiskGb()).sum(),
                    metric.createContext(Collections.emptyMap()));
     }
 
-    private List<Node> allocatedNodes() {
+    private Collection<ResourceSnapshot> getAllResourceSnapshots() {
         return controller().zoneRegistry().zones()
                 .ofCloud(CloudName.from("aws"))
                 .reachable().zones().stream()
-                .flatMap(zone -> nodeRepository.list(zone.getId()).stream())
-                .filter(node -> node.owner().isPresent())
-                .filter(node -> ! node.owner().get().tenant().value().equals("hosted-vespa"))
+                .map(ZoneApi::getId)
+                .map(zoneId -> createResourceSnapshotsFromNodes(zoneId, nodeRepository.list(zoneId)))
+                .flatMap(Collection::stream)
                 .collect(Collectors.toList());
     }
 
-    private Collection<ResourceSnapshot> getResourceSnapshots(List<Node> nodes) {
+    private Collection<ResourceSnapshot> createResourceSnapshotsFromNodes(ZoneId zoneId, List<Node> nodes) {
         return nodes.stream()
-                    .collect(Collectors.groupingBy(node -> node.owner().get(),
-                                                   Collectors.collectingAndThen(Collectors.toList(),
-                                                                                nodeList -> ResourceSnapshot.from(nodeList,
-                                                                                                                  clock.instant()))
-                                    )).values();
+                .filter(unlessNodeOwnerIsHostedVespa())
+                .collect(Collectors.groupingBy(node ->
+                                node.owner().get(),
+                                Collectors.collectingAndThen(Collectors.toList(),
+                                        nodeList -> ResourceSnapshot.from(
+                                                nodeList,
+                                                clock.instant(),
+                                                zoneId))
+                                )).values();
     }
 
+    private Predicate<Node> unlessNodeOwnerIsHostedVespa() {
+        return node -> node.owner().map(owner ->
+                !owner.tenant().value().equals("hosted-vespa")
+        ).orElse(false);
+    }
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
index f28ce83e643..0245e7475f7 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/ResourceMeterMaintainerTest.java
@@ -11,7 +11,6 @@ import org.junit.Test;
 
 import java.time.Duration;
 import java.util.Collection;
-import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 
@@ -27,20 +26,14 @@ public class ResourceMeterMaintainerTest {
 
     @Test
     public void testMaintainer() {
-        var awsZone = ZoneApiMock.newBuilder().withId("prod.aws-us-east-1").withCloud("aws").build();
-        tester.zoneRegistry().setZones(
-                ZoneApiMock.newBuilder().withId("prod.us-east-3").build(),
-                ZoneApiMock.newBuilder().withId("prod.us-west-1").build(),
-                ZoneApiMock.newBuilder().withId("prod.us-central-1").build(),
-                awsZone);
-        tester.configServer().nodeRepository().addFixedNodes(awsZone.getId());
+        setUpZones();
 
         ResourceMeterMaintainer resourceMeterMaintainer = new ResourceMeterMaintainer(tester.controller(), Duration.ofMinutes(5), new JobControl(tester.curator()), metrics, snapshotConsumer);
         resourceMeterMaintainer.maintain();
         Collection<ResourceSnapshot> consumedResources = snapshotConsumer.consumedResources();
 
         // The mocked repository contains two applications, so we should also consume two ResourceSnapshots
-        assertEquals(2, consumedResources.size());
+        assertEquals(4, consumedResources.size());
         ResourceSnapshot app1 = consumedResources.stream().filter(snapshot -> snapshot.getApplicationId().equals(ApplicationId.from("tenant1", "app1", "default"))).findFirst().orElseThrow();
         ResourceSnapshot app2 = consumedResources.stream().filter(snapshot -> snapshot.getApplicationId().equals(ApplicationId.from("tenant2", "app2", "default"))).findFirst().orElseThrow();
 
@@ -53,7 +46,19 @@ public class ResourceMeterMaintainerTest {
         assertEquals(500, app2.getDiskGb(), DELTA);
 
         assertEquals(tester.clock().millis()/1000, metrics.getMetric("metering_last_reported"));
-        assertEquals(1112.0d, (Double) metrics.getMetric("metering_total_reported"), DELTA);
+        assertEquals(2224.0d, (Double) metrics.getMetric("metering_total_reported"), DELTA);
     }
 
+    private void setUpZones() {
+        ZoneApiMock nonAwsZone = ZoneApiMock.newBuilder().withId("test.region-1").build();
+        ZoneApiMock awsZone1 = ZoneApiMock.newBuilder().withId("prod.region-2").withCloud("aws").build();
+        ZoneApiMock awsZone2 = ZoneApiMock.newBuilder().withId("test.region-3").withCloud("aws").build();
+        tester.zoneRegistry().setZones(
+                nonAwsZone,
+                awsZone1,
+                awsZone2);
+        tester.configServer().nodeRepository().addFixedNodes(nonAwsZone.getId());
+        tester.configServer().nodeRepository().addFixedNodes(awsZone1.getId());
+        tester.configServer().nodeRepository().addFixedNodes(awsZone2.getId());
+    }
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
index fb59a0e51d8..71b235425b3 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiTest.java
@@ -953,9 +953,9 @@ public class ApplicationApiTest extends ControllerContainerTest {
         ResourceAllocation lastMonth = new ResourceAllocation(24, 48, 2000);
         ApplicationId applicationId = ApplicationId.from("doesnotexist", "doesnotexist", "default");
         Map<ApplicationId, List<ResourceSnapshot>> snapshotHistory = Map.of(applicationId, List.of(
-                new ResourceSnapshot(applicationId, 1, 2,3, Instant.ofEpochMilli(123)),
-                new ResourceSnapshot(applicationId, 1, 2,3, Instant.ofEpochMilli(246)),
-                new ResourceSnapshot(applicationId, 1, 2,3, Instant.ofEpochMilli(492))));
+                new ResourceSnapshot(applicationId, 1, 2,3, Instant.ofEpochMilli(123), ZoneId.defaultId()),
+                new ResourceSnapshot(applicationId, 1, 2,3, Instant.ofEpochMilli(246), ZoneId.defaultId()),
+                new ResourceSnapshot(applicationId, 1, 2,3, Instant.ofEpochMilli(492), ZoneId.defaultId())));
 
         mockMeteringClient.setMeteringInfo(new MeteringInfo(thisMonth, lastMonth, currentSnapshot, snapshotHistory));