JSON logging refactoring and X-Opaque-ID support by pgomulka · Pull Request #41354 · elastic/elasticsearch

pgomulka · 2019-04-18T15:43:17Z

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

elasticmachine · 2019-04-18T15:43:19Z

Pinging @elastic/es-core-infra

pgomulka · 2019-04-19T13:25:53Z

test/logger-usage/src/main/java/org/elasticsearch/test/loggerusage/ESLoggerUsageChecker.java

-            System.err.println(wrongLoggerUsage.getErrorLines());
-            wrongUsageFound[0] = true;
-        }, args);
+//        checkLoggerUsage(wrongLoggerUsage -> {


I still need to figure out how to fix this test

This test is still commented out. We should still make it work?

absolutely, thanks for reminding!

…csearch into scratch/logging-refactor

pgomulka · 2019-07-03T06:30:17Z

@elasticmachine run elasticsearch-ci/packaging-sample

rjernst

Looks good. A few final requests

rjernst · 2019-07-04T08:46:28Z

server/src/main/java/org/elasticsearch/common/logging/DeprecatedMessage.java

+        super(fieldMap(xOpaqueId), messagePattern, args);
+    }
+
+    public static Map<String, Object> fieldMap(String xOpaqueId) {


this can be private?

it should - fixed

rjernst · 2019-07-04T08:47:59Z

server/src/main/java/org/elasticsearch/common/logging/ESJsonLayout.java

+        private String esMessageFields;
+
+        public Builder() {
+            super();


Looks like this wasn't addressed

rjernst · 2019-07-04T08:52:46Z

server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java

    public StoredContext stashContext() {
        final ThreadContextStruct context = threadLocal.get();
-        threadLocal.set(null);
+        if(context.requestHeaders.containsKey(Task.X_OPAQUE_ID)) {


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

added comment and fixed spacing

pgomulka · 2019-07-04T09:32:32Z

@elasticmachine update branch

pgomulka · 2019-07-05T08:59:36Z

@elasticmachine run elasticsearch-ci/packaging-sample

ycombinator

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

By mistake in 7.x types field was removed from slow logs. Types are still present in that version, so this have to be present as a JSON field relates #41354 backport that was causing this #44178

…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

pgomulka added 13 commits April 10, 2019 09:40

draft Poc

dd93dd1

rename pattern converter

53beb6f

code style

2e5d533

test fixes

6b6c906

unchecked annotation

e3d4aba

preserve x-opaque-id on stashs

0597c0e

checkstyle

e912d8f

revert build changes

f742221

checkstyle nad logger usage checker

aa0c89f

revert config

d90b8c1

Merge branch 'master' into feature/improved_logging

978dfd1

almost working

1a133c8

working

5510d0d

pgomulka added WIP :Core/Infra/Logging Log management and logging utilities labels Apr 18, 2019

pgomulka self-assigned this Apr 18, 2019

pgomulka added 5 commits April 18, 2019 17:55

checkstyle

457d259

checkstyle

2a2cd9a

failing test

818892b

cleanup and javadoc

f4c7005

codestyle

02e7cfc

pgomulka commented Apr 19, 2019

View reviewed changes

pgomulka added 6 commits April 19, 2019 15:26

cleanups

c95d0c6

more tests

0f15a0a

cleanup

c6bc147

javadoc

ce189e8

remove the custom user fields

5efb60b

improts and javadocs

3bdd45d

pgomulka mentioned this pull request Apr 23, 2019

WIP origin information and logs improvements #41053

Closed

pgomulka added 2 commits July 2, 2019 18:03

javadoc and empty line

8fc02be

Merge branch 'scratch/logging-refactor' of github.com:pgomulka/elasti…

12a0064

…csearch into scratch/logging-refactor

pgomulka requested a review from rjernst July 2, 2019 21:05

rjernst approved these changes Jul 4, 2019

View reviewed changes

additional comments and cleanup

52e7d02

Merge branch 'master' into scratch/logging-refactor

1247102

ycombinator approved these changes Jul 9, 2019

View reviewed changes

pgomulka merged commit 03f4e81 into elastic:master Jul 10, 2019

pgomulka mentioned this pull request Jul 10, 2019

JSON logging refactoring and X-Opaque-ID support backport(#41354) #44178

Merged

pgomulka added v8.0.0 backport pending v7.4.0 labels Jul 12, 2019

pgomulka removed the backport pending label Jul 12, 2019

pgomulka mentioned this pull request Jul 18, 2019

Add types field to JSON slow logs in 7.x #44592

Merged

pgomulka mentioned this pull request Jul 22, 2019

Deprecation messages with the same key but different x-opaque-id are allowed (#44587) #44587

Merged

pgomulka mentioned this pull request Jul 22, 2019

Deprecation messages with the same key but different x-opaque-id are allowed backport(#44587) #44682

Merged

colings86 added the >refactoring label Aug 30, 2019

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

ChrisHegarty unassigned pgomulka Oct 29, 2024

Conversation

pgomulka commented Apr 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Apr 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgomulka commented Jul 3, 2019

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgomulka commented Jul 4, 2019

Uh oh!

pgomulka commented Jul 5, 2019

Uh oh!

ycombinator left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pgomulka commented Apr 18, 2019 •

edited

Loading

ycombinator left a comment •

edited

Loading