Skip to content

Replace stack walking getLogger with explicit calls#84480

Merged
ChrisHegarty merged 4 commits intoelastic:masterfrom
ChrisHegarty:stack_logger
Mar 2, 2022
Merged

Replace stack walking getLogger with explicit calls#84480
ChrisHegarty merged 4 commits intoelastic:masterfrom
ChrisHegarty:stack_logger

Conversation

@ChrisHegarty
Copy link
Copy Markdown
Contributor

Replace the no-args LogManager::getLogger calls with the single-arg
variant that accepts a j.l.Class reference, which avoids the stack walk
of the no-args variant. The no-args variant determines the caller's
class by looking at the stack frame two positions from itself. The use
of the 1-args variant is more explicit and avoids the need for the stack
walk, while retaining the very same behaviour. Standardizing on the
1-args variant will reduce the need to have different ways to retrieve
logger references.

@ChrisHegarty ChrisHegarty requested a review from pgomulka March 1, 2022 09:41
@ChrisHegarty ChrisHegarty added the Team:Core/Infra Meta label for core/infra team label Mar 1, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@ChrisHegarty ChrisHegarty mentioned this pull request Mar 1, 2022
19 tasks
@DaveCTurner
Copy link
Copy Markdown
Member

👍 but we should also adjust the contribution docs which say to use the no-arg constructor: https://github.com/elastic/elasticsearch/blob/master/CONTRIBUTING.md#logging

Also consider adding the no-arg constructor to build-tools-internal/src/main/resources/forbidden/es-server-signatures.txt so that this doesn't get unconsciously and progressively reverted over time.

@pugnascotia pugnascotia added :Core/Infra/Logging Log management and logging utilities >refactoring and removed >non-issue labels Mar 1, 2022
@ChrisHegarty
Copy link
Copy Markdown
Contributor Author

Great suggestions @DaveCTurner

👍 but we should also adjust the contribution docs which say to use the no-arg constructor: https://github.com/elastic/elasticsearch/blob/master/CONTRIBUTING.md#logging

Done.

Also consider adding the no-arg constructor to build-tools-internal/src/main/resources/forbidden/es-server-signatures.txt so that this doesn't get unconsciously and progressively reverted over time.

Done.

Copy link
Copy Markdown
Contributor

@pgomulka pgomulka left a comment

Choose a reason for hiding this comment

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

LGTM

@ChrisHegarty ChrisHegarty merged commit ae8eeae into elastic:master Mar 2, 2022
@ChrisHegarty ChrisHegarty deleted the stack_logger branch March 2, 2022 09:42
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 >refactoring Team:Core/Infra Meta label for core/infra team v8.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants