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 13 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
20 changes: 20 additions & 0 deletions distribution/src/config/log4j2.properties
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,26 @@ logger.deprecation.level = warn
logger.deprecation.appenderRef.deprecation_rolling.ref = deprecation_rolling
logger.deprecation.additivity = false

######## Compatible JSON #######################
appender.compatible_rolling.type = RollingFile
appender.compatible_rolling.name = compatible_rolling
appender.compatible_rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_compatible.json
appender.compatible_rolling.layout.type = ECSJsonLayout
appender.compatible_rolling.layout.type_name = compatible

appender.compatible_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_compatible-%i.json.gz
appender.compatible_rolling.policies.type = Policies
appender.compatible_rolling.policies.size.type = SizeBasedTriggeringPolicy
appender.compatible_rolling.policies.size.size = 1GB
appender.compatible_rolling.strategy.type = DefaultRolloverStrategy
appender.compatible_rolling.strategy.max = 4
#################################################

logger.compatible.name = org.elasticsearch.compatible
logger.compatible.level = warn
logger.compatible.appenderRef.deprecation_rolling.ref = compatible_rolling
logger.compatible.additivity = false

######## Search slowlog JSON ####################
appender.index_search_slowlog_rolling.type = RollingFile
appender.index_search_slowlog_rolling.name = index_search_slowlog_rolling
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 @@ -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 -> HeaderWarningLogger.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(HeaderWarningLogger.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 Down Expand Up @@ -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 @@ -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.HeaderWarningLogger;
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(HeaderWarningLogger.WARNING_HEADER_PATTERN.pattern()));
}
final List<String> actualWarningValues =
deprecatedWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true))
deprecatedWarnings.stream().map(s -> HeaderWarningLogger.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 @@ -40,7 +40,7 @@
import org.elasticsearch.common.ValidationException;
import org.elasticsearch.common.compress.CompressedXContent;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.ThrottlingAndHeaderWarningLogger;
import org.elasticsearch.common.regex.Regex;
import org.elasticsearch.common.settings.IndexScopedSettings;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -78,8 +78,8 @@
public class MetadataIndexTemplateService {

private static final Logger logger = LogManager.getLogger(MetadataIndexTemplateService.class);
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);

// private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
private static final ThrottlingAndHeaderWarningLogger deprecationLogger = new ThrottlingAndHeaderWarningLogger(logger.getName());
private final ClusterService clusterService;
private final AliasValidator aliasValidator;
private final IndicesService indicesService;
Expand Down Expand Up @@ -367,7 +367,7 @@ public ClusterState addIndexTemplateV2(final ClusterState currentState, final bo
.collect(Collectors.joining(",")),
name);
logger.warn(warning);
deprecationLogger.deprecatedAndMaybeLog("index_template_pattern_overlap", warning);
deprecationLogger.headerWarnAndThrottleLog("index_template_pattern_overlap", warning);
}

IndexTemplateV2 finalIndexTemplate = template;
Expand Down Expand Up @@ -601,7 +601,7 @@ static ClusterState innerPutTemplate(final ClusterState currentState, PutRequest
.collect(Collectors.joining(",")),
request.name);
logger.warn(warning);
deprecationLogger.deprecatedAndMaybeLog("index_template_pattern_overlap", warning);
deprecationLogger.headerWarnAndThrottleLog("index_template_pattern_overlap", warning);
} else {
// Otherwise, this is a hard error, the user should use V2 index templates instead
String error = String.format(Locale.ROOT, "template [%s] has index patterns %s matching patterns" +
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.elasticsearch.common.logging;

import org.apache.logging.log4j.Logger;

/**
* A logger that logs compatible API notices.
*/
public class CompatibleApiLogger extends ThrottlingAndHeaderWarningLogger{

public CompatibleApiLogger(Logger parentLogger) {
super(compatibleApiLogger(parentLogger));
}

private static String compatibleApiLogger(Logger parentLogger) {
String name = parentLogger.getName();
if (name.startsWith("org.elasticsearch")) {
name = name.replace("org.elasticsearch.", "org.elasticsearch.compatible.");
} else {
name = "compatible." + name;
}
return name;
}
}
Loading