Revert "Revert "Remove access logging for internal container clusters""

11 files changed, 99 insertions, 41 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java
index 33915d48bd4..4b1467158ef 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/LogserverContainerCluster.java
@@ -6,7 +6,6 @@ import com.yahoo.config.model.producer.TreeConfigProducer;
 import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.search.config.QrStartConfig;
 import com.yahoo.vespa.model.container.ContainerCluster;
-import com.yahoo.vespa.model.container.component.AccessLogComponent;
 import com.yahoo.vespa.model.container.component.Handler;
 import com.yahoo.vespa.model.container.component.SystemBindingPattern;
 
@@ -24,11 +23,7 @@ public class LogserverContainerCluster extends ContainerCluster<LogserverContain
         addLogHandler();
         setJvmGCOptions(deployState.getProperties().jvmGCOptions(Optional.of(ClusterSpec.Type.admin)));
         if (isHostedVespa())
-            addComponent(new AccessLogComponent(this,
-                                                AccessLogComponent.AccessLogType.jsonAccessLog,
-                                                deployState.featureFlags().logFileCompressionAlgorithm("zstd"),
-                                                Optional.of(getName()),
-                                                isHostedVespa()));
+            addAccessLog(getName());
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
index 7b465e3eb48..14b18cd8711 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/clustercontroller/ClusterControllerContainerCluster.java
@@ -8,7 +8,6 @@ import com.yahoo.config.provision.ClusterSpec;
 import com.yahoo.search.config.QrStartConfig;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.PlatformBundles;
-import com.yahoo.vespa.model.container.component.AccessLogComponent;
 
 import java.nio.file.Path;
 import java.util.Collections;
@@ -34,11 +33,7 @@ public class ClusterControllerContainerCluster extends ContainerCluster<ClusterC
         this.reindexingContext = createReindexingContext(deployState);
         setJvmGCOptions(deployState.getProperties().jvmGCOptions(Optional.of(ClusterSpec.Type.admin)));
         if (isHostedVespa())
-            addComponent(new AccessLogComponent(this,
-                                                AccessLogComponent.AccessLogType.jsonAccessLog,
-                                                deployState.featureFlags().logFileCompressionAlgorithm("zstd"),
-                                                Optional.of("controller"),
-                                                isHostedVespa()));
+            addAccessLog("controller");
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
index 78374cd4030..58e624960f9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerCluster.java
@@ -32,7 +32,6 @@ import com.yahoo.vespa.model.admin.monitoring.MetricsConsumer;
 import com.yahoo.vespa.model.admin.monitoring.Monitoring;
 import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.PlatformBundles;
-import com.yahoo.vespa.model.container.component.AccessLogComponent;
 import com.yahoo.vespa.model.container.component.Handler;
 import com.yahoo.vespa.model.container.component.SystemBindingPattern;
 
@@ -87,12 +86,10 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
         static final String LEGACY_APPLICATION = "app";        // app.instance
     }
 
-    private final TreeConfigProducer<?> parent;
     private final ApplicationId applicationId;
 
     public MetricsProxyContainerCluster(TreeConfigProducer<?> parent, String name, DeployState deployState) {
         super(parent, name, name, deployState, true);
-        this.parent = parent;
         applicationId = deployState.getProperties().applicationId();
 
         setRpcServerEnabled(true);
@@ -101,11 +98,7 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC
         addPlatformBundle(METRICS_PROXY_BUNDLE_FILE);
         addClusterComponents();
         if (isHostedVespa())
-            addComponent(new AccessLogComponent(this,
-                                                AccessLogComponent.AccessLogType.jsonAccessLog,
-                                                "zstd",
-                                                Optional.of("metrics-proxy"),
-                                                isHostedVespa()));
+            addAccessLog("metrics-proxy");
     }
 
     @Override
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
index d101cea428e..6977a5ca465 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ApplicationContainerCluster.java
@@ -336,6 +336,12 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat
 
     protected boolean messageBusEnabled() { return messageBusEnabled; }
 
+    public void addAccessLog() {
+        // In hosted there is one application container per node, so we do not use the container name to distinguish log files
+        Optional<String> clusterName = isHostedVespa ? Optional.empty() : Optional.of(getName());
+        addAccessLog(clusterName);
+    }
+
     public void addMbusServer(ComponentId chainId) {
         ComponentId serviceId = chainId.nestInNamespace(ComponentId.fromString("MbusServer"));
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
index 45060f2f27d..6bbc24e8739 100755
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
@@ -26,6 +26,7 @@ import com.yahoo.container.jdisc.state.StateHandler;
 import com.yahoo.container.logging.AccessLog;
 import com.yahoo.container.usability.BindingsOverviewHandler;
 import com.yahoo.document.config.DocumentmanagerConfig;
+import com.yahoo.jdisc.http.server.jetty.VoidRequestLog;
 import com.yahoo.osgi.provider.model.ComponentModel;
 import com.yahoo.prelude.semantics.SemanticRulesConfig;
 import com.yahoo.search.config.IndexInfoConfig;
@@ -73,6 +74,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.TreeSet;
 
+import static com.yahoo.vespa.model.container.component.AccessLogComponent.AccessLogType.jsonAccessLog;
 import static com.yahoo.vespa.model.container.component.chain.ProcessingHandler.PROCESSING_HANDLER_CLASS;
 
 /**
@@ -148,7 +150,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
     private final Set<Path> platformBundles = new TreeSet<>(); // Ensure stable ordering
 
     private final ComponentGroup<Component<?, ?>> componentGroup;
-    private final boolean isHostedVespa;
+    protected final boolean isHostedVespa;
     private final boolean zooKeeperLocalhostAffinity;
     private final String compressionType;
 
@@ -169,6 +171,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
     public ContainerCluster(TreeConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState, boolean zooKeeperLocalhostAffinity) {
         this(parent, configSubId, clusterId, deployState, zooKeeperLocalhostAffinity, 1);
     }
+
     public ContainerCluster(TreeConfigProducer<?> parent, String configSubId, String clusterId, DeployState deployState, boolean zooKeeperLocalhostAffinity, int defaultPoolNumThreads) {
         super(parent, configSubId);
         this.name = clusterId;
@@ -180,7 +183,7 @@ public abstract class ContainerCluster<CONTAINER extends Container>
         componentGroup = new ComponentGroup<>(this, "component");
 
         addCommonVespaBundles();
-        addSimpleComponent(AccessLog.class);
+        addSimpleComponent(VoidRequestLog.class);
         addComponent(new DefaultThreadpoolProvider(this, defaultPoolNumThreads));
         addComponent(defaultHandlerThreadpool);
         addSimpleComponent(com.yahoo.concurrent.classlock.ClassLocking.class);
@@ -287,7 +290,11 @@ public abstract class ContainerCluster<CONTAINER extends Container>
         return componentGroup.removeComponent(componentId);
     }
 
-    private void addSimpleComponent(Class<?> clazz) {
+    public void removeSimpleComponent(Class<?> clazz) {
+        removeComponent(new SimpleComponent(clazz.getName()).getComponentId());
+    }
+
+    public void addSimpleComponent(Class<?> clazz) {
         addSimpleComponent(clazz.getName());
     }
 
@@ -593,12 +600,23 @@ public abstract class ContainerCluster<CONTAINER extends Container>
         if (containerSearch != null) containerSearch.connectSearchClusters(clusterMap);
     }
 
-    public void addDefaultSearchAccessLog() {
-        // In hosted Vespa with one application container per node we do not use the container name to distinguish log files
-        Optional<String> clusterName = isHostedVespa ? Optional.empty() : Optional.of(getName());
-        addComponent(new AccessLogComponent(this, AccessLogComponent.AccessLogType.jsonAccessLog, compressionType, clusterName, isHostedVespa));
+    public void addAccessLog(String clusterName) {
+        addAccessLog(Optional.ofNullable(clusterName));
     }
 
+    public void addAccessLog(String fileNamePattern, String symlinkName) {
+        removeSimpleComponent(VoidRequestLog.class);
+        addSimpleComponent(AccessLog.class);
+        addComponent(new AccessLogComponent(jsonAccessLog, compressionType, fileNamePattern, null, true, true, symlinkName, 1024, 256 * 1024));
+    }
+
+    protected void addAccessLog(Optional<String> clusterName) {
+        removeSimpleComponent(VoidRequestLog.class);
+        addSimpleComponent(AccessLog.class);
+        addComponent(new AccessLogComponent(this, jsonAccessLog, compressionType, clusterName, isHostedVespa));
+    }
+
+
     @Override
     public void getConfig(IlscriptsConfig.Builder builder) {
         for (SearchCluster searchCluster : Content.getSearchClusters(getRoot().configModelRepo())) {
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 49361b887c4..883dbebd34d 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
@@ -3,7 +3,9 @@ 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.AccessLog;
 import com.yahoo.container.logging.FileConnectionLog;
+import com.yahoo.jdisc.http.server.jetty.VoidRequestLog;
 import com.yahoo.vespa.model.container.ApplicationContainerCluster;
 import com.yahoo.vespa.model.container.ContainerModel;
 import com.yahoo.vespa.model.container.component.AccessLogComponent;
@@ -12,6 +14,8 @@ import com.yahoo.vespa.model.container.configserver.ConfigserverCluster;
 import com.yahoo.vespa.model.container.configserver.option.CloudConfigOptions;
 import org.w3c.dom.Element;
 
+import static com.yahoo.vespa.model.container.component.AccessLogComponent.AccessLogType.jsonAccessLog;
+
 /**
  * Builds the config model for the standalone config server.
  *
@@ -45,10 +49,7 @@ public class ConfigServerContainerModelBuilder extends ContainerModelBuilder {
     @Override
     protected void addAccessLogs(DeployState deployState, ApplicationContainerCluster cluster, Element spec) {
         if (isHosted()){
-            cluster.addComponent(
-                    new AccessLogComponent(
-                            AccessLogComponent.AccessLogType.jsonAccessLog, "zstd",
-                            "logs/vespa/configserver/access-json.log.%Y%m%d%H%M%S", null, true, true, "access-json.log", 1024,256*1024));
+            cluster.addAccessLog("logs/vespa/configserver/access-json.log.%Y%m%d%H%M%S", "access-json.log");
             cluster.addComponent(new ConnectionLogComponent(cluster, FileConnectionLog.class, "configserver"));
         } else {
             super.addAccessLogs(deployState, cluster, spec);
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 a1e88092195..3d1c8ca1d76 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
@@ -36,8 +36,10 @@ import com.yahoo.config.provision.Zone;
 import com.yahoo.config.provision.ZoneEndpoint;
 import com.yahoo.config.provision.zone.ZoneId;
 import com.yahoo.container.bundle.BundleInstantiationSpecification;
+import com.yahoo.container.logging.AccessLog;
 import com.yahoo.container.logging.FileConnectionLog;
 import com.yahoo.io.IOUtils;
+import com.yahoo.jdisc.http.server.jetty.VoidRequestLog;
 import com.yahoo.osgi.provider.model.ComponentModel;
 import com.yahoo.path.Path;
 import com.yahoo.schema.OnnxModel;
@@ -417,19 +419,26 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
     protected void addAccessLogs(DeployState deployState, ApplicationContainerCluster cluster, Element spec) {
         List<Element> accessLogElements = getAccessLogElements(spec);
 
-        if (cluster.isHostedVespa() && !accessLogElements.isEmpty()) {
-            accessLogElements.clear();
-            log.logApplicationPackage(
-                    Level.WARNING, "Applications are not allowed to override the 'accesslog' element");
+        if (accessLogElements.isEmpty() && deployState.getAccessLoggingEnabledByDefault()) {
+            cluster.addAccessLog();
         } else {
-            for (Element accessLog : accessLogElements) {
-                AccessLogBuilder.buildIfNotDisabled(deployState, cluster, accessLog).ifPresent(cluster::addComponent);
+            if (cluster.isHostedVespa()) {
+                log.logApplicationPackage(WARNING, "Applications are not allowed to override the 'accesslog' element");
+            } else {
+                List<AccessLogComponent> components = new ArrayList<>();
+                for (Element accessLog : accessLogElements) {
+                    AccessLogBuilder.buildIfNotDisabled(deployState, cluster, accessLog).ifPresent(accessLogComponent -> {
+                        components.add(accessLogComponent);
+                        cluster.addComponent(accessLogComponent);
+                    });
+                }
+                if (components.size() > 0) {
+                    cluster.removeSimpleComponent(VoidRequestLog.class);
+                    cluster.addSimpleComponent(AccessLog.class);
+                }
             }
         }
 
-        if (accessLogElements.isEmpty() && deployState.getAccessLoggingEnabledByDefault())
-            cluster.addDefaultSearchAccessLog();
-
         // Add connection log if access log is configured
         if (cluster.getAllComponents().stream().anyMatch(component -> component instanceof AccessLogComponent))
             cluster.addComponent(new ConnectionLogComponent(cluster, FileConnectionLog.class, "access"));
@@ -439,7 +448,6 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
         return XML.getChildren(spec, "accesslog");
     }
 
-
     protected void addHttp(DeployState deployState, Element spec, ApplicationContainerCluster cluster, ConfigModelContext context) {
         Element httpElement = XML.getChild(spec, "http");
         if (httpElement != null) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java
index 519fc967511..e044b97546c 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/Content.java
@@ -311,6 +311,7 @@ public class Content extends ConfigModel {
 
             indexingCluster.addDefaultHandlersWithVip();
             indexingCluster.addAllPlatformBundles();
+            indexingCluster.addAccessLog();
             addDocproc(indexingCluster);
 
             List<ApplicationContainer> nodes = new ArrayList<>();
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java
index a388fdd1b3d..0390ea14fa4 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/admin/metricsproxy/MetricsProxyContainerClusterTest.java
@@ -15,7 +15,10 @@ import com.yahoo.container.core.ApplicationMetadataConfig;
 import com.yahoo.container.di.config.PlatformBundlesConfig;
 import com.yahoo.vespa.model.VespaModel;
 import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainerCluster.AppDimensionNames;
+import com.yahoo.vespa.model.container.Container;
+import com.yahoo.vespa.model.container.ContainerCluster;
 import com.yahoo.vespa.model.container.PlatformBundles;
+import com.yahoo.vespa.model.container.component.AccessLogComponent;
 import com.yahoo.vespa.model.container.component.Component;
 import com.yahoo.vespa.model.container.component.Handler;
 import org.junit.jupiter.api.Test;
@@ -39,6 +42,8 @@ import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.g
 import static com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyModelTester.servicesWithAdminOnly;
 import static java.util.stream.Collectors.toSet;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
@@ -112,6 +117,29 @@ public class MetricsProxyContainerClusterTest {
         assertNodeConfig(config.node(1));
     }
 
+    @Test
+    void no_access_logging_self_hosted() {
+        VespaModel hostedModel = getModel(servicesWithTwoNodes(), self_hosted);
+        assertNull(getAccessLogComponent(hostedModel.getAdmin().getMetricsProxyCluster()));
+    }
+
+    @Test
+    void access_logging_hosted() {
+        VespaModel hostedModel = getModel(servicesWithTwoNodes(), hosted);
+        var accessLog = getAccessLogComponent(hostedModel.getAdmin().getMetricsProxyCluster());
+        assertNotNull(accessLog);
+        assertEquals("logs/vespa/access/JsonAccessLog.metrics-proxy.%Y%m%d%H%M%S", accessLog.getFileNamePattern());
+    }
+
+    private AccessLogComponent getAccessLogComponent(ContainerCluster<? extends Container> cluster) {
+        for (Component<?, ?> component : cluster.getAllComponents()) {
+            if (component instanceof AccessLogComponent)
+                return (AccessLogComponent) component;
+        }
+        return null;
+    }
+
+
     private void assertNodeConfig(MetricsNodesConfig.Node node) {
         assertTrue(node.role().startsWith("container/foo/0/"));
         assertTrue(node.hostname().startsWith("node-1-3-50-"));
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java
index 05e83de9157..fbf7454de50 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessLogTest.java
@@ -11,6 +11,7 @@ import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.container.core.AccessLogConfig;
+import com.yahoo.container.logging.AccessLog;
 import com.yahoo.container.logging.ConnectionLogConfig;
 import com.yahoo.container.logging.FileConnectionLog;
 import com.yahoo.container.logging.JSONAccessLog;
@@ -84,6 +85,7 @@ public class AccessLogTest extends ContainerModelBuilderTestBase {
         createModel(root, clusterElem);
         assertNotNull(getJsonAccessLog("default"));
         assertNotNull(getVespaAccessLog("default"));
+        assertNotNull(getAccessLog("default"));
 
         { // vespa
             Component<?, ?> accessLogComponent = getComponent("default", VespaAccessLog.class.getName());
@@ -174,4 +176,9 @@ public class AccessLogTest extends ContainerModelBuilderTestBase {
                 logger.msgs.get(0).getSecond());
     }
 
+    private Component<?, ?> getAccessLog(String clusterName) {
+        ApplicationContainerCluster cluster = (ApplicationContainerCluster) root.getChildren().get(clusterName);
+        return cluster.getComponentsMap().get(ComponentId.fromString((AccessLog.class.getName())));
+    }
+
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexingAndDocprocRoutingTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexingAndDocprocRoutingTest.java
index 430628238d9..4476e128196 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexingAndDocprocRoutingTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexingAndDocprocRoutingTest.java
@@ -1,6 +1,9 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.model.content;
 
+import com.yahoo.component.ComponentId;
+import com.yahoo.container.logging.AccessLog;
+import com.yahoo.container.logging.JSONAccessLog;
 import com.yahoo.messagebus.routing.Hop;
 import com.yahoo.messagebus.routing.HopBlueprint;
 import com.yahoo.messagebus.routing.PolicyDirective;
@@ -243,6 +246,9 @@ public class IndexingAndDocprocRoutingTest extends ContentBaseTest {
             }
 
             assertTrue(actualDocprocChains.containsAll(expectedDocprocChainStrings));
+
+            assertNotNull(docprocCluster.getComponentsMap().get(ComponentId.fromString(AccessLog.class.getName())));
+            assertNotNull(docprocCluster.getComponentsMap().get(ComponentId.fromString(JSONAccessLog.class.getName())));
         }
     }