Skip to content

Commit

Permalink
Remove warn-date from warning headers (#37622)
Browse files Browse the repository at this point in the history 
This commit removes the warn-date from warning headers. Previously we
were stamping every warning header with when the request
occurred. However, this has a severe performance penalty when
deprecation logging is called frequently, as obtaining the current time
and formatting it properly is expensive. A previous change moved to
using the startup time as the time to stamp on every warning header, but
this was only to prove that the timestamping was expensive. Since the
warn-date is optional, we elect to remove it from the warning
header. Prior to this commit, we worked in Kibana to make the warn-date
treated as optional there so that we can follow-up in Elasticsearch and
remove the warn-date. This commit does that.
  • Loading branch information
@jasontedor
jasontedor committed Jan 22, 2019
1 parent 7053fbf commit a589e29
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 94 deletions.
4 changes: 2 additions & 2 deletions client/rest/src/main/java/org/elasticsearch/client/Response.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public HttpEntity getEntity() {
private static final Pattern WARNING_HEADER_PATTERN = Pattern.compile(
"299 " + // warn code
"Elasticsearch-\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-(?:[a-f0-9]{7}|Unknown) " + // warn agent
"\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\" " + // quoted warning value, captured
"\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\"( " + // quoted warning value, captured
// quoted RFC 1123 date format
"\"" + // opening quote
"(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // weekday
Expand All @@ -112,7 +112,7 @@ public HttpEntity getEntity() {
"\\d{4} " + // 4-digit year
"\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second)
"GMT" + // GMT
"\""); // closing quote
"\")?"); // closing quote (optional, since an older version can still send a warn-date)

/**
* Returns a list of all warning headers returned in the response.
Expand Down
104 changes: 12 additions & 92 deletions server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,8 @@
import org.elasticsearch.common.util.concurrent.ThreadContext;

import java.nio.charset.Charset;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.format.SignStyle;
import java.util.BitSet;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Locale;
Expand All @@ -45,14 +39,6 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static java.time.temporal.ChronoField.DAY_OF_MONTH;
import static java.time.temporal.ChronoField.DAY_OF_WEEK;
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
import static java.time.temporal.ChronoField.YEAR;

/**
* A logger that logs deprecation notices.
*/
Expand Down Expand Up @@ -163,73 +149,14 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje
Build.CURRENT.isSnapshot() ? "-SNAPSHOT" : "",
Build.CURRENT.shortHash());

/*
* RFC 7234 section 5.5 specifies that the warn-date is a quoted HTTP-date. HTTP-date is defined in RFC 7234 Appendix B as being from
* RFC 7231 section 7.1.1.1. RFC 7231 specifies an HTTP-date as an IMF-fixdate (or an obs-date referring to obsolete formats). The
* grammar for IMF-fixdate is specified as 'day-name "," SP date1 SP time-of-day SP GMT'. Here, day-name is
* (Mon|Tue|Wed|Thu|Fri|Sat|Sun). Then, date1 is 'day SP month SP year' where day is 2DIGIT, month is
* (Jan|Feb|Mar|Apr|May|Jun|Jul|Aug|Sep|Oct|Nov|Dec), and year is 4DIGIT. Lastly, time-of-day is 'hour ":" minute ":" second' where
* hour is 2DIGIT, minute is 2DIGIT, and second is 2DIGIT. Finally, 2DIGIT and 4DIGIT have the obvious definitions.
*/
private static final DateTimeFormatter RFC_7231_DATE_TIME;

static {
final Map<Long, String> dow = new HashMap<>();
dow.put(1L, "Mon");
dow.put(2L, "Tue");
dow.put(3L, "Wed");
dow.put(4L, "Thu");
dow.put(5L, "Fri");
dow.put(6L, "Sat");
dow.put(7L, "Sun");
final Map<Long, String> moy = new HashMap<>();
moy.put(1L, "Jan");
moy.put(2L, "Feb");
moy.put(3L, "Mar");
moy.put(4L, "Apr");
moy.put(5L, "May");
moy.put(6L, "Jun");
moy.put(7L, "Jul");
moy.put(8L, "Aug");
moy.put(9L, "Sep");
moy.put(10L, "Oct");
moy.put(11L, "Nov");
moy.put(12L, "Dec");
RFC_7231_DATE_TIME = new DateTimeFormatterBuilder()
.parseCaseInsensitive()
.parseLenient()
.optionalStart()
.appendText(DAY_OF_WEEK, dow)
.appendLiteral(", ")
.optionalEnd()
.appendValue(DAY_OF_MONTH, 2, 2, SignStyle.NOT_NEGATIVE)
.appendLiteral(' ')
.appendText(MONTH_OF_YEAR, moy)
.appendLiteral(' ')
.appendValue(YEAR, 4)
.appendLiteral(' ')
.appendValue(HOUR_OF_DAY, 2)
.appendLiteral(':')
.appendValue(MINUTE_OF_HOUR, 2)
.optionalStart()
.appendLiteral(':')
.appendValue(SECOND_OF_MINUTE, 2)
.optionalEnd()
.appendLiteral(' ')
.appendOffset("+HHMM", "GMT")
.toFormatter(Locale.getDefault(Locale.Category.FORMAT));
}

private static final String STARTUP_TIME = RFC_7231_DATE_TIME.format(ZonedDateTime.now(ZoneId.of("GMT")));

/**
* Regular expression to test if a string matches the RFC7234 specification for warning headers. This pattern assumes that the warn code
* is always 299. Further, this pattern assumes that the warn agent represents a version of Elasticsearch including the build hash.
*/
public static Pattern WARNING_HEADER_PATTERN = Pattern.compile(
public static final Pattern WARNING_HEADER_PATTERN = Pattern.compile(
"299 " + // warn code
"Elasticsearch-\\d+\\.\\d+\\.\\d+(?:-(?:alpha|beta|rc)\\d+)?(?:-SNAPSHOT)?-(?:[a-f0-9]{7}|Unknown) " + // warn agent
"\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\" " + // quoted warning value, captured
"\"((?:\t| |!|[\\x23-\\x5B]|[\\x5D-\\x7E]|[\\x80-\\xFF]|\\\\|\\\\\")*)\"( " + // quoted warning value, captured
// quoted RFC 1123 date format
"\"" + // opening quote
"(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), " + // weekday
Expand All @@ -238,36 +165,31 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje
"\\d{4} " + // 4-digit year
"\\d{2}:\\d{2}:\\d{2} " + // (two-digit hour):(two-digit minute):(two-digit second)
"GMT" + // GMT
"\""); // closing quote
"\")?"); // closing quote (optional, since an older version can still send a warn-date)

/**
* Extracts the warning value from the value of a warning header that is formatted according to RFC 7234. That is, given a string
* {@code 299 Elasticsearch-6.0.0 "warning value" "Sat, 25 Feb 2017 10:27:43 GMT"}, the return value of this method would be {@code
* warning value}.
* {@code 299 Elasticsearch-6.0.0 "warning value"}, the return value of this method would be {@code warning value}.
*
* @param s the value of a warning header formatted according to RFC 7234.
* @return the extracted warning value
*/
public static String extractWarningValueFromWarningHeader(final String s) {
/*
* We know the exact format of the warning header, so to extract the warning value we can skip forward from the front to the first
* quote, and skip backwards from the end to the penultimate quote:
*
* 299 Elasticsearch-6.0.0 "warning value" "Sat, 25, Feb 2017 10:27:43 GMT"
* ^ ^ ^
* firstQuote penultimateQuote lastQuote
* quote and we know the last quote is at the end of the string
*
* We do it this way rather than seeking forward after the first quote because there could be escaped quotes in the warning value
* but since there are none in the warning date, we can skip backwards to find the quote that closes the quoted warning value.
* 299 Elasticsearch-6.0.0 "warning value"
* ^ ^
* firstQuote lastQuote
*
* We parse this manually rather than using the capturing regular expression because the regular expression involves a lot of
* backtracking and carries a performance penalty. However, when assertions are enabled, we still use the regular expression to
* verify that we are maintaining the warning header format.
*/
final int firstQuote = s.indexOf('\"');
final int lastQuote = s.lastIndexOf('\"');
final int penultimateQuote = s.lastIndexOf('\"', lastQuote - 1);
final String warningValue = s.substring(firstQuote + 1, penultimateQuote - 2);
final int lastQuote = s.length() - 1;
final String warningValue = s.substring(firstQuote + 1, lastQuote);
assert assertWarningValue(s, warningValue);
return warningValue;
}
Expand Down Expand Up @@ -329,9 +251,7 @@ void deprecated(final Set<ThreadContext> threadContexts, final String message, f
* @return a warning value formatted according to RFC 7234
*/
public static String formatWarning(final String s) {
return WARNING_PREFIX + " "
+ "\"" + escapeAndEncode(s) + "\"" + " "
+ "\"" + STARTUP_TIME + "\"";
return WARNING_PREFIX + " " + "\"" + escapeAndEncode(s) + "\"";
}

/**
Expand Down Expand Up @@ -442,7 +362,7 @@ static String encode(final String s) {
int startIndex = i;
do {
i++;
} while (i < s.length() && !doesNotNeedEncoding.get(s.charAt(i)));
} while (i < s.length() && doesNotNeedEncoding.get(s.charAt(i)) == false);

final byte[] bytes = s.substring(startIndex, i).getBytes(UTF_8);
// noinspection ForLoopReplaceableByForEach
Expand Down

0 comments on commit a589e29

Please sign in to comment.