Skip to content

Replace global loggers with local loggers in tlscommon package #334

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Aug 4, 2025
Merged

Replace global loggers with local loggers in tlscommon package #334

merged 11 commits into from
Aug 4, 2025

Conversation

@khushijain21
Copy link
Contributor

@khushijain21 khushijain21 commented Jul 30, 2025 •
edited
Loading

What does this PR do?

Currently, logs from tlscommon package print ca_trusted_fingerprint provided by user and also those that are available on the server. This could be a sensitive information and should not be shipped.

Why is it important?

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

Related issues

Part of https://github.com/elastic/ingest-dev/issues/5251

@khushijain21 khushijain21 requested a review from a team as a code owner July 30, 2025 10:46
@khushijain21 khushijain21 requested review from VihasMakwana and andrzej-stencel and removed request for a team July 30, 2025 10:46
@khushijain21 khushijain21 marked this pull request as draft July 30, 2025 11:57
@khushijain21 khushijain21 changed the title Add event tag for logs from tls Replace global loggers with local loggers Jul 30, 2025
@khushijain21 khushijain21 changed the title Replace global loggers with local loggers Replace global loggers with local loggers in tlscommon package Jul 31, 2025
@khushijain21 khushijain21 marked this pull request as ready for review July 31, 2025 05:00
kruskall
kruskall reviewed Aug 1, 2025
View reviewed changes
mauri870
mauri870 previously approved these changes Aug 1, 2025
View reviewed changes
Copy link
Member

@mauri870 mauri870 left a comment

Choose a reason for hiding this comment

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

LGTM. Please make sure to address Joseph's feedback first.

AndersonQ
AndersonQ reviewed Aug 1, 2025
View reviewed changes
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@khushijain21 khushijain21 self-assigned this Aug 1, 2025
@khushijain21 khushijain21 merged commit a9550f1 into elastic:main Aug 4, 2025
2 of 5 checks passed
@khushijain21 khushijain21 mentioned this pull request Aug 4, 2025
Merged
6 tasks
This was referenced Aug 5, 2025
Merged
Merged
This was referenced Aug 7, 2025
Merged
Merged
This was referenced Aug 19, 2025
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndersonQ AndersonQ AndersonQ approved these changes

@andrzej-stencel andrzej-stencel Awaiting requested review from andrzej-stencel andrzej-stencel is a code owner automatically assigned from elastic/elastic-agent-data-plane

@VihasMakwana VihasMakwana Awaiting requested review from VihasMakwana VihasMakwana is a code owner automatically assigned from elastic/elastic-agent-data-plane

@leehinman leehinman Awaiting requested review from leehinman

@mauri870 mauri870 Awaiting requested review from mauri870

@kruskall kruskall Awaiting requested review from kruskall

Assignees

@khushijain21 khushijain21

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@khushijain21 @elasticmachine @AndersonQ @mauri870 @kruskall