Merge pull request #9824 from vespa-engine/gjoranv/metricsproxy-cleanup

Gjoranv/metricsproxy cleanup

8 files changed, 10 insertions, 45 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index 55373f425e0..514ca2a00f5 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -56,7 +56,8 @@ public interface ModelContext {
         boolean useFdispatchByDefault();
         boolean dispatchWithProtobuf();
         boolean useAdaptiveDispatch();
-        boolean enableMetricsProxyContainer();
+        // TODO: Remove when 7.61 is the oldest model in use
+        default boolean enableMetricsProxyContainer() { return false; }
     }
 
 }
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
index 4b35af53154..0a54dd6790d 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
@@ -37,7 +37,6 @@ public class TestProperties implements ModelContext.Properties {
     private boolean useFdispatchByDefault = true;
     private boolean dispatchWithProtobuf = true;
     private boolean useAdaptiveDispatch = false;
-    private boolean enableMetricsProxyContainer = false;
 
 
     @Override public boolean multitenant() { return multitenant; }
@@ -55,7 +54,6 @@ public class TestProperties implements ModelContext.Properties {
     @Override public boolean useDedicatedNodeForLogserver() { return useDedicatedNodeForLogserver; }
     @Override public boolean useFdispatchByDefault() { return useFdispatchByDefault; }
     @Override public boolean dispatchWithProtobuf() { return dispatchWithProtobuf; }
-    @Override public boolean enableMetricsProxyContainer() { return enableMetricsProxyContainer; }
 
     public TestProperties setApplicationId(ApplicationId applicationId) {
         this.applicationId = applicationId;
@@ -87,10 +85,6 @@ public class TestProperties implements ModelContext.Properties {
         return this;
     }
 
-    public TestProperties setEnableMetricsProxyContainer(boolean enableMetricsProxyContainer) {
-        this.enableMetricsProxyContainer = enableMetricsProxyContainer;
-        return this;
-    }
 
     public static class Spec implements ConfigServerSpec {
 
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java
index a700368d1f5..f755871ac4b 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerTest.java
@@ -32,7 +32,6 @@ public class MetricsProxyContainerTest {
     public void one_metrics_proxy_container_is_added_to_every_node() {
         var numberOfHosts = 4;
         var tester = new VespaModelTester();
-        tester.enableMetricsProxyContainer(true);
         tester.addHosts(numberOfHosts);
 
         VespaModel model = tester.createModel(servicesWithManyNodes(), true);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java
index 940b6e666e4..bac9d674460 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyModelTester.java
@@ -36,7 +36,6 @@ class MetricsProxyModelTester {
     static VespaModel getModel(String servicesXml) {
         var numberOfHosts = 1;
         var tester = new VespaModelTester();
-        tester.enableMetricsProxyContainer(true);
         tester.addHosts(numberOfHosts);
         tester.setHosted(false);
         return tester.createModel(servicesXml, true);
@@ -45,7 +44,6 @@ class MetricsProxyModelTester {
     static VespaModel getHostedModel(String servicesXml) {
         var numberOfHosts = 2;
         var tester = new VespaModelTester();
-        tester.enableMetricsProxyContainer(true);
         tester.addHosts(numberOfHosts);
         tester.setHosted(true);
         tester.setApplicationId(MY_TENANT, MY_APPLICATION, MY_INSTANCE);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
index 866c4027711..2e5acb9025d 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/test/VespaModelTester.java
@@ -48,7 +48,6 @@ public class VespaModelTester {
     private Map<NodeResources, Collection<Host>> hostsByResources = new HashMap<>();
     private ApplicationId applicationId = ApplicationId.defaultId();
     private boolean useDedicatedNodeForLogserver = false;
-    private boolean enableMetricsProxyContainer = false;
 
     public VespaModelTester() {
         this(new NullConfigModelRegistry());
@@ -105,10 +104,6 @@ public class VespaModelTester {
         this.useDedicatedNodeForLogserver = useDedicatedNodeForLogserver;
     }
 
-    public void enableMetricsProxyContainer(boolean enableMetricsProxyContainer) {
-        this.enableMetricsProxyContainer = enableMetricsProxyContainer;
-    }
-
     /** Creates a model which uses 0 as start index and fails on out of capacity */
     public VespaModel createModel(String services, String ... retiredHostNames) {
         return createModel(Zone.defaultZone(), services, true, retiredHostNames);
@@ -149,8 +144,7 @@ public class VespaModelTester {
                 .setMultitenant(true)
                 .setHostedVespa(hosted)
                 .setApplicationId(applicationId)
-                .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver)
-                .setEnableMetricsProxyContainer(enableMetricsProxyContainer);
+                .setUseDedicatedNodeForLogserver(useDedicatedNodeForLogserver);
 
         DeployState deployState = new DeployState.Builder()
                 .applicationPackage(appPkg)
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index 0279d175488..fc6667087c6 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -132,7 +132,6 @@ public class ModelContextImpl implements ModelContext {
         private final boolean useFdispatchByDefault;
         private final boolean useAdaptiveDispatch;
         private final boolean dispatchWithProtobuf;
-        private final boolean enableMetricsProxyContainer;
 
         public Properties(ApplicationId applicationId,
                           boolean multitenantFromConfig,
@@ -165,8 +164,6 @@ public class ModelContextImpl implements ModelContext {
                     .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
             this.useAdaptiveDispatch = Flags.USE_ADAPTIVE_DISPATCH.bindTo(flagSource)
                     .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
-            this.enableMetricsProxyContainer = Flags.ENABLE_METRICS_PROXY_CONTAINER.bindTo(flagSource)
-                    .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
         }
 
         @Override
@@ -218,8 +215,6 @@ public class ModelContextImpl implements ModelContext {
         @Override
         public boolean useAdaptiveDispatch() { return useAdaptiveDispatch; }
 
-        @Override
-        public boolean enableMetricsProxyContainer() { return enableMetricsProxyContainer; }
     }
 
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
index da6c5224f74..2716cc8cc59 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java
@@ -40,7 +40,6 @@ import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Function;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import static com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentImpl.ContainerState.ABSENT;
@@ -594,27 +593,15 @@ public class NodeAgentImpl implements NodeAgent {
             for (DimensionMetrics dimensionMetrics : metrics) {
                 params.append(dimensionMetrics.toSecretAgentReport());
             }
-        } catch (JsonProcessingException e) {
-            // TODO: wrap everything into one try-block (to avoid 'return') when old metrics proxy is discontinued
-            context.log(logger, LogLevel.WARNING, "Failed to wrap metrics in secret agent report", e);
-            return;
-        }
-        String wrappedMetrics = "s:" + params.toString();
-
-        // Push metrics to the metrics proxy in each container
-        runPushMetricsCommand(context, wrappedMetrics, true);
-        runPushMetricsCommand(context, wrappedMetrics, false);
-    }
+            String wrappedMetrics = "s:" + params.toString();
 
-    // TODO: Clean up and inline method when old metrics proxy has been discontinued.
-    private void runPushMetricsCommand(NodeAgentContext context, String wrappedMetrics, boolean newMetricsProxy) {
-        int port = newMetricsProxy ? 19095 : 19091;
-        String[] command = {"vespa-rpc-invoke",  "-t", "2",  "tcp/localhost:" + port,  "setExtraMetrics", wrappedMetrics};
-        try {
+            // Push metrics to the metrics proxy in each container.
+            // TODO Remove port selection logic when all hosted apps have upgraded to Vespa 7.
+            int port = context.node().getVespaVersion().map(version -> version.getMajor() == 6).orElse(false) ? 19091 : 19095;
+            String[] command = {"vespa-rpc-invoke",  "-t", "2",  "tcp/localhost:" + port,  "setExtraMetrics", wrappedMetrics};
             dockerOperations.executeCommandInContainerAsRoot(context, 5L, command);
-        } catch (DockerExecTimeoutException  e) {
-            Level level = newMetricsProxy ? LogLevel.DEBUG : LogLevel.WARNING;
-            context.log(logger, level, "Failed to push metrics to container", e);
+        } catch (JsonProcessingException | DockerExecTimeoutException  e) {
+            context.log(logger, LogLevel.WARNING, "Failed to push metrics to container", e);
         }
 
     }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
index 3a92e345801..f754d1798ec 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java
@@ -685,9 +685,6 @@ public class NodeAgentImplTest {
                     .replaceAll("\"timestamp\":\\d+", "\"timestamp\":0")
                     .replaceAll("([0-9]+\\.[0-9]{1,3})([0-9]*)", "$1"); // Only keep the first 3 decimals
 
-            // TODO: Remove when old metrics proxy is discontinued.
-            calledCommand[3] = calledCommand[3].replaceFirst("19091", "19095");
-
             assertEquals(context, calledContainerName);
             assertEquals(5L, calledTimeout);
             assertArrayEquals(expectedCommand, calledCommand);