-
Notifications
You must be signed in to change notification settings - Fork 25.4k
JSON logging refactoring and X-Opaque-ID support #41354
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
Conversation
Pinging @elastic/es-core-infra |
System.err.println(wrongLoggerUsage.getErrorLines()); | ||
wrongUsageFound[0] = true; | ||
}, args); | ||
// checkLoggerUsage(wrongLoggerUsage -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still need to figure out how to fix this test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test is still commented out. We should still make it work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
absolutely, thanks for reminding!
…csearch into scratch/logging-refactor
@elasticmachine run elasticsearch-ci/packaging-sample |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. A few final requests
super(fieldMap(xOpaqueId), messagePattern, args); | ||
} | ||
|
||
public static Map<String, Object> fieldMap(String xOpaqueId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be private?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should - fixed
private String esMessageFields; | ||
|
||
public Builder() { | ||
super(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this wasn't addressed
@@ -129,7 +130,13 @@ public void close() { | |||
*/ | |||
public StoredContext stashContext() { | |||
final ThreadContextStruct context = threadLocal.get(); | |||
threadLocal.set(null); | |||
if(context.requestHeaders.containsKey(Task.X_OPAQUE_ID)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we must have this special case, then there should at least be a good explanation in comment on why it is necessary.
minor nit: there are spacing issues in the if and else lines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added comment and fixed spacing
@elasticmachine update branch |
@elasticmachine run elasticsearch-ci/packaging-sample |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've retested the current state of this PR with elastic/beats#11939 and all still looks good from a Filebeat parsing POV. Thanks!
This is a refactor to current JSON logging to make it more open for extensions and support for custom ES log messages used inDeprecationLogger IndexingSlowLog , SearchSLowLog We want to include x-opaque-id in deprecation logs. The easiest way to have this as an additional JSON field instead of part of the message is to create a custom DeprecatedMessage (extends ESLogMEssage) These messages are regular log4j messages with a text, but also carry a map of fields which can then populate the log pattern. The logic for this lives in ESJsonLayout and ESMessageFieldConverter. Similar approach can be used to refactor IndexingSlowLog and SearchSlowLog JSON logs to contain fields previously only present as escaped JSON string in a message field. closes elastic#41350
…4178) This is a refactor to current JSON logging to make it more open for extensions and support for custom ES log messages used inDeprecationLogger IndexingSlowLog , SearchSLowLog We want to include x-opaque-id in deprecation logs. The easiest way to have this as an additional JSON field instead of part of the message is to create a custom DeprecatedMessage (extends ESLogMEssage) These messages are regular log4j messages with a text, but also carry a map of fields which can then populate the log pattern. The logic for this lives in ESJsonLayout and ESMessageFieldConverter. Similar approach can be used to refactor IndexingSlowLog and SearchSlowLog JSON logs to contain fields previously only present as escaped JSON string in a message field. closes #41350 backport #41354
…allowed (#44587) Deprecation logger was filtering log entries by key, that means that if two log messages with the same key are logged from different users, then the second log messages will be filtered. This change allows to log deprecation message with the same key by different users. relates #41354
…allowed (elastic#44587) Deprecation logger was filtering log entries by key, that means that if two log messages with the same key are logged from different users, then the second log messages will be filtered. This change allows to log deprecation message with the same key by different users. relates elastic#41354
…allowed backport(#44587) #44682 Deprecation logger was filtering log entries by key, that means that if two log messages with the same key are logged from different users, then the second log messages will be filtered. This change allows to log deprecation message with the same key by different users. relates #41354 backport #44587
This is a refactor to current JSON logging to make it more open for extensions
and support for custom ES log messages used in
DeprecationLogger
IndexingSlowLog
,SearchSLowLog
We want to include
x-opaque-id
in deprecation logs. The easiest way to have this as an additional JSON field instead of part of the message is to create a customDeprecatedMessage
(extendsESLogMEssage
)These messages are regular log4j messages with a text, but also carry a map of fields which can then populate the log pattern. The logic for this lives in
ESJsonLayout
andESMessageFieldConverter
.Similar approach can be used to refactor
IndexingSlowLog
andSearchSlowLog
JSON logs to contain fields previously only present as escaped JSON string in a message field.closes #41350