Skip to content

TDB-19271 : implementing a common security logger #1374

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.

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nishchay-naresh-IBM
Copy link

No description provided.

@nishchay-naresh-IBM nishchay-naresh-IBM marked this pull request as draft July 18, 2025 16:30
@myronkscott myronkscott self-requested a review July 24, 2025 17:40
Copy link
Member

@myronkscott myronkscott left a comment

Choose a reason for hiding this comment

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

tests are failing in TCLogbackLogging because the tests assumes only one file in the logging directory

consoleAppender.start();

ch.qos.logback.classic.Logger SECURITY_LOGGER = loggerContext.getLogger("SECURITY_LOGGER");
SECURITY_LOGGER.setLevel(Level.DEBUG);
Copy link
Member

Choose a reason for hiding this comment

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

you can probably use normal variable conventions here.

Copy link
Author

Choose a reason for hiding this comment

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

accommodated

root.addAppender(continuingAppender);
disableBufferingAppender(continuingAppender);
installSecurityLogger(logDirFile);
Copy link
Member

Choose a reason for hiding this comment

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

should calling this method be conditional on having these logs here?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, calling this method will be conditional.
serverLogPath will be the default option if user is not passing security log path in propFile


private void logWithMDC(String message, Consumer<String> consumer) {
try {
MDC.put("SECURITY_LOGGER_NAME", name);
Copy link
Member

Choose a reason for hiding this comment

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

This looks like an unconventional usage of MDC. Normally this would be some context information that changes with each log like username or IP for the particular connection or some other identifying information that changes at runtime. Coding the name of the logger that is available at compile time this way seems like overkill. Is there a more conventional way to get the logger name in the log?

Copy link
Author

@nishchay-naresh-IBM nishchay-naresh-IBM Aug 4, 2025

Choose a reason for hiding this comment

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

Refactred this MDC based substitution in log with String concatenationof name with logging message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants