Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Header warning logging refactoring #55941

Merged
merged 33 commits into from
Jun 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
4d1732e
draft
pgomulka Apr 21, 2020
c1d5b13
abstract logger and compatible logger usage
pgomulka Apr 22, 2020
50a1896
logging
pgomulka Apr 23, 2020
47aca03
unnecessary changes
pgomulka Apr 30, 2020
f6cb937
delegation instead inhertiance
pgomulka Apr 30, 2020
f7d459a
metadataindex template usage
pgomulka Apr 30, 2020
521c09e
tests restructure
pgomulka Apr 30, 2020
104a1bc
checkxtyle
pgomulka May 4, 2020
913fa42
test fixes and javadoc
pgomulka May 4, 2020
0ee82ae
Merge branch 'master' into logging-refactor
pgomulka May 4, 2020
c17e55f
fix test assert about warning
pgomulka May 4, 2020
afd8d22
unused code
pgomulka May 4, 2020
3cb8ff9
reorder methods
pgomulka May 4, 2020
7aa9cdb
clean up refactor
pgomulka May 6, 2020
d0006b6
fluent builder start
pgomulka May 7, 2020
5f55470
use fluent builder
pgomulka May 7, 2020
039a950
rename
pgomulka May 7, 2020
408cc80
do not throttle to server log
pgomulka May 7, 2020
a6327e5
fix esloggerusagechecker failures
pgomulka May 8, 2020
106f528
rename headerwarning
pgomulka May 8, 2020
bf219ba
fix tests
pgomulka May 8, 2020
6777c92
Merge branch 'master' into logging-refactor
pgomulka May 8, 2020
4f68bab
change to deprecate fluent method
pgomulka May 8, 2020
3f6c3a6
line length
pgomulka May 11, 2020
65a67bc
inline log method
pgomulka May 11, 2020
7342640
arguments fix
pgomulka May 11, 2020
a37e40b
unused import
pgomulka May 11, 2020
d48a323
fix test
pgomulka May 11, 2020
11fe609
follow up
pgomulka May 14, 2020
c77405f
Merge branch 'master' into logging-refactor
pgomulka May 14, 2020
aa22a54
Merge branch 'master' into logging-refactor
pgomulka May 28, 2020
20125dd
javadoc
pgomulka Jun 1, 2020
dffd16d
Merge branch 'master' into logging-refactor
pgomulka Jun 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did this slip in, or is it intentional ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not directly related to the refactoring, but it is useful when running this test from intellij to get that reminder. not obvious from just a failure message why the test failed

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