Skip to content

Commit

Permalink
Header warning logging refactoring (#55941)
Browse files Browse the repository at this point in the history
Splitting DeprecationLogger into two. HeaderWarningLogger - responsible for adding a response warning headers and ThrottlingLogger - responsible for limiting the duplicated log entries for the same key (previously deprecateAndMaybeLog).
Introducing A ThrottlingAndHeaderWarningLogger which is a base for other common logging usages where both response warning header and logging throttling was needed.

relates #55699
relates #52369
  • Loading branch information
pgomulka authored Jun 1, 2020
1 parent 911c13b commit 4d6dc51
Show file tree
Hide file tree
Showing 85 changed files with 1,077 additions and 876 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ private void assertDeprecationWarnings(List<String> warningHeaderTexts, List<Str
}

/**
* Emulates Elasticsearch's DeprecationLogger.formatWarning in simple
* Emulates Elasticsearch's HeaderWarningLogger.formatWarning in simple
* cases. We don't have that available because we're testing against 1.7.
*/
private static String formatWarningWithoutDate(String warningBody) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,9 @@ public TokenStream create(TokenStream tokenStream) {
"The [edgeNGram] token filter name was deprecated in 6.4 and cannot be used in new indices. "
+ "Please change the filter name to [edge_ngram] instead.");
} else {
deprecationLogger.deprecatedAndMaybeLog("edgeNGram_deprecation",
"The [edgeNGram] token filter name is deprecated and will be removed in a future version. "
+ "Please change the filter name to [edge_ngram] instead.");
deprecationLogger.deprecate("edgeNGram_deprecation",
"The [edgeNGram] token filter name is deprecated and will be removed in a future version. "
+ "Please change the filter name to [edge_ngram] instead.");
}
return super.create(tokenStream);
}
Expand Down Expand Up @@ -291,9 +291,9 @@ public TokenStream create(TokenStream tokenStream) {
"The [nGram] token filter name was deprecated in 6.4 and cannot be used in new indices. "
+ "Please change the filter name to [ngram] instead.");
} else {
deprecationLogger.deprecatedAndMaybeLog("nGram_deprecation",
"The [nGram] token filter name is deprecated and will be removed in a future version. "
+ "Please change the filter name to [ngram] instead.");
deprecationLogger.deprecate("nGram_deprecation",
"The [nGram] token filter name is deprecated and will be removed in a future version. "
+ "Please change the filter name to [ngram] instead.");
}
return super.create(tokenStream);
}
Expand Down Expand Up @@ -347,9 +347,9 @@ public Map<String, AnalysisProvider<TokenizerFactory>> getTokenizers() {
throw new IllegalArgumentException("The [nGram] tokenizer name was deprecated in 7.6. "
+ "Please use the tokenizer name to [ngram] for indices created in versions 8 or higher instead.");
} else if (indexSettings.getIndexVersionCreated().onOrAfter(org.elasticsearch.Version.V_7_6_0)) {
deprecationLogger.deprecatedAndMaybeLog("nGram_tokenizer_deprecation",
"The [nGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [ngram] instead.");
deprecationLogger.deprecate("nGram_tokenizer_deprecation",
"The [nGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [ngram] instead.");
}
return new NGramTokenizerFactory(indexSettings, environment, name, settings);
});
Expand All @@ -359,9 +359,9 @@ public Map<String, AnalysisProvider<TokenizerFactory>> getTokenizers() {
throw new IllegalArgumentException("The [edgeNGram] tokenizer name was deprecated in 7.6. "
+ "Please use the tokenizer name to [edge_nGram] for indices created in versions 8 or higher instead.");
} else if (indexSettings.getIndexVersionCreated().onOrAfter(org.elasticsearch.Version.V_7_6_0)) {
deprecationLogger.deprecatedAndMaybeLog("edgeNGram_tokenizer_deprecation",
"The [edgeNGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [edge_ngram] instead.");
deprecationLogger.deprecate("edgeNGram_tokenizer_deprecation",
"The [edgeNGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [edge_ngram] instead.");
}
return new EdgeNGramTokenizerFactory(indexSettings, environment, name, settings);
});
Expand Down Expand Up @@ -552,9 +552,9 @@ public List<PreConfiguredTokenizer> getPreConfiguredTokenizers() {
throw new IllegalArgumentException("The [nGram] tokenizer name was deprecated in 7.6. "
+ "Please use the tokenizer name to [ngram] for indices created in versions 8 or higher instead.");
} else if (version.onOrAfter(org.elasticsearch.Version.V_7_6_0)) {
deprecationLogger.deprecatedAndMaybeLog("nGram_tokenizer_deprecation",
"The [nGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [ngram] instead.");
deprecationLogger.deprecate("nGram_tokenizer_deprecation",
"The [nGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [ngram] instead.");
}
return new NGramTokenizer();
}));
Expand All @@ -563,9 +563,9 @@ public List<PreConfiguredTokenizer> getPreConfiguredTokenizers() {
throw new IllegalArgumentException("The [edgeNGram] tokenizer name was deprecated in 7.6. "
+ "Please use the tokenizer name to [edge_ngram] for indices created in versions 8 or higher instead.");
} else if (version.onOrAfter(org.elasticsearch.Version.V_7_6_0)) {
deprecationLogger.deprecatedAndMaybeLog("edgeNGram_tokenizer_deprecation",
"The [edgeNGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [edge_ngram] instead.");
deprecationLogger.deprecate("edgeNGram_tokenizer_deprecation",
"The [edgeNGram] tokenizer name is deprecated and will be removed in a future version. "
+ "Please change the tokenizer name to [edge_ngram] instead.");
}
if (version.onOrAfter(Version.V_7_3_0)) {
return new EdgeNGramTokenizer(NGramTokenizer.DEFAULT_MIN_NGRAM_SIZE, NGramTokenizer.DEFAULT_MAX_NGRAM_SIZE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@ public class SynonymTokenFilterFactory extends AbstractTokenFilterFactory {
this.settings = settings;

if (settings.get("ignore_case") != null) {
DEPRECATION_LOGGER.deprecatedAndMaybeLog(
"synonym_ignore_case_option",
DEPRECATION_LOGGER.deprecate("synonym_ignore_case_option",
"The ignore_case option on the synonym_graph filter is deprecated. " +
"Instead, insert a lowercase filter in the filter chain before the synonym_graph filter.");
"Instead, insert a lowercase filter in the filter chain before the synonym_graph filter.");
}

this.expand = settings.getAsBoolean("expand", true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public UserAgentProcessor create(Map<String, Processor.Factory> factories, Strin
boolean ignoreMissing = readBooleanProperty(TYPE, processorTag, config, "ignore_missing", false);
Object ecsValue = config.remove("ecs");
if (ecsValue != null) {
deprecationLogger.deprecatedAndMaybeLog("ingest_useragent_ecs_settings",
deprecationLogger.deprecate("ingest_useragent_ecs_settings",
"setting [ecs] is deprecated as ECS format is the default and only option");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ void initialValidation(ReindexRequest request) {
state);
SearchSourceBuilder searchSource = request.getSearchRequest().source();
if (searchSource != null && searchSource.sorts() != null && searchSource.sorts().isEmpty() == false) {
deprecationLogger.deprecatedAndMaybeLog("reindex_sort", SORT_DEPRECATED_MESSAGE);
deprecationLogger.deprecate("reindex_sort", SORT_DEPRECATED_MESSAGE);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public class AzureDiscoveryPlugin extends Plugin implements DiscoveryPlugin {

public AzureDiscoveryPlugin(Settings settings) {
this.settings = settings;
deprecationLogger.deprecatedAndMaybeLog("azure_discovery_plugin", "azure classic discovery plugin is deprecated.");
deprecationLogger.deprecate("azure_discovery_plugin", "azure classic discovery plugin is deprecated.");
logger.trace("starting azure classic discovery plugin...");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,13 @@ static AWSCredentials loadCredentials(Settings settings) {
return null;
} else {
if (key.length() == 0) {
deprecationLogger.deprecatedAndMaybeLog("ec2_invalid_settings",
deprecationLogger.deprecate("ec2_invalid_settings",
"Setting [{}] is set but [{}] is not, which will be unsupported in future",
SECRET_KEY_SETTING.getKey(), ACCESS_KEY_SETTING.getKey());
}
if (secret.length() == 0) {
deprecationLogger.deprecatedAndMaybeLog("ec2_invalid_settings",
"Setting [{}] is set but [{}] is not, which will be unsupported in future",
deprecationLogger.deprecate("ec2_invalid_settings",
"Setting [{}] is set but [{}] is not, which will be unsupported in future",
ACCESS_KEY_SETTING.getKey(), SECRET_KEY_SETTING.getKey());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public class EvilLoggerTests extends ESTestCase {

@Override
public void setUp() throws Exception {
assert "false".equals(System.getProperty("tests.security.manager")) : "-Dtests.security.manager=false has to be set";
super.setUp();
LogConfigurator.registerErrorListener();
}
Expand Down Expand Up @@ -126,7 +127,7 @@ public void testConcurrentDeprecationLogger() throws IOException, UserException,
}
for (int j = 0; j < iterations; j++) {
for (final Integer id : ids) {
deprecationLogger.deprecatedAndMaybeLog(Integer.toString(id), "This is a maybe logged deprecation message" + id);
deprecationLogger.deprecate(Integer.toString(id), "This is a maybe logged deprecation message" + id);
}
}

Expand All @@ -137,12 +138,12 @@ public void testConcurrentDeprecationLogger() throws IOException, UserException,
*/
final List<String> warnings = threadContext.getResponseHeaders().get("Warning");
final Set<String> actualWarningValues =
warnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true))
warnings.stream().map(s -> HeaderWarning.extractWarningValueFromWarningHeader(s, true))
.collect(Collectors.toSet());
for (int j = 0; j < 128; j++) {
assertThat(
actualWarningValues,
hasItem(DeprecationLogger.escapeAndEncode("This is a maybe logged deprecation message" + j)));
hasItem(HeaderWarning.escapeAndEncode("This is a maybe logged deprecation message" + j)));
}

try {
Expand Down Expand Up @@ -180,7 +181,7 @@ public void testConcurrentDeprecationLogger() throws IOException, UserException,
assertLogLine(
deprecationEvents.get(i),
Level.WARN,
"org.elasticsearch.common.logging.DeprecationLogger\\$2\\.run",
"org.elasticsearch.common.logging.ThrottlingLogger\\$2\\.run",
"This is a maybe logged deprecation message" + i);
}

Expand All @@ -198,17 +199,17 @@ public void testDeprecationLoggerMaybeLog() throws IOException, UserException {
final int iterations = randomIntBetween(1, 16);

for (int i = 0; i < iterations; i++) {
deprecationLogger.deprecatedAndMaybeLog("key", "This is a maybe logged deprecation message");
deprecationLogger.deprecate("key", "This is a maybe logged deprecation message");
assertWarnings("This is a maybe logged deprecation message");
}
for (int k = 0; k < 128; k++) {
for (int i = 0; i < iterations; i++) {
deprecationLogger.deprecatedAndMaybeLog("key" + k, "This is a maybe logged deprecation message" + k);
deprecationLogger.deprecate("key" + k, "This is a maybe logged deprecation message" + k);
assertWarnings("This is a maybe logged deprecation message" + k);
}
}
for (int i = 0; i < iterations; i++) {
deprecationLogger.deprecatedAndMaybeLog("key", "This is a maybe logged deprecation message");
deprecationLogger.deprecate("key", "This is a maybe logged deprecation message");
assertWarnings("This is a maybe logged deprecation message");
}

Expand All @@ -222,13 +223,13 @@ public void testDeprecationLoggerMaybeLog() throws IOException, UserException {
assertLogLine(
deprecationEvents.get(0),
Level.WARN,
"org.elasticsearch.common.logging.DeprecationLogger\\$2\\.run",
"org.elasticsearch.common.logging.ThrottlingLogger\\$2\\.run",
"This is a maybe logged deprecation message");
for (int k = 0; k < 128; k++) {
assertLogLine(
deprecationEvents.get(1 + k),
Level.WARN,
"org.elasticsearch.common.logging.DeprecationLogger\\$2\\.run",
"org.elasticsearch.common.logging.ThrottlingLogger\\$2\\.run",
"This is a maybe logged deprecation message" + k);
}
}
Expand Down Expand Up @@ -256,7 +257,7 @@ public void testDeprecatedSettings() throws IOException, UserException {
assertLogLine(
deprecationEvents.get(0),
Level.WARN,
"org.elasticsearch.common.logging.DeprecationLogger\\$2\\.run",
"org.elasticsearch.common.logging.ThrottlingLogger\\$2\\.run",
"\\[deprecated.foo\\] setting was deprecated in Elasticsearch and will be removed in a future release! " +
"See the breaking changes documentation for the next major version.");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ public void testDuplicateLogMessages() throws IOException {
try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
threadContext.putHeader(Task.X_OPAQUE_ID, "ID1");
DeprecationLogger.setThreadContext(threadContext);
deprecationLogger.deprecatedAndMaybeLog("key", "message1");
deprecationLogger.deprecatedAndMaybeLog("key", "message2");
deprecationLogger.deprecate("key", "message1");
deprecationLogger.deprecate("key", "message2");
assertWarnings("message1", "message2");

final Path path = PathUtils.get(System.getProperty("es.logs.base_path"),
Expand Down Expand Up @@ -375,8 +375,8 @@ public void testDuplicateLogMessages() throws IOException {
try (ThreadContext.StoredContext ignore = threadContext.stashContext()) {
threadContext.putHeader(Task.X_OPAQUE_ID, "ID2");
DeprecationLogger.setThreadContext(threadContext);
deprecationLogger.deprecatedAndMaybeLog("key", "message1");
deprecationLogger.deprecatedAndMaybeLog("key", "message2");
deprecationLogger.deprecate("key", "message1");
deprecationLogger.deprecate("key", "message2");
assertWarnings("message1", "message2");

final Path path = PathUtils.get(System.getProperty("es.logs.base_path"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.HeaderWarning;
import org.elasticsearch.common.logging.LoggerMessageFormat;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Settings;
Expand All @@ -42,7 +42,6 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.elasticsearch.common.logging.DeprecationLogger.WARNING_HEADER_PATTERN;
import static org.elasticsearch.http.TestDeprecationHeaderRestAction.TEST_DEPRECATED_SETTING_TRUE1;
import static org.elasticsearch.http.TestDeprecationHeaderRestAction.TEST_DEPRECATED_SETTING_TRUE2;
import static org.elasticsearch.http.TestDeprecationHeaderRestAction.TEST_NOT_DEPRECATED_SETTING;
Expand Down Expand Up @@ -184,10 +183,10 @@ private void doTestDeprecationWarningsAppearInHeaders() throws IOException {

assertThat(deprecatedWarnings, hasSize(headerMatchers.size()));
for (final String deprecatedWarning : deprecatedWarnings) {
assertThat(deprecatedWarning, matches(WARNING_HEADER_PATTERN.pattern()));
assertThat(deprecatedWarning, matches(HeaderWarning.WARNING_HEADER_PATTERN.pattern()));
}
final List<String> actualWarningValues =
deprecatedWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true))
deprecatedWarnings.stream().map(s -> HeaderWarning.extractWarningValueFromWarningHeader(s, true))
.collect(Collectors.toList());
for (Matcher<String> headerMatcher : headerMatchers) {
assertThat(actualWarningValues, hasItem(headerMatcher));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ public String getWriteableName() {

@Override
protected Query doToQuery(QueryShardContext context) throws IOException {
deprecationLogger.deprecatedAndMaybeLog(NAME, "[{}] query is deprecated, but used on [{}] index", NAME, context.index().getName());
deprecationLogger.deprecate(NAME, "[{}] query is deprecated, but used on [{}] index", NAME, context.index().getName());

return Queries.newMatchAllQuery();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public RestChannelConsumer prepareRequest(RestRequest request, NodeClient client
final Map<String, Object> source = parser.map();

if (source.containsKey("deprecated_settings")) {
deprecationLogger.deprecatedAndMaybeLog("deprecated_settings", DEPRECATED_USAGE);
deprecationLogger.deprecate("deprecated_settings", DEPRECATED_USAGE);

settings = (List<String>)source.get("deprecated_settings");
} else {
Expand Down
Loading

0 comments on commit 4d6dc51

Please sign in to comment.