From 90d52bfa46fb3264a9c6e972650d76aa72849807 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Mon, 8 May 2023 10:58:09 +0200 Subject: Revert "Revert "Remove access logging for internal container clusters"" --- .../model/admin/LogserverContainerCluster.java | 7 +---- .../ClusterControllerContainerCluster.java | 7 +---- .../metricsproxy/MetricsProxyContainerCluster.java | 9 +----- .../container/ApplicationContainerCluster.java | 6 ++++ .../vespa/model/container/ContainerCluster.java | 32 +++++++++++++++++----- .../xml/ConfigServerContainerModelBuilder.java | 9 +++--- .../model/container/xml/ContainerModelBuilder.java | 28 ++++++++++++------- .../com/yahoo/vespa/model/content/Content.java | 1 + .../MetricsProxyContainerClusterTest.java | 28 +++++++++++++++++++ .../vespa/model/container/xml/AccessLogTest.java | 7 +++++ .../content/IndexingAndDocprocRoutingTest.java | 6 ++++ 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 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 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 private final Set platformBundles = new TreeSet<>(); // Ensure stable ordering private final ComponentGroup> 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 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 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 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 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 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 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 { protected void addAccessLogs(DeployState deployState, ApplicationContainerCluster cluster, Element spec) { List 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 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 { 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 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 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()))); } } -- cgit v1.2.3