Skip to content

Commit 88250c2

Browse files
masseykemasseyke
authored
Exposing the ability to log deprecated settings at non-critical level (#79107)
A recent change for the deprecation logs provided the capability to emit deprecation's at critical vs. warning levels, #77482. However deprecated settings always log at critical level without the ability to express that the setting deprecation is only a warning. This commit exposes the ability to set the deprecation level when deprecating a setting. Closes #78781
1 parent dfbaa66 commit 88250c2

File tree

21 files changed

+245
-104
lines changed

21 files changed

+245
-104
lines changed

‎client/rest/src/main/java/org/elasticsearch/client/Response.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,14 +136,14 @@ public HttpEntity getEntity() {
136136

137137
/**
138138
* Tests if a string matches the RFC 7234 specification for warning headers.
139-
* This assumes that the warn code is always 299 and the warn agent is always
140-
* Elasticsearch.
139+
* This assumes that the warn code is always 299 or 300 and the warn agent is
140+
* always Elasticsearch.
141141
*
142142
* @param s the value of a warning header formatted according to RFC 7234
143143
* @return {@code true} if the input string matches the specification
144144
*/
145145
private static boolean matchWarningHeaderPatternByPrefix(final String s) {
146-
return s.startsWith("299 Elasticsearch-");
146+
return s.startsWith("299 Elasticsearch-") || s.startsWith("300 Elasticsearch-");
147147
}
148148

149149
/**

‎libs/x-content/src/test/java/org/elasticsearch/xcontent/ConstructingObjectParserTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -626,8 +626,8 @@ public void testCompatibleFieldDeclarations() throws IOException {
626626
RestApiVersion.minimumSupported());
627627
StructWithCompatibleFields o = StructWithCompatibleFields.PARSER.parse(parser, null);
628628
assertEquals(1, o.intField);
629-
assertWarnings(false, "[struct_with_compatible_fields][1:14] " +
630-
"Deprecated field [old_name] used, expected [new_name] instead");
629+
assertWarnings(false, new DeprecationWarning(DeprecationLogger.CRITICAL, "[struct_with_compatible_fields][1:14] " +
630+
"Deprecated field [old_name] used, expected [new_name] instead"));
631631
}
632632
}
633633

‎libs/x-content/src/test/java/org/elasticsearch/xcontent/ObjectParserTests.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
package org.elasticsearch.xcontent;
99

10+
import org.elasticsearch.common.logging.DeprecationLogger;
1011
import org.elasticsearch.common.xcontent.XContentParserUtils;
1112
import org.elasticsearch.core.CheckedFunction;
1213
import org.elasticsearch.core.RestApiVersion;
@@ -211,7 +212,8 @@ class TestStruct {
211212
objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING);
212213
objectParser.parse(parser, s, null);
213214
assertEquals("foo", s.test);
214-
assertWarnings(false, "[foo][1:15] Deprecated field [old_test] used, expected [test] instead");
215+
assertWarnings(false, new DeprecationWarning(DeprecationLogger.CRITICAL, "[foo][1:15] Deprecated field [old_test] used, " +
216+
"expected [test] instead"));
215217
}
216218

217219
public void testFailOnValueType() throws IOException {
@@ -1072,8 +1074,8 @@ public void testCompatibleFieldDeclarations() throws IOException {
10721074
RestApiVersion.minimumSupported());
10731075
StructWithCompatibleFields o = StructWithCompatibleFields.PARSER.parse(parser, null);
10741076
assertEquals(1, o.intField);
1075-
assertWarnings(false, "[struct_with_compatible_fields][1:14] " +
1076-
"Deprecated field [old_name] used, expected [new_name] instead");
1077+
assertWarnings(false, new DeprecationWarning(DeprecationLogger.CRITICAL, "[struct_with_compatible_fields][1:14] " +
1078+
"Deprecated field [old_name] used, expected [new_name] instead"));
10771079

10781080
}
10791081
}

‎modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/GeoIpProcessor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -470,8 +470,8 @@ public Processor create(
470470

471471
boolean valid = metadata.isValid(currentState.metadata().settings());
472472
if (valid && metadata.isCloseToExpiration()) {
473-
HeaderWarning.addWarning("database [{}] was not updated for over 25 days, geoip processor will stop working if there " +
474-
"is no update for 30 days", databaseFile);
473+
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, "database [{}] was not updated for over 25 days, geoip processor" +
474+
" will stop working if there is no update for 30 days", databaseFile);
475475
}
476476

477477
return valid;

‎plugins/discovery-ec2/src/test/java/org/elasticsearch/discovery/ec2/AwsEc2ServiceImplTests.java

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@
1414
import com.amazonaws.auth.AWSCredentialsProvider;
1515
import com.amazonaws.auth.BasicSessionCredentials;
1616
import com.amazonaws.auth.DefaultAWSCredentialsProviderChain;
17+
18+
import org.elasticsearch.common.logging.DeprecationLogger;
1719
import org.elasticsearch.common.settings.MockSecureSettings;
20+
import org.elasticsearch.common.settings.Setting;
1821
import org.elasticsearch.common.settings.Settings;
1922
import org.elasticsearch.common.settings.SettingsException;
2023
import org.elasticsearch.test.ESTestCase;
@@ -59,8 +62,9 @@ public void testDeprecationOfLoneAccessKey() {
5962
Ec2ClientSettings.getClientSettings(Settings.builder().setSecureSettings(secureSettings).build())).getCredentials();
6063
assertThat(credentials.getAWSAccessKeyId(), is("aws_key"));
6164
assertThat(credentials.getAWSSecretKey(), is(""));
62-
assertSettingDeprecationsAndWarnings(new String[]{},
63-
"Setting [discovery.ec2.access_key] is set but [discovery.ec2.secret_key] is not, which will be unsupported in future");
65+
assertSettingDeprecationsAndWarnings(new Setting<?>[]{},
66+
new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.access_key] is set but " +
67+
"[discovery.ec2.secret_key] is not, which will be unsupported in future"));
6468
}
6569

6670
public void testDeprecationOfLoneSecretKey() {
@@ -70,8 +74,9 @@ public void testDeprecationOfLoneSecretKey() {
7074
Ec2ClientSettings.getClientSettings(Settings.builder().setSecureSettings(secureSettings).build())).getCredentials();
7175
assertThat(credentials.getAWSAccessKeyId(), is(""));
7276
assertThat(credentials.getAWSSecretKey(), is("aws_secret"));
73-
assertSettingDeprecationsAndWarnings(new String[]{},
74-
"Setting [discovery.ec2.secret_key] is set but [discovery.ec2.access_key] is not, which will be unsupported in future");
77+
assertSettingDeprecationsAndWarnings(new Setting<?>[]{},
78+
new DeprecationWarning(DeprecationLogger.CRITICAL, "Setting [discovery.ec2.secret_key] is set but " +
79+
"[discovery.ec2.access_key] is not, which will be unsupported in future"));
7580
}
7681

7782
public void testRejectionOfLoneSessionToken() {

‎qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ public void testDeprecationWarnMessage() throws IOException {
100100
);
101101
}
102102

103-
assertWarnings("deprecated warn message1");
103+
assertWarnings(true, new DeprecationWarning(Level.WARN, "deprecated warn message1")) ;
104104
}
105105

106106
public void testDeprecatedMessageWithoutXOpaqueId() throws IOException {

‎server/src/main/java/org/elasticsearch/cluster/metadata/MetadataIndexTemplateService.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.elasticsearch.cluster.ClusterState;
2525
import org.elasticsearch.cluster.ClusterStateUpdateTask;
2626
import org.elasticsearch.cluster.service.ClusterService;
27+
import org.elasticsearch.common.logging.DeprecationLogger;
2728
import org.elasticsearch.core.Nullable;
2829
import org.elasticsearch.common.Priority;
2930
import org.elasticsearch.common.Strings;
@@ -498,7 +499,7 @@ public ClusterState addIndexTemplateV2(final ClusterState currentState, final bo
498499
.collect(Collectors.joining(",")),
499500
name);
500501
logger.warn(warning);
501-
HeaderWarning.addWarning(warning);
502+
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning);
502503
}
503504

504505
ComposableIndexTemplate finalIndexTemplate = template;
@@ -828,7 +829,7 @@ static ClusterState innerPutTemplate(final ClusterState currentState, PutRequest
828829
.collect(Collectors.joining(",")),
829830
request.name);
830831
logger.warn(warning);
831-
HeaderWarning.addWarning(warning);
832+
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning);
832833
} else {
833834
// Otherwise, this is a hard error, the user should use V2 index templates instead
834835
String error = String.format(Locale.ROOT, "legacy template [%s] has index patterns %s matching patterns" +

‎server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.elasticsearch.common.logging;
1010

11+
import org.apache.logging.log4j.Level;
1112
import org.elasticsearch.Build;
1213
import org.elasticsearch.Version;
1314
import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -32,10 +33,11 @@
3233
public class HeaderWarning {
3334
/**
3435
* Regular expression to test if a string matches the RFC7234 specification for warning headers. This pattern assumes that the warn code
35-
* is always 299. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build hash.
36+
* is always 299 or 300. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build
37+
* hash.
3638
*/
3739
public static final Pattern WARNING_HEADER_PATTERN = Pattern.compile(
38-
"299 " + // warn code
40+
"(?:299|300) " + // log level code
3941
"Elasticsearch-" + // warn agent
4042
"\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-" + // warn agent
4143
"(?:[a-f0-9]{7}(?:[a-f0-9]{33})?|unknown) " + // warn agent
@@ -53,15 +55,16 @@ public class HeaderWarning {
5355

5456
/*
5557
* RFC7234 specifies the warning format as warn-code <space> warn-agent <space> "warn-text" [<space> "warn-date"]. Here, warn-code is a
56-
* three-digit number with various standard warn codes specified. The warn code 299 is apt for our purposes as it represents a
57-
* miscellaneous persistent warning (can be presented to a human, or logged, and must not be removed by a cache). The warn-agent is an
58-
* arbitrary token; here we use the Elasticsearch version and build hash. The warn text must be quoted. The warn-date is an optional
59-
* quoted field that can be in a variety of specified date formats; here we use RFC 1123 format.
58+
* three-digit number with various standard warn codes specified, and is left off of this static prefix so that it can be added based
59+
* on the log level received. The warn code will be either 299 or 300 at runtime, which are apt for our purposes as
60+
* they represent miscellaneous persistent warnings (can be presented to a human, or logged, and must not be removed by a cache).
61+
* The warn-agent is an arbitrary token; here we use the Elasticsearch version and build hash. The warn text must be quoted. The
62+
* warn-date is an optional quoted field that can be in a variety of specified date formats; here we use RFC 1123 format.
6063
*/
6164
private static final String WARNING_PREFIX =
6265
String.format(
6366
Locale.ROOT,
64-
"299 Elasticsearch-%s%s-%s",
67+
" Elasticsearch-%s%s-%s",
6568
Version.CURRENT.toString(),
6669
Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "",
6770
Build.CURRENT.hash());
@@ -189,14 +192,15 @@ private static boolean assertWarningValue(final String s, final String warningVa
189192
* Format a warning string in the proper warning format by prepending a warn code, warn agent, wrapping the warning string in quotes,
190193
* and appending the RFC 7231 date.
191194
*
195+
* @param level the level of the warning - Level.WARN or DeprecationLogger.CRITICAL
192196
* @param s the warning string to format
193197
* @return a warning value formatted according to RFC 7234
194198
*/
195-
public static String formatWarning(final String s) {
199+
public static String formatWarning(final Level level, final String s) {
196200
// Assume that the common scenario won't have a string to escape and encode.
197-
int length = WARNING_PREFIX.length() + s.length() + 3;
201+
int length = WARNING_PREFIX.length() + s.length() + 6;
198202
final StringBuilder sb = new StringBuilder(length);
199-
sb.append(WARNING_PREFIX).append(" \"").append(escapeAndEncode(s)).append("\"");
203+
sb.append(level.intLevel() + WARNING_PREFIX).append(" \"").append(escapeAndEncode(s)).append("\"");
200204
return sb.toString();
201205
}
202206

@@ -310,16 +314,21 @@ public static String getXOpaqueId() {
310314
.orElse("");
311315
}
312316

313-
public static void addWarning(String message, Object... params) {
314-
addWarning(THREAD_CONTEXT, message, params);
317+
public static void addWarning(Level level, String message, Object... params) {
318+
addWarning(THREAD_CONTEXT, level, message, params);
315319
}
316320

317321
// package scope for testing
318322
static void addWarning(Set<ThreadContext> threadContexts, String message, Object... params) {
323+
addWarning(threadContexts, DeprecationLogger.CRITICAL, message, params);
324+
}
325+
326+
// package scope for testing
327+
static void addWarning(Set<ThreadContext> threadContexts, Level level, String message, Object... params) {
319328
final Iterator<ThreadContext> iterator = threadContexts.iterator();
320329
if (iterator.hasNext()) {
321330
final String formattedMessage = LoggerMessageFormat.format(message, params);
322-
final String warningHeaderValue = formatWarning(formattedMessage);
331+
final String warningHeaderValue = formatWarning(level, formattedMessage);
323332
assert WARNING_HEADER_PATTERN.matcher(warningHeaderValue).matches();
324333
assert extractWarningValueFromWarningHeader(warningHeaderValue, false)
325334
.equals(escapeAndEncode(formattedMessage));

‎server/src/main/java/org/elasticsearch/common/logging/HeaderWarningAppender.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ public void append(LogEvent event) {
3535
String messagePattern = esLogMessage.getMessagePattern();
3636
Object[] arguments = esLogMessage.getArguments();
3737

38-
HeaderWarning.addWarning(messagePattern, arguments);
38+
HeaderWarning.addWarning(event.getLevel(), messagePattern, arguments);
3939
} else {
4040
final String formattedMessage = event.getMessage().getFormattedMessage();
41-
HeaderWarning.addWarning(formattedMessage);
41+
HeaderWarning.addWarning(event.getLevel(), formattedMessage);
4242
}
4343
}
4444

‎server/src/main/java/org/elasticsearch/common/settings/Setting.java

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,15 @@ public enum Property {
9999
Final,
100100

101101
/**
102-
* mark this setting as deprecated
102+
* mark this setting as deprecated (critical level)
103103
*/
104104
Deprecated,
105105

106+
/**
107+
* mark this setting as deprecated (warning level)
108+
*/
109+
DeprecatedWarning,
110+
106111
/**
107112
* Node scope
108113
*/
@@ -169,6 +174,9 @@ private Setting(Key key, @Nullable Setting<T> fallbackSetting, Function<Settings
169174
if (propertiesAsSet.contains(Property.Dynamic) && propertiesAsSet.contains(Property.OperatorDynamic)) {
170175
throw new IllegalArgumentException("setting [" + key + "] cannot be both dynamic and operator dynamic");
171176
}
177+
if (propertiesAsSet.contains(Property.Deprecated) && propertiesAsSet.contains(Property.DeprecatedWarning)) {
178+
throw new IllegalArgumentException("setting [" + key + "] cannot be deprecated at both critical and warning levels");
179+
}
172180
checkPropertyRequiresIndexScope(propertiesAsSet, Property.NotCopyableOnResize);
173181
checkPropertyRequiresIndexScope(propertiesAsSet, Property.InternalIndex);
174182
checkPropertyRequiresIndexScope(propertiesAsSet, Property.PrivateIndex);
@@ -370,8 +378,12 @@ public boolean hasIndexScope() {
370378
/**
371379
* Returns <code>true</code> if this setting is deprecated, otherwise <code>false</code>
372380
*/
373-
public boolean isDeprecated() {
374-
return properties.contains(Property.Deprecated);
381+
private boolean isDeprecated() {
382+
return properties.contains(Property.Deprecated) || properties.contains(Property.DeprecatedWarning);
383+
}
384+
385+
private boolean isDeprecatedWarningOnly() {
386+
return properties.contains(Property.DeprecatedWarning);
375387
}
376388

377389
/**
@@ -552,13 +564,15 @@ void checkDeprecation(Settings settings) {
552564
if (this.isDeprecated() && this.exists(settings)) {
553565
// It would be convenient to show its replacement key, but replacement is often not so simple
554566
final String key = getKey();
555-
556567
List<String> skipTheseDeprecations = settings.getAsList("deprecation.skip_deprecated_settings");
557568
if (Regex.simpleMatch(skipTheseDeprecations, key) == false) {
558-
Settings.DeprecationLoggerHolder.deprecationLogger
559-
.critical(DeprecationCategory.SETTINGS, key,
560-
"[{}] setting was deprecated in Elasticsearch and will be removed in a future release! "
561-
+ "See the breaking changes documentation for the next major version.", key);
569+
String message = "[{}] setting was deprecated in Elasticsearch and will be removed in a future release! "
570+
+ "See the breaking changes documentation for the next major version.";
571+
if (this.isDeprecatedWarningOnly()) {
572+
Settings.DeprecationLoggerHolder.deprecationLogger.warn(DeprecationCategory.SETTINGS, key, message, key);
573+
} else {
574+
Settings.DeprecationLoggerHolder.deprecationLogger.critical(DeprecationCategory.SETTINGS, key, message, key);
575+
}
562576
}
563577
}
564578
}

‎server/src/test/java/org/elasticsearch/common/logging/HeaderWarningTests.java

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
package org.elasticsearch.common.logging;
99

1010
import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator;
11+
12+
import org.apache.logging.log4j.Level;
1113
import org.elasticsearch.common.settings.Settings;
1214
import org.elasticsearch.common.util.concurrent.ThreadContext;
1315
import org.elasticsearch.test.ESTestCase;
@@ -190,16 +192,16 @@ public void testFailsWhenRemovingUnknownThreadContext() throws IOException {
190192

191193
public void testWarningValueFromWarningHeader() {
192194
final String s = randomAlphaOfLength(16);
193-
final String first = HeaderWarning.formatWarning(s);
195+
final String first = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, s);
194196
assertThat(HeaderWarning.extractWarningValueFromWarningHeader(first, false), equalTo(s));
195197

196198
final String withPos = "[context][1:11] Blah blah blah";
197-
final String formatted = HeaderWarning.formatWarning(withPos);
199+
final String formatted = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, withPos);
198200
assertThat(HeaderWarning.extractWarningValueFromWarningHeader(formatted, true), equalTo("Blah blah blah"));
199201

200202
final String withNegativePos = "[context][-1:-1] Blah blah blah";
201-
assertThat(HeaderWarning.extractWarningValueFromWarningHeader(HeaderWarning.formatWarning(withNegativePos), true),
202-
equalTo("Blah blah blah"));
203+
assertThat(HeaderWarning.extractWarningValueFromWarningHeader(HeaderWarning.formatWarning(DeprecationLogger.CRITICAL,
204+
withNegativePos), true), equalTo("Blah blah blah"));
203205
}
204206

205207
public void testEscapeBackslashesAndQuotes() {
@@ -285,4 +287,22 @@ private String range(int lowerInclusive, int upperInclusive) {
285287
.toString();
286288
}
287289

290+
public void testAddWarningNonDefaultLogLevel() {
291+
final int maxWarningHeaderCount = 2;
292+
Settings settings = Settings.builder()
293+
.put("http.max_warning_header_count", maxWarningHeaderCount)
294+
.build();
295+
ThreadContext threadContext = new ThreadContext(settings);
296+
final Set<ThreadContext> threadContexts = Collections.singleton(threadContext);
297+
HeaderWarning.addWarning(threadContexts, Level.WARN, "A simple message 1");
298+
final Map<String, List<String>> responseHeaders = threadContext.getResponseHeaders();
299+
300+
assertThat(responseHeaders.size(), equalTo(1));
301+
final List<String> responses = responseHeaders.get("Warning");
302+
assertThat(responses, hasSize(1));
303+
assertThat(responses.get(0), warningValueMatcher);
304+
assertThat(responses.get(0), containsString("\"A simple message 1\""));
305+
assertThat(responses.get(0), containsString(Integer.toString(Level.WARN.intLevel())));
306+
}
307+
288308
}

0 commit comments

Comments
 (0)