Second attempt at overriding access log for hosted configserver/controller

Override addAccessLogs in ConfigServerContainerModelBuilder to override
access log configuration for hosted configserver/controller.
Previous attempt did not work as information in DeployState was not reliable.

3 files changed, 28 insertions, 19 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/AccessLogBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/AccessLogBuilder.java
index 936b9f1c851..a66d98ad52c 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/AccessLogBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/AccessLogBuilder.java
@@ -46,19 +46,17 @@ public class AccessLogBuilder {
     private static class DomBuilder extends VespaDomBuilder.DomConfigProducerBuilder<AccessLogComponent> {
         private final AccessLogType accessLogType;
         private final boolean isHostedVespa;
-        private final boolean isConfigserver;
 
-        public DomBuilder(AccessLogType accessLogType, boolean isHostedVespa, boolean isConfigserver) {
+        public DomBuilder(AccessLogType accessLogType, boolean isHostedVespa) {
             this.accessLogType = accessLogType;
             this.isHostedVespa = isHostedVespa;
-            this.isConfigserver = isConfigserver;
         }
 
         @Override
         protected AccessLogComponent doBuild(DeployState deployState, AbstractConfigProducer<?> ancestor, Element spec) {
             return new AccessLogComponent(
                     accessLogType,
-                    compressionType(spec, deployState, isHostedVespa, isConfigserver),
+                    compressionType(spec, deployState, isHostedVespa),
                     fileNamePattern(spec),
                     rotationInterval(spec),
                     compressOnRotation(spec),
@@ -83,14 +81,13 @@ public class AccessLogBuilder {
             return nullIfEmpty(spec.getAttribute("fileNamePattern"));
         }
 
-        private static CompressionType compressionType(Element spec, DeployState deployState, boolean isHostedVespa, boolean isConfigserver) {
+        private static CompressionType compressionType(Element spec, DeployState deployState, boolean isHostedVespa) {
             CompressionType fallback;
-            if (isHostedVespa && (isConfigserver || deployState.featureFlags().enableZstdCompressionAccessLog())) {
+            if (isHostedVespa && deployState.featureFlags().enableZstdCompressionAccessLog()) {
                 fallback = CompressionType.ZSTD;
             } else {
                 fallback = CompressionType.GZIP;
             }
-            if (isConfigserver && isHostedVespa) return CompressionType.ZSTD;
             return Optional.ofNullable(spec.getAttribute("compressionType"))
                     .filter(value -> !value.isBlank())
                     .map(value -> {
@@ -120,7 +117,7 @@ public class AccessLogBuilder {
         }
     }
 
-    public static Optional<AccessLogComponent> buildIfNotDisabled(DeployState deployState, ContainerCluster<?> cluster, Element accessLogSpec, boolean isConfigserver) {
+    public static Optional<AccessLogComponent> buildIfNotDisabled(DeployState deployState, ContainerCluster<?> cluster, Element accessLogSpec) {
         AccessLogTypeLiteral typeLiteral =
                 getOptionalAttribute(accessLogSpec, "type").
                         map(AccessLogTypeLiteral::fromAttributeValue).
@@ -130,9 +127,6 @@ public class AccessLogBuilder {
             return Optional.empty();
         }
         boolean hosted = cluster.isHostedVespa();
-        if (hosted && isConfigserver && logType != AccessLogType.jsonAccessLog) {
-            return Optional.empty(); // Only enable JSON access logging for hosted configserver/controller
-        }
-        return Optional.of(new DomBuilder(logType, hosted, isConfigserver).build(deployState, cluster, accessLogSpec));
+        return Optional.of(new DomBuilder(logType, hosted).build(deployState, cluster, accessLogSpec));
     }
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java
index e7db4ab0564..8be57ff3bde 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ConfigServerContainerModelBuilder.java
@@ -2,8 +2,12 @@
 package com.yahoo.vespa.model.container.xml;
 
 import com.yahoo.config.model.ConfigModelContext;
+import com.yahoo.config.model.deploy.DeployState;
+import com.yahoo.container.logging.FileConnectionLog;
 import com.yahoo.vespa.model.container.ApplicationContainerCluster;
 import com.yahoo.vespa.model.container.ContainerModel;
+import com.yahoo.vespa.model.container.component.AccessLogComponent;
+import com.yahoo.vespa.model.container.component.ConnectionLogComponent;
 import com.yahoo.vespa.model.container.configserver.ConfigserverCluster;
 import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
 import org.w3c.dom.Element;
@@ -22,8 +26,6 @@ public class ConfigServerContainerModelBuilder extends ContainerModelBuilder {
         this.options = options;
     }
 
-    @Override protected boolean isConfigserver() { return true; }
-
     @Override
     public void doBuild(ContainerModel model, Element spec, ConfigModelContext modelContext) {
         ConfigserverCluster cluster = new ConfigserverCluster(modelContext.getParentProducer(), "configserver",
@@ -36,7 +38,22 @@ public class ConfigServerContainerModelBuilder extends ContainerModelBuilder {
     // in ConfigModelContext.DeployState.properties are not set)
     @Override
     protected void addStatusHandlers(ApplicationContainerCluster cluster, boolean isHostedVespa) {
-        super.addStatusHandlers(cluster, options.hostedVespa().orElse(Boolean.FALSE));
+        super.addStatusHandlers(cluster, isHosted());
+    }
+
+    // Override access log configuration for hosted configserver/controller
+    @Override
+    protected void addAccessLogs(DeployState deployState, ApplicationContainerCluster cluster, Element spec) {
+        if (isHosted()){
+            cluster.addComponent(
+                    new AccessLogComponent(
+                            AccessLogComponent.AccessLogType.jsonAccessLog, AccessLogComponent.CompressionType.ZSTD,
+                            "logs/vespa/configserver/access-json.log.%Y%m%d%H%M%S", null, true, true, "access-json.log"));
+            cluster.addComponent(new ConnectionLogComponent(FileConnectionLog.class, cluster.getName()));
+        } else {
+            super.addAccessLogs(deployState, cluster, spec);
+        }
     }
 
+    private boolean isHosted() { return options.hostedVespa().orElse(Boolean.FALSE); }
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index 07ccf3808fd..7477ccdd970 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -147,8 +147,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         this.httpServerEnabled = networking == Networking.enable;
     }
 
-    protected boolean isConfigserver() { return false; }
-
     @Override
     public List<ConfigModelId> handlesElements() {
         return configModelIds;
@@ -340,11 +338,11 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         addConfiguredComponents(deployState, cluster, spec, "server");
     }
 
-    private void addAccessLogs(DeployState deployState, ApplicationContainerCluster cluster, Element spec) {
+    protected void addAccessLogs(DeployState deployState, ApplicationContainerCluster cluster, Element spec) {
         List<Element> accessLogElements = getAccessLogElements(spec);
 
         for (Element accessLog : accessLogElements) {
-            AccessLogBuilder.buildIfNotDisabled(deployState, cluster, accessLog, isConfigserver()).ifPresent(cluster::addComponent);
+            AccessLogBuilder.buildIfNotDisabled(deployState, cluster, accessLog).ifPresent(cluster::addComponent);
         }
 
         if (accessLogElements.isEmpty() && deployState.getAccessLoggingEnabledByDefault())