Skip to content

Commit e4271b6

Browse files
masseykemasseyke
authored
Changing HeaderWarning to always use 299 as the warning code (#80304) (#80503)
* Changing HeaderWarning to always use 299 as the warning code (#80304) This commit changes the deprecation logger so that all messages (critical or warning) are written out with "299" as the level at the beginning of the header in order to be compliant with https://www.rfc-editor.org/rfc/rfc7234.html#section-5.5.7. In #79107 we mistakenly began logging warning-level messages with the 300 code, which is not valid in the RFC. * fixing compilation errors from merge
1 parent c5ca20e commit e4271b6

File tree

16 files changed

+42
-72
lines changed

16 files changed

+42
-72
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import org.elasticsearch.cluster.ClusterState;
2424
import org.elasticsearch.cluster.service.ClusterService;
2525
import org.elasticsearch.common.CheckedSupplier;
26-
import org.elasticsearch.common.logging.DeprecationLogger;
2726
import org.elasticsearch.common.logging.HeaderWarning;
2827
import org.elasticsearch.common.network.InetAddresses;
2928
import org.elasticsearch.common.network.NetworkAddress;
@@ -464,7 +463,6 @@ public GeoIpProcessor create(
464463
boolean valid = metadata.isValid(currentState.metadata().settings());
465464
if (valid && metadata.isCloseToExpiration()) {
466465
HeaderWarning.addWarning(
467-
DeprecationLogger.CRITICAL,
468466
"database [{}] was not updated for over 25 days, geoip processor"
469467
+ " will stop working if there is no update for 30 days",
470468
databaseFile

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/cluster.put_settings/10_basic.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
"Test put and reset transient settings":
33
- skip:
4-
version: " - 7.15.99"
4+
version: " - 7.99.99"
55
reason: "transient settings deprecation"
66
features: "warnings"
77

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import org.elasticsearch.common.collect.ImmutableOpenMap;
3636
import org.elasticsearch.common.io.stream.StreamInput;
3737
import org.elasticsearch.common.io.stream.StreamOutput;
38-
import org.elasticsearch.common.logging.DeprecationLogger;
3938
import org.elasticsearch.common.logging.HeaderWarning;
4039
import org.elasticsearch.common.regex.Regex;
4140
import org.elasticsearch.common.settings.Setting;
@@ -1954,7 +1953,7 @@ static void validateDataStreams(SortedMap<String, IndexAbstraction> indicesLooku
19541953
// log as debug, this method is executed each time a new cluster state is created and
19551954
// could result in many logs:
19561955
logger.debug(warning);
1957-
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning);
1956+
HeaderWarning.addWarning(warning);
19581957
}
19591958
}
19601959
}

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.elasticsearch.common.bytes.BytesReference;
3232
import org.elasticsearch.common.compress.CompressedXContent;
3333
import org.elasticsearch.common.inject.Inject;
34-
import org.elasticsearch.common.logging.DeprecationLogger;
3534
import org.elasticsearch.common.logging.HeaderWarning;
3635
import org.elasticsearch.common.regex.Regex;
3736
import org.elasticsearch.common.settings.IndexScopedSettings;
@@ -560,7 +559,7 @@ public ClusterState addIndexTemplateV2(
560559
name
561560
);
562561
logger.warn(warning);
563-
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning);
562+
HeaderWarning.addWarning(warning);
564563
}
565564

566565
ComposableIndexTemplate finalIndexTemplate = template;
@@ -949,7 +948,7 @@ static ClusterState innerPutTemplate(
949948
request.name
950949
);
951950
logger.warn(warning);
952-
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning);
951+
HeaderWarning.addWarning(warning);
953952
}
954953

955954
templateBuilder.order(request.order);

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

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

99
package org.elasticsearch.common.logging;
1010

11-
import org.apache.logging.log4j.Level;
1211
import org.elasticsearch.Build;
1312
import org.elasticsearch.Version;
1413
import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -33,10 +32,10 @@
3332
public class HeaderWarning {
3433
/**
3534
* Regular expression to test if a string matches the RFC7234 specification for warning headers. This pattern assumes that the warn code
36-
* is always 299 or 300. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build
35+
* is always 299. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build
3736
* hash.
3837
*/
39-
public static final Pattern WARNING_HEADER_PATTERN = Pattern.compile("(?:299|300) " + // log level code
38+
public static final Pattern WARNING_HEADER_PATTERN = Pattern.compile("299 " + // log level code
4039
"Elasticsearch-" + // warn agent
4140
"\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-" + // warn agent
4241
"(?:[a-f0-9]{7}(?:[a-f0-9]{33})?|unknown) " + // warn agent
@@ -54,15 +53,14 @@ public class HeaderWarning {
5453

5554
/*
5655
* RFC7234 specifies the warning format as warn-code <space> warn-agent <space> "warn-text" [<space> "warn-date"]. Here, warn-code is a
57-
* three-digit number with various standard warn codes specified, and is left off of this static prefix so that it can be added based
58-
* on the log level received. The warn code will be either 299 or 300 at runtime, which are apt for our purposes as
59-
* they represent miscellaneous persistent warnings (can be presented to a human, or logged, and must not be removed by a cache).
60-
* The warn-agent is an arbitrary token; here we use the Elasticsearch version and build hash. The warn text must be quoted. The
61-
* warn-date is an optional quoted field that can be in a variety of specified date formats; here we use RFC 1123 format.
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.
6260
*/
6361
private static final String WARNING_PREFIX = String.format(
6462
Locale.ROOT,
65-
" Elasticsearch-%s%s-%s",
63+
"299 Elasticsearch-%s%s-%s",
6664
Version.CURRENT.toString(),
6765
Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "",
6866
Build.CURRENT.hash()
@@ -191,15 +189,14 @@ private static boolean assertWarningValue(final String s, final String warningVa
191189
* Format a warning string in the proper warning format by prepending a warn code, warn agent, wrapping the warning string in quotes,
192190
* and appending the RFC 7231 date.
193191
*
194-
* @param level the level of the warning - Level.WARN or DeprecationLogger.CRITICAL
195192
* @param s the warning string to format
196193
* @return a warning value formatted according to RFC 7234
197194
*/
198-
public static String formatWarning(final Level level, final String s) {
195+
public static String formatWarning(final String s) {
199196
// Assume that the common scenario won't have a string to escape and encode.
200197
int length = WARNING_PREFIX.length() + s.length() + 6;
201198
final StringBuilder sb = new StringBuilder(length);
202-
sb.append(level.intLevel() + WARNING_PREFIX).append(" \"").append(escapeAndEncode(s)).append("\"");
199+
sb.append(WARNING_PREFIX).append(" \"").append(escapeAndEncode(s)).append("\"");
203200
return sb.toString();
204201
}
205202

@@ -313,21 +310,16 @@ public static String getXOpaqueId() {
313310
.orElse("");
314311
}
315312

316-
public static void addWarning(Level level, String message, Object... params) {
317-
addWarning(THREAD_CONTEXT, level, message, params);
313+
public static void addWarning(String message, Object... params) {
314+
addWarning(THREAD_CONTEXT, message, params);
318315
}
319316

320317
// package scope for testing
321318
static void addWarning(Set<ThreadContext> threadContexts, String message, Object... params) {
322-
addWarning(threadContexts, DeprecationLogger.CRITICAL, message, params);
323-
}
324-
325-
// package scope for testing
326-
static void addWarning(Set<ThreadContext> threadContexts, Level level, String message, Object... params) {
327319
final Iterator<ThreadContext> iterator = threadContexts.iterator();
328320
if (iterator.hasNext()) {
329321
final String formattedMessage = LoggerMessageFormat.format(message, params);
330-
final String warningHeaderValue = formatWarning(level, formattedMessage);
322+
final String warningHeaderValue = formatWarning(formattedMessage);
331323
assert WARNING_HEADER_PATTERN.matcher(warningHeaderValue).matches();
332324
assert extractWarningValueFromWarningHeader(warningHeaderValue, false).equals(escapeAndEncode(formattedMessage));
333325
while (iterator.hasNext()) {

‎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(event.getLevel(), messagePattern, arguments);
38+
HeaderWarning.addWarning(messagePattern, arguments);
3939
} else {
4040
final String formattedMessage = event.getMessage().getFormattedMessage();
41-
HeaderWarning.addWarning(event.getLevel(), formattedMessage);
41+
HeaderWarning.addWarning(formattedMessage);
4242
}
4343
}
4444

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
import com.carrotsearch.randomizedtesting.generators.CodepointSetGenerator;
1111

12-
import org.apache.logging.log4j.Level;
1312
import org.elasticsearch.common.settings.Settings;
1413
import org.elasticsearch.common.util.concurrent.ThreadContext;
1514
import org.elasticsearch.test.ESTestCase;
@@ -191,19 +190,16 @@ public void testFailsWhenRemovingUnknownThreadContext() throws IOException {
191190

192191
public void testWarningValueFromWarningHeader() {
193192
final String s = randomAlphaOfLength(16);
194-
final String first = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, s);
193+
final String first = HeaderWarning.formatWarning(s);
195194
assertThat(HeaderWarning.extractWarningValueFromWarningHeader(first, false), equalTo(s));
196195

197196
final String withPos = "[context][1:11] Blah blah blah";
198-
final String formatted = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, withPos);
197+
final String formatted = HeaderWarning.formatWarning(withPos);
199198
assertThat(HeaderWarning.extractWarningValueFromWarningHeader(formatted, true), equalTo("Blah blah blah"));
200199

201200
final String withNegativePos = "[context][-1:-1] Blah blah blah";
202201
assertThat(
203-
HeaderWarning.extractWarningValueFromWarningHeader(
204-
HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, withNegativePos),
205-
true
206-
),
202+
HeaderWarning.extractWarningValueFromWarningHeader(HeaderWarning.formatWarning(withNegativePos), true),
207203
equalTo("Blah blah blah")
208204
);
209205
}
@@ -289,15 +285,15 @@ public void testAddWarningNonDefaultLogLevel() {
289285
Settings settings = Settings.builder().put("http.max_warning_header_count", maxWarningHeaderCount).build();
290286
ThreadContext threadContext = new ThreadContext(settings);
291287
final Set<ThreadContext> threadContexts = Collections.singleton(threadContext);
292-
HeaderWarning.addWarning(threadContexts, Level.WARN, "A simple message 1");
288+
HeaderWarning.addWarning(threadContexts, "A simple message 1");
293289
final Map<String, List<String>> responseHeaders = threadContext.getResponseHeaders();
294290

295291
assertThat(responseHeaders.size(), equalTo(1));
296292
final List<String> responses = responseHeaders.get("Warning");
297293
assertThat(responses, hasSize(1));
298294
assertThat(responses.get(0), warningValueMatcher);
299295
assertThat(responses.get(0), containsString("\"A simple message 1\""));
300-
assertThat(responses.get(0), containsString(Integer.toString(Level.WARN.intLevel())));
296+
assertThat(responses.get(0), containsString(Integer.toString(299)));
301297
}
302298

303299
}

‎server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
package org.elasticsearch.common.util.concurrent;
99

1010
import org.elasticsearch.common.io.stream.BytesStreamOutput;
11-
import org.elasticsearch.common.logging.DeprecationLogger;
1211
import org.elasticsearch.common.logging.HeaderWarning;
1312
import org.elasticsearch.common.settings.Settings;
1413
import org.elasticsearch.test.ESTestCase;
@@ -279,11 +278,11 @@ public void testResponseHeaders() {
279278
threadContext.addResponseHeader("foo", "bar");
280279
}
281280

282-
final String value = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "qux");
281+
final String value = HeaderWarning.formatWarning("qux");
283282
threadContext.addResponseHeader("baz", value, s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false));
284283
// pretend that another thread created the same response at a different time
285284
if (randomBoolean()) {
286-
final String duplicateValue = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "qux");
285+
final String duplicateValue = HeaderWarning.formatWarning("qux");
287286
threadContext.addResponseHeader("baz", duplicateValue, s -> HeaderWarning.extractWarningValueFromWarningHeader(s, false));
288287
}
289288

‎test/external-modules/error-query/src/main/java/org/elasticsearch/test/errorquery/ErrorQueryBuilder.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
import org.apache.lucene.search.Query;
1313
import org.elasticsearch.common.io.stream.StreamInput;
1414
import org.elasticsearch.common.io.stream.StreamOutput;
15-
import org.elasticsearch.common.logging.DeprecationLogger;
1615
import org.elasticsearch.common.logging.HeaderWarning;
1716
import org.elasticsearch.index.query.AbstractQueryBuilder;
1817
import org.elasticsearch.index.query.SearchExecutionContext;
@@ -82,7 +81,7 @@ protected Query doToQuery(SearchExecutionContext context) throws IOException {
8281
}
8382
final String header = "[" + context.index().getName() + "][" + context.getShardId() + "]";
8483
if (error.getErrorType() == IndexError.ERROR_TYPE.WARNING) {
85-
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, header + " " + error.getMessage());
84+
HeaderWarning.addWarning(header + " " + error.getMessage());
8685
return new MatchAllDocsQuery();
8786
} else {
8887
throw new RuntimeException(header + " " + error.getMessage());

‎test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1740,7 +1740,7 @@ protected static InetAddress randomIp(boolean v4) {
17401740
}
17411741

17421742
public static final class DeprecationWarning {
1743-
private final Level level;
1743+
private final Level level; // Intentionally ignoring level for the sake of equality for now
17441744
private final String message;
17451745

17461746
public DeprecationWarning(Level level, String message) {
@@ -1750,20 +1750,20 @@ public DeprecationWarning(Level level, String message) {
17501750

17511751
@Override
17521752
public int hashCode() {
1753-
return Objects.hash(level, message);
1753+
return Objects.hash(message);
17541754
}
17551755

17561756
@Override
17571757
public boolean equals(Object o) {
17581758
if (this == o) return true;
17591759
if (o == null || getClass() != o.getClass()) return false;
17601760
DeprecationWarning that = (DeprecationWarning) o;
1761-
return Objects.equals(level, that.level) && Objects.equals(message, that.message);
1761+
return Objects.equals(message, that.message);
17621762
}
17631763

17641764
@Override
17651765
public String toString() {
1766-
return String.format(Locale.ROOT, "%s (%s): %s", level.name(), level.intLevel(), message);
1766+
return String.format(Locale.ROOT, "%s: %s", level.name(), message);
17671767
}
17681768
}
17691769
}

‎test/framework/src/test/java/org/elasticsearch/test/rest/yaml/section/DoSectionTests.java

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
import org.elasticsearch.client.Node;
1414
import org.elasticsearch.client.NodeSelector;
1515
import org.elasticsearch.common.ParsingException;
16-
import org.elasticsearch.common.logging.DeprecationLogger;
1716
import org.elasticsearch.common.logging.HeaderWarning;
1817
import org.elasticsearch.test.rest.yaml.ClientYamlTestExecutionContext;
1918
import org.elasticsearch.test.rest.yaml.ClientYamlTestResponse;
@@ -52,10 +51,10 @@ public void testWarningHeaders() {
5251
section.checkWarningHeaders(emptyList(), Version.CURRENT);
5352
}
5453

55-
final String testHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "test");
56-
final String anotherHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "another \"with quotes and \\ backslashes\"");
57-
final String someMoreHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "some more");
58-
final String catHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "cat");
54+
final String testHeader = HeaderWarning.formatWarning("test");
55+
final String anotherHeader = HeaderWarning.formatWarning("another \"with quotes and \\ backslashes\"");
56+
final String someMoreHeader = HeaderWarning.formatWarning("some more");
57+
final String catHeader = HeaderWarning.formatWarning("cat");
5958
// Any warning headers fail
6059
{
6160
final DoSection section = new DoSection(new XContentLocation(1, 1));
@@ -141,17 +140,13 @@ public void testWarningHeaders() {
141140

142141
public void testWarningHeadersRegex() {
143142

144-
final String testHeader = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "test");
143+
final String testHeader = HeaderWarning.formatWarning("test");
145144
final String realisticTestHeader = HeaderWarning.formatWarning(
146-
DeprecationLogger.CRITICAL,
147145
"index template [my-it] has index "
148146
+ "patterns [test-*] matching patterns from existing older templates [global] with patterns (global => [*]); this template "
149147
+ "[my-it] will take precedence during new index creation"
150148
);
151-
final String testHeaderWithQuotesAndBackslashes = HeaderWarning.formatWarning(
152-
DeprecationLogger.CRITICAL,
153-
"test \"with quotes and \\ backslashes\""
154-
);
149+
final String testHeaderWithQuotesAndBackslashes = HeaderWarning.formatWarning("test \"with quotes and \\ backslashes\"");
155150

156151
// require header and it matches (basic example)
157152
DoSection section = new DoSection(new XContentLocation(1, 1));
@@ -224,9 +219,8 @@ public void testWarningHeadersRegex() {
224219
}
225220

226221
public void testIgnoreTypesWarnings() {
227-
String legitimateWarning = HeaderWarning.formatWarning(DeprecationLogger.CRITICAL, "warning");
222+
String legitimateWarning = HeaderWarning.formatWarning("warning");
228223
String typesWarning = HeaderWarning.formatWarning(
229-
DeprecationLogger.CRITICAL,
230224
"[types removal] " + "The endpoint /{index}/{type}/_count is deprecated, use /{index}/_count instead."
231225
);
232226

‎x-pack/plugin/core/src/main/java/org/elasticsearch/license/XPackLicenseState.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
package org.elasticsearch.license;
88

99
import org.elasticsearch.common.Strings;
10-
import org.elasticsearch.common.logging.DeprecationLogger;
1110
import org.elasticsearch.common.logging.HeaderWarning;
1211
import org.elasticsearch.common.logging.LoggerMessageFormat;
1312
import org.elasticsearch.common.settings.Settings;
@@ -443,7 +442,7 @@ boolean isAllowed(LicensedFeature feature) {
443442
void checkExpiry() {
444443
String warning = status.expiryWarning;
445444
if (warning != null) {
446-
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning);
445+
HeaderWarning.addWarning(warning);
447446
}
448447
}
449448

‎x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/TransportPutTrainedModelAliasAction.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
import org.elasticsearch.cluster.metadata.Metadata;
2222
import org.elasticsearch.cluster.service.ClusterService;
2323
import org.elasticsearch.common.inject.Inject;
24-
import org.elasticsearch.common.logging.DeprecationLogger;
2524
import org.elasticsearch.common.logging.HeaderWarning;
2625
import org.elasticsearch.common.util.set.Sets;
2726
import org.elasticsearch.license.LicenseUtils;
@@ -163,7 +162,7 @@ protected void masterOperation(
163162
String warning = Messages.getMessage(TRAINED_MODEL_INPUTS_DIFFER_SIGNIFICANTLY, request.getModelId(), oldModelId);
164163
auditor.warning(oldModelId, warning);
165164
logger.warn("[{}] {}", oldModelId, warning);
166-
HeaderWarning.addWarning(DeprecationLogger.CRITICAL, warning);
165+
HeaderWarning.addWarning(warning);
167166
}
168167
}
169168
clusterService.submitStateUpdateTask("update-model-alias", new AckedClusterStateUpdateTask(request, listener) {

0 commit comments

Comments
 (0)