Skip to content

MockLogAppender takes string logger names for capturing#104971

Merged
elasticsearchmachine merged 2 commits intoelastic:mainfrom
ywangd:mock-log-appender-with-string-names
Jan 31, 2024
Merged

MockLogAppender takes string logger names for capturing#104971
elasticsearchmachine merged 2 commits intoelastic:mainfrom
ywangd:mock-log-appender-with-string-names

Conversation

@ywangd
Copy link
Copy Markdown
Member

@ywangd ywangd commented Jan 31, 2024

For classes that are not publically accessible, we can use the cannoical names for capturing.

For classes that are not publically accessible, we can use the cannoical
names for capturing.
@ywangd ywangd added >non-issue :Core/Infra/Logging Log management and logging utilities v8.13.0 labels Jan 31, 2024
@ywangd ywangd requested a review from DaveCTurner January 31, 2024 12:40
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jan 31, 2024
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

blobStore.bucket(),
blobKey,
purpose,
purpose.getKey(),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Irrelevant to this PR. But it's too trivial to have its own PR.

Copy link
Copy Markdown
Member

@DaveCTurner DaveCTurner left a comment

Choose a reason for hiding this comment

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

LGTM, one nit

}

public Releasable capturing(String... names) {
final var loggers = Arrays.stream(names).map(LogManager::getLogger).toArray(Logger[]::new);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: not sure why I used toArray here in the first place, toList() would be neater

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yep I updated both places to use toList.

@ywangd ywangd added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jan 31, 2024
@elasticsearchmachine elasticsearchmachine merged commit 1fd2756 into elastic:main Jan 31, 2024
@ywangd ywangd deleted the mock-log-appender-with-string-names branch January 31, 2024 13:45
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Core/Infra/Logging Log management and logging utilities >non-issue Team:Core/Infra Meta label for core/infra team v8.13.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants