diff options
author | Harald Musum <musum@yahooinc.com> | 2023-04-28 14:40:22 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-04-28 14:40:22 +0200 |
commit | 946273381f3efa54f4d5bfe6992cf408dee01a4f (patch) | |
tree | 1887017de5c8a48fbb271706f38bbe38c0b4d33f /config-model | |
parent | 0d2f9fd89a897e9587fdf8a819ea69cc27c4396f (diff) |
Revert "Reapply "Remove access logging for container clusters that are internal""
Diffstat (limited to 'config-model')
9 files changed, 24 insertions, 84 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 31eb3a7bcc2..fef7d534c30 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 @@ -4,8 +4,6 @@ package com.yahoo.vespa.model.admin; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.TreeConfigProducer; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.container.logging.AccessLog; -import com.yahoo.jdisc.http.server.jetty.VoidRequestLog; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.component.Handler; @@ -23,7 +21,6 @@ public class LogserverContainerCluster extends ContainerCluster<LogserverContain addDefaultHandlersWithVip(); addLogHandler(); - addAccessLog(); setJvmGCOptions(deployState.getProperties().jvmGCOptions(Optional.of(ClusterSpec.Type.admin))); } 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 15e3c113d16..c65abfb0189 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 @@ -5,7 +5,6 @@ import com.yahoo.config.model.api.Reindexing; import com.yahoo.config.model.deploy.DeployState; import com.yahoo.config.model.producer.TreeConfigProducer; import com.yahoo.config.provision.ClusterSpec; -import com.yahoo.jdisc.http.server.jetty.VoidRequestLog; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.model.container.ContainerCluster; import com.yahoo.vespa.model.container.PlatformBundles; @@ -31,7 +30,6 @@ public class ClusterControllerContainerCluster extends ContainerCluster<ClusterC TreeConfigProducer<?> parent, String subId, String name, DeployState deployState) { super(parent, subId, name, deployState, false); addDefaultHandlersWithVip(); - addAccessLog(); this.reindexingContext = createReindexingContext(deployState); setJvmGCOptions(deployState.getProperties().jvmGCOptions(Optional.of(ClusterSpec.Type.admin))); } 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 aedd27334e3..ada647b535d 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 @@ -86,10 +86,12 @@ 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); @@ -97,7 +99,6 @@ public class MetricsProxyContainerCluster extends ContainerCluster<MetricsProxyC addPlatformBundle(METRICS_PROXY_BUNDLE_FILE); addClusterComponents(); - addAccessLog(); } @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 e0bf9bac9cf..d101cea428e 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 @@ -24,8 +24,6 @@ import com.yahoo.container.handler.metrics.MetricsV2Handler; import com.yahoo.container.handler.metrics.PrometheusV1Handler; import com.yahoo.container.jdisc.ContainerMbusConfig; import com.yahoo.container.jdisc.messagebus.MbusServerProvider; -import com.yahoo.container.logging.AccessLog; -import com.yahoo.jdisc.http.server.jetty.VoidRequestLog; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.search.config.QrStartConfig; import com.yahoo.vespa.config.search.RankProfilesConfig; @@ -34,7 +32,6 @@ import com.yahoo.vespa.config.search.core.RankingConstantsConfig; import com.yahoo.vespa.config.search.core.RankingExpressionsConfig; import com.yahoo.vespa.model.AbstractService; import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer; -import com.yahoo.vespa.model.container.component.AccessLogComponent; import com.yahoo.vespa.model.container.component.BindingPattern; import com.yahoo.vespa.model.container.component.Component; import com.yahoo.vespa.model.container.component.Handler; @@ -51,7 +48,6 @@ import java.util.Set; import java.util.stream.Collectors; import static com.yahoo.config.model.api.ApplicationClusterEndpoint.RoutingMethod.sharedLayer4; -import static com.yahoo.vespa.model.container.component.AccessLogComponent.AccessLogType.jsonAccessLog; import static com.yahoo.vespa.model.container.docproc.DocprocChains.DOCUMENT_TYPE_MANAGER_CLASS; /** @@ -340,17 +336,6 @@ public final class ApplicationContainerCluster extends ContainerCluster<Applicat protected boolean messageBusEnabled() { return messageBusEnabled; } - public void addAccessLog() { - addSimpleComponent(AccessLog.class); - // 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()); - addComponent(new AccessLogComponent(this, jsonAccessLog, compressionType, clusterName, isHostedVespa)); - } - - public void addVoidAccessLog() { - addSimpleComponent(VoidRequestLog.class); - } - 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 28cc787d1b7..45060f2f27d 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,7 +26,6 @@ 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; @@ -62,7 +61,6 @@ import com.yahoo.vespa.model.container.search.ContainerSearch; import com.yahoo.vespa.model.container.search.searchchain.SearchChains; import com.yahoo.vespa.model.content.Content; import com.yahoo.vespa.model.search.SearchCluster; - import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; @@ -75,7 +73,6 @@ 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; /** @@ -151,9 +148,9 @@ public abstract class ContainerCluster<CONTAINER extends Container> private final Set<Path> platformBundles = new TreeSet<>(); // Ensure stable ordering private final ComponentGroup<Component<?, ?>> componentGroup; - protected final boolean isHostedVespa; + private final boolean isHostedVespa; private final boolean zooKeeperLocalhostAffinity; - protected final String compressionType; + private final String compressionType; private final Map<String, String> concreteDocumentTypes = new LinkedHashMap<>(); @@ -183,6 +180,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> componentGroup = new ComponentGroup<>(this, "component"); addCommonVespaBundles(); + addSimpleComponent(AccessLog.class); addComponent(new DefaultThreadpoolProvider(this, defaultPoolNumThreads)); addComponent(defaultHandlerThreadpool); addSimpleComponent(com.yahoo.concurrent.classlock.ClassLocking.class); @@ -289,7 +287,7 @@ public abstract class ContainerCluster<CONTAINER extends Container> return componentGroup.removeComponent(componentId); } - public void addSimpleComponent(Class<?> clazz) { + private void addSimpleComponent(Class<?> clazz) { addSimpleComponent(clazz.getName()); } @@ -595,14 +593,10 @@ public abstract class ContainerCluster<CONTAINER extends Container> if (containerSearch != null) containerSearch.connectSearchClusters(clusterMap); } - protected void addAccessLog() { /* No access logging by default */ - if (isHostedVespa) { - addSimpleComponent(AccessLog.class); - // 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()); - addComponent(new AccessLogComponent(this, jsonAccessLog, compressionType, clusterName, isHostedVespa)); - } else - addSimpleComponent(VoidRequestLog.class); + 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)); } @Override diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/DefaultThreadpoolProvider.java b/config-model/src/main/java/com/yahoo/vespa/model/container/DefaultThreadpoolProvider.java index 559c7683026..0fdd36b8811 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/DefaultThreadpoolProvider.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/DefaultThreadpoolProvider.java @@ -31,7 +31,7 @@ class DefaultThreadpoolProvider extends SimpleComponent implements ThreadpoolCon public void getConfig(ThreadpoolConfig.Builder builder) { if (cluster instanceof ApplicationContainerCluster) { // Core pool size of 2xcores, and max of 100xcores and using a synchronous Q - // This is the default pool used by both federation and generally when you ask for an Executor. + // This is the deafault pool used by both federation and generally when you ask for an Executor. builder.corePoolSize(-2).maxthreads(-100).queueSize(0); } else { // Container clusters such as logserver, metricsproxy and clustercontroller 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 e81d3fe77e7..96be5190a51 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,10 +36,8 @@ 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; @@ -419,23 +417,19 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { protected void addAccessLogs(DeployState deployState, ApplicationContainerCluster cluster, Element spec) { List<Element> accessLogElements = getAccessLogElements(spec); - if (accessLogElements.isEmpty()) { - if (deployState.getAccessLoggingEnabledByDefault()) - cluster.addAccessLog(); - else - cluster.addVoidAccessLog(); - } - else { - if (cluster.isHostedVespa()) { - log.logApplicationPackage(WARNING, "Applications are not allowed to override the 'accesslog' element"); - } else { - cluster.addSimpleComponent(AccessLog.class); - for (Element accessLog : accessLogElements) { - AccessLogBuilder.buildIfNotDisabled(deployState, cluster, accessLog).ifPresent(cluster::addComponent); - } + if (cluster.isHostedVespa() && !accessLogElements.isEmpty()) { + accessLogElements.clear(); + log.logApplicationPackage( + Level.WARNING, "Applications are not allowed to override the 'accesslog' element"); + } else { + for (Element accessLog : accessLogElements) { + AccessLogBuilder.buildIfNotDisabled(deployState, cluster, accessLog).ifPresent(cluster::addComponent); } } + if (accessLogElements.isEmpty() && deployState.getAccessLoggingEnabledByDefault()) + cluster.addDefaultSearchAccessLog(); + // Add connection log if access log is configured if (cluster.getAllComponents().stream().anyMatch(component -> component instanceof AccessLogComponent)) { // TODO: Vespa > 8: Clean up @@ -451,6 +445,7 @@ 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) { @@ -1228,7 +1223,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { private static final Pattern validPattern = Pattern.compile("-[a-zA-z0-9=:./,+*-]+"); // debug port will not be available in hosted, don't allow - private static final Pattern invalidInHostedPattern = Pattern.compile("-Xrunjdwp:transport=.*"); + private static final Pattern invalidInHostedatttern = Pattern.compile("-Xrunjdwp:transport=.*"); private final Element nodesElement; private final DeployLogger logger; @@ -1281,7 +1276,7 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> { if (isHosted) invalidOptions.addAll(Arrays.stream(optionList) .filter(option -> !option.isEmpty()) - .filter(option -> Pattern.matches(invalidInHostedPattern.pattern(), option)) + .filter(option -> Pattern.matches(invalidInHostedatttern.pattern(), option)) .sorted().toList()); if (invalidOptions.isEmpty()) return; 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 15cfcda7605..a388fdd1b3d 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,10 +15,7 @@ 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; @@ -42,7 +39,6 @@ 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.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; /** @@ -116,25 +112,6 @@ public class MetricsProxyContainerClusterTest { assertNodeConfig(config.node(1)); } - @Test - void no_access_logging_self_hosted() { - VespaModel hostedModel = getModel(servicesWithTwoNodes(), self_hosted); - assertFalse(hasAccessLogComponent(hostedModel.getAdmin().getMetricsProxyCluster())); - } - - @Test - void access_logging_hosted() { - VespaModel hostedModel = getModel(servicesWithTwoNodes(), hosted); - assertTrue(hasAccessLogComponent(hostedModel.getAdmin().getMetricsProxyCluster())); - } - - private boolean hasAccessLogComponent(ContainerCluster<? extends Container> cluster) { - for (Component<?, ?> component : cluster.getAllComponents()) { - if (component instanceof AccessLogComponent) return true; - } - return false; - } - 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 332b6481784..05e83de9157 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,7 +11,6 @@ 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; @@ -71,11 +70,6 @@ public class AccessLogTest extends ContainerModelBuilderTestBase { return cluster.getComponentsMap().get(ComponentId.fromString((JSONAccessLog.class.getName()))); } - private Component<?, ?> getAccessLog(String clusterName) { - ApplicationContainerCluster cluster = (ApplicationContainerCluster) root.getChildren().get(clusterName); - return cluster.getComponentsMap().get(ComponentId.fromString((AccessLog.class.getName()))); - } - @Test void access_log_can_be_configured() { Element clusterElem = DomBuilderTest.parse( @@ -90,7 +84,6 @@ 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()); |