Skip to content

Conversation

@pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Sep 10, 2021

Deprecation logs in 7.x are using ESJsonLayout and it requires
additional fields to be declared in a log4j config in order to emit
values to logs.
This commit adds category field to deprecation log pattern

context: in 8.0 we use ECSLayout which do not require to declare additional fields upfront. In 7.x we have to declare them in the logging config. The PR introducing categories in master #67443. A backport to 7.x which missed this field #68061

Deprecation logs in 7.x are using ESJsonLayout and it requires
additional fields to be declared in a log4j config in order to emit
values to logs.
This commit adds category field to deprecation log pattern
@pgomulka pgomulka added :Core/Infra/Logging Log management and logging utilities v7.16.0 labels Sep 10, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Sep 10, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@pgomulka pgomulka changed the base branch from master to 7.x September 10, 2021 11:44
@pgomulka pgomulka changed the title Logging/add category to deprecation logs [7.x] Add category field to deprecation logs Sep 10, 2021
@pgomulka pgomulka removed the v8.0.0 label Sep 10, 2021
@pgomulka
Copy link
Contributor Author

ok to test

@pgomulka
Copy link
Contributor Author

@elasticmachine test this please

@pgomulka
Copy link
Contributor Author

@elasticmachine update branch

@elasticmachine
Copy link
Collaborator

merge conflict between base and head

@pgomulka pgomulka requested a review from pugnascotia October 18, 2021 08:44
Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM! This looks good to me, I just had a question for my own better understanding, does the order of the fields declared in the .properties files matter? I can see that DeprecationIndexingComponent sets them in this order category,key,x-opaque-id, which is reversed from the properties files.

Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

LGTM.

@pgomulka
Copy link
Contributor Author

I just had a question for my own better understanding, does the order of the fields declared in the .properties files matter? I can see that DeprecationIndexingComponent sets them in this order category,key,x-opaque-id, which is reversed from the properties files.

@grcevski good catch! To be fair it would be better if we have them in the same order, but at the same time I don't think this would affect anyone - it is just the order of fields in indexed JSON vs order of fields in a log.
The order on other fields is also mixed.
I guess we could follow up with a separate cleanup PR for this.

@pgomulka pgomulka merged commit b7968e6 into elastic:7.x Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Logging Log management and logging utilities >deprecation Team:Core/Infra Meta label for core/infra team v7.16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants