refactor(logging): topicLogger for headers

[tomcat323]

Problem

Log message can appear in output channels(such as Amazon Q Logs as shown), IDE Debug console, or write to pre-defined log file destinations. They come from all parts of our services and can amount to large and lengthy chunks quickly.

We don’t have a reliable way to identify log messages from various services and modules of our product. Some messages might have headers while others don’t, and the existing headers are hard-coded on a message by message basis, with no checks for consistency.

1. It’s difficult to locate and trace unidentified log messages when the size of logs grow quickly.
2. Without regulation, the hard-coded custom headers can lead to legacy issues down-the-road and inconsistencies within the codebase.

Solution

1. Make message topic identifiers a variable of the getLogger() function.
2. Define a set of topics to be used in message headers, covering all services and modules.

---

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

[justinmk3]

Suggested change

	type LogTopic = 'Unknown' | 'Test'
	type LogTopic = 'unknown' | 'test'

[justinmk3]

WinstonToolkitLogger is referenced in only a few isolated places--it's already "encapsulated". The logger interface is essentially getLogger(), and already abstracts logging for the codebase.

Our base logger is winstonToolkitLogger.ts, but it has a misleading name. At present it happens to use winston as the "backend", but that can always be changed in the future without any accruing costs / tech debt. Whereas in the meantime, adding extra indirection (in the form of 2 new classes, TopicLogger and baseLogger) is a permanent cost that may never pay off.

To summarize, the near-term steps we can take are:

• rename winstonToolkitLogger.ts to toolkitLogger.ts.
• add addTopicToMessage to it.

In the future if we need other logger implementations, those can be added at near-zero cost because callers are already using the getLogger() abstraction.

[nkomonen-amazon]

It looks like this Base class can apply to the existing loggers NullLogger, ConsoleLogger, and the WinstonLogger. Would it make sense to have them all the implement this abstract class? This will allow us to drop each of their duplicated info(), debug(),... methods

Winston logger would still have the addTopicToMessage() method as Justin suggested

[justinmk3]

sure that makes sense