diff --git a/solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java b/solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java index a626e55f7078..6e0fc91179c1 100644 --- a/solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java +++ b/solr/core/src/java/org/apache/solr/util/tracing/SimplePropagator.java @@ -21,14 +21,11 @@ import io.opentelemetry.context.propagation.TextMapGetter; import io.opentelemetry.context.propagation.TextMapPropagator; import io.opentelemetry.context.propagation.TextMapSetter; -import java.lang.invoke.MethodHandles; import java.util.Collection; import java.util.List; import java.util.concurrent.atomic.AtomicLong; import org.apache.solr.common.util.EnvUtils; import org.apache.solr.logging.MDCLoggingContext; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * Simple Http Header Propagator. When enabled, this will only propagate the trace id from the @@ -39,10 +36,9 @@ * replacing/upgrading once that becomes generally available */ public class SimplePropagator implements TextMapPropagator { - private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static final String TRACE_HOST_NAME = - EnvUtils.getProperty("solr.traceHostName", EnvUtils.getProperty("host")); + EnvUtils.getProperty("solr.traceHostName", EnvUtils.getProperty("solr.host.advertise")); private static final TextMapPropagator INSTANCE = new SimplePropagator(); private static final ContextKey TRACE_ID_KEY = ContextKey.named("trace_id"); diff --git a/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java b/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java index ae1aa625991c..d3cb53f5f74c 100644 --- a/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java +++ b/solr/modules/opentelemetry/src/java/org/apache/solr/opentelemetry/OtelTracerConfigurator.java @@ -74,8 +74,8 @@ void prepareConfiguration(NamedList args) { setDefaultIfNotConfigured("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf"); setDefaultIfNotConfigured("OTEL_TRACES_SAMPLER", "parentbased_always_on"); setDefaultIfNotConfigured("OTEL_PROPAGATORS", "tracecontext,baggage"); - if (EnvUtils.getProperty("host") != null) { - addOtelResourceAttributes(Map.of("host.name", EnvUtils.getProperty("host"))); + if (EnvUtils.getProperty("solr.host.advertise") != null) { + addOtelResourceAttributes(Map.of("host.name", EnvUtils.getProperty("solr.host.advertise"))); } final String currentConfig = getCurrentOtelConfigAsString(); diff --git a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/OtelTracerConfiguratorTest.java b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/OtelTracerConfiguratorTest.java index a1cfc7bf10d9..0818e9961a5d 100644 --- a/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/OtelTracerConfiguratorTest.java +++ b/solr/modules/opentelemetry/src/test/org/apache/solr/opentelemetry/OtelTracerConfiguratorTest.java @@ -39,7 +39,7 @@ public void setUp() throws Exception { System.setProperty("otelnothere", "bar"); System.setProperty("otel.k1", "prop-k1"); System.setProperty("otel.k3", "prop-k3"); - System.setProperty("host", "my.solr.host"); + System.setProperty("solr.host.advertise", "my.solr.host"); instance = new OtelTracerConfigurator(currentEnv); } diff --git a/solr/solrj/src/resources/EnvToSyspropMappings.properties b/solr/solrj/src/resources/EnvToSyspropMappings.properties index 176bccf7a136..61fabd7ea8f7 100644 --- a/solr/solrj/src/resources/EnvToSyspropMappings.properties +++ b/solr/solrj/src/resources/EnvToSyspropMappings.properties @@ -1,17 +1,18 @@ # Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements; and to You under the Apache License, Version 2.0. # -# Mapping from Environment variable to system property -# This file only contains non-standard mappings that do not follow the standard naming convention -# Map to nothing to avoid setting any system property for the env.variable -# CamelCase properties are mapped to dot separated lowercase -# This way, env SOLR_FOO_BAR will also match property 'solr.foo.bar' without a mapping in this file -# TODO: Deprecate non-standard sysprops and standardize on solr.foo.bar in Solr 10 +# Mapping from Environment variable to system property. +# This file only contains non-standard mappings that do not follow the standard naming convention. +# Map to nothing to avoid setting any system property for the env.variable. +# Env var names are mapped to sys props by lowercasing and replacing '_' with '.', so +# SOLR_FOO_BAR will match property 'solr.foo.bar' without needing a mapping in this file. +# Separately, sysprop lookups (EnvUtils.getProperty) also fall back from camelCase to +# dot-separated form (e.g. 'solr.logLevel' <-> 'solr.log.level'), independent of this file. +# TODO: Deprecate non-standard sysprops and standardize on solr.foo.bar. AWS_PROFILE=aws.profile -SOLR_ALWAYS_ON_TRACE_ID=solr.alwaysOnTraceId -SOLR_AUTH_JWT_ALLOW_OUTBOUND_HTTP=solr.auth.jwt.allowOutboundHttp -SOLR_HIDDEN_SYS_PROPS=solr.hiddenSysProps +SOLR_ALWAYS_ON_TRACE_ID=solr.tracing.always.on.enabled +SOLR_AUTH_JWT_ALLOW_OUTBOUND_HTTP=solr.auth.jwt.outbound.http.enabled +SOLR_HIDDEN_SYS_PROPS=solr.responses.hidden.sys.props SOLR_HOME=solr.solr.home -SOLR_HOST_ADVERTISE=host SOLR_HTTP_DISABLE_COOKIES=solr.http.disableCookies # Couldn't we just use solr.ip.allowlist everywhere instead of solr.jetty.inetaccess.includes? SOLR_IP_ALLOWLIST=solr.jetty.inetaccess.includes @@ -20,8 +21,8 @@ SOLR_OTEL_DEFAULT_CONFIGURATOR=solr.otelDefaultConfigurator SOLR_TIMEZONE=user.timezone SOLR_TIP=solr.install.dir SOLR_TIP_SYM=solr.install.symDir -ZK_CLIENT_TIMEOUT=zkClientTimeout -ZK_CREATE_CHROOT=createZkChroot +ZK_CLIENT_TIMEOUT=solr.zookeeper.client.timeout +ZK_CREATE_CHROOT=solr.zookeeper.chroot.create ZK_CREDENTIALS_INJECTOR=zkCredentialsInjector ZK_CREDENTIALS_PROVIDER=zkCredentialsProvider ZK_DIGEST=PASSWORD=zkDigestPassword @@ -32,8 +33,8 @@ ZK_DIGEST_USERNAME=zkDigestUsername ZK_HOST=zkHost # Commonly used in solr.xml -SOLR_ALLOW_PATHS=solr.allowPaths -SOLR_ALLOW_URLS=solr.allowUrls +SOLR_ALLOW_PATHS=solr.security.allow.paths +SOLR_ALLOW_URLS=solr.security.allow.urls SOLR_MAX_BOOLEAN_CLAUSES=solr.max.booleanClauses SOLR_METRICS_ENABLED=metricsEnabled SOLR_SHARED_LIB=solr.sharedLib diff --git a/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java b/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java index 346aee08b424..0ca364050fb3 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/EnvUtilsTest.java @@ -17,10 +17,16 @@ package org.apache.solr.common.util; +import java.io.IOException; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.apache.solr.SolrTestCase; +import org.apache.solr.util.LogListener; import org.junit.BeforeClass; import org.junit.Test; @@ -71,15 +77,14 @@ public void getPropWithCamelCase() { assertEquals("INFO", EnvUtils.getProperty("solr.logLevel")); assertEquals("INFO", EnvUtils.getProperty("solr.LogLevel")); assertEquals(Long.valueOf(1234567890L), EnvUtils.getPropertyAsLong("solrLong")); - assertEquals(Boolean.TRUE, EnvUtils.getPropertyAsBool("solr.alwaysOnTraceId")); - assertEquals(Boolean.TRUE, EnvUtils.getPropertyAsBool("solr.always.on.trace.id")); + assertEquals(Boolean.TRUE, EnvUtils.getPropertyAsBool("solr.tracing.always.on.enabled")); } @Test public void testEnvsWithCustomKeyNameMappings() { // These have different names than the environment variables assertEquals(ENV.get("SOLR_HOME"), EnvUtils.getProperty("solr.solr.home")); - assertEquals(ENV.get("SOLR_HOST_ADVERTISE"), EnvUtils.getProperty("host")); + assertEquals(ENV.get("SOLR_HOST_ADVERTISE"), EnvUtils.getProperty("solr.host.advertise")); assertEquals(ENV.get("SOLR_LOGS_DIR"), EnvUtils.getProperty("solr.logs.dir")); } @@ -143,4 +148,91 @@ public void testFlippingDisabledToEnabledPropertyName() { EnvUtils.init(false, env, defaultProps); assertEquals(false, EnvUtils.getPropertyAsBool("solr.ui.enabled")); } + + /** + * These env vars must map directly to their current sysprop name, not to a legacy/intermediate + * name that DeprecatedSystemPropertyMappings.properties also treats as deprecated -- otherwise + * EnvUtils' own deprecation-forwarding logic trips on itself and logs a confusing warning, even + * though the value still resolves correctly via that indirection. A value-only assertion wouldn't + * catch a regression here, since the value resolves fine either way -- the warning is the actual + * symptom, so this asserts on both. + * + *

SOLR_ALWAYS_ON_TRACE_ID is the same pattern (see {@link #getPropWithCamelCase}) but is + * deliberately excluded here: it shares a target sysprop with that other test, and this test + * would clobber it with a non-boolean value depending on random test execution order. + * + *

The LogListener is scoped to only these six properties' names (rather than listening for + * *any* WARN from EnvUtils) because {@code init()} is called here with the real, live {@code + * System.getProperties()} -- its deprecated-property-forwarding loop rescans *all* current system + * properties every time, so leftover deprecated markers set by unrelated tests earlier in this + * same suite/JVM (e.g. {@link #testFlippingDisabledToEnabledPropertyName}) would otherwise be + * re-detected and re-warned-about here too, causing flaky, unrelated failures. + */ + @Test + public void envToSyspropMappingsDoNotTriggerDeprecationWarnings() { + var envVarToExpectedSysprop = + Map.of( + "ZK_CLIENT_TIMEOUT", "solr.zookeeper.client.timeout", + "ZK_CREATE_CHROOT", "solr.zookeeper.chroot.create", + "SOLR_AUTH_JWT_ALLOW_OUTBOUND_HTTP", "solr.auth.jwt.outbound.http.enabled", + "SOLR_HIDDEN_SYS_PROPS", "solr.responses.hidden.sys.props", + "SOLR_ALLOW_PATHS", "solr.security.allow.paths", + "SOLR_ALLOW_URLS", "solr.security.allow.urls"); + var onlyOurTargets = + Pattern.compile( + envVarToExpectedSysprop.values().stream() + .map(Pattern::quote) + .collect(Collectors.joining("|"))); + + try (LogListener warnLog = LogListener.warn(EnvUtils.class).regex(onlyOurTargets)) { + for (var entry : envVarToExpectedSysprop.entrySet()) { + EnvUtils.init(true, Map.of(entry.getKey(), entry.getKey()), System.getProperties()); + assertEquals( + "env var " + entry.getKey() + " should map to " + entry.getValue(), + entry.getKey(), + EnvUtils.getProperty(entry.getValue())); + } + assertEquals( + "No deprecated-property warnings should be logged for these mappings", + 0, + warnLog.getCount()); + } + } + + @Test + public void envToSyspropMappingsDoNotMapToDeprecatedSystemProperties() throws IOException { + Properties envMappings = loadProperties("EnvToSyspropMappings.properties"); + Properties deprecatedMappings = loadProperties("DeprecatedSystemPropertyMappings.properties"); + Map reverseDeprecatedMappings = + deprecatedMappings.entrySet().stream() + .collect(Collectors.toMap(e -> (String) e.getValue(), e -> (String) e.getKey())); + + for (String envVar : envMappings.stringPropertyNames()) { + String sysProp = envMappings.getProperty(envVar); + String newSysProp = reverseDeprecatedMappings.get(sysProp); + if (newSysProp != null) { + fail( + "expected <" + + sysProp + + "> " + + "mapped from <" + + envVar + + "> to not be deprecated, " + + "but it was replaced by <" + + newSysProp + + ">"); + } + } + } + + private static Properties loadProperties(String resourceName) throws IOException { + Properties properties = new Properties(); + try (var resource = + new InputStreamReader( + EnvUtils.class.getClassLoader().getResourceAsStream(resourceName), + StandardCharsets.UTF_8)) { + properties.load(resource); + } + return properties; + } }