-
Notifications
You must be signed in to change notification settings - Fork 26
fix(logging): Update kafka client logging to use pino logger, set log level from environment variable #46
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
Conversation
…ization with enhanced logging
…r level during initialization - Added LOG_LEVEL to the environment schema for configurable logging levels (trace, debug, info, warn, error, fatal). - Implemented setLogLevel function in logger module to update the loggers level based on the environment variable. - Updated main function to set the logger level after environment initialization, enhancing logging flexibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request enhances logging functionality and refines Kafka client configuration by dynamically setting log levels and adjusting client initialization. Key changes include:
- Introducing a dynamic log level configuration via the new LOG_LEVEL environment variable and the setLogLevel function.
- Updating the Kafka client initialization to include a custom kafkaLogger and resolving TypeScript type-checking issues.
- Bumping the package version from 1.0.2 to 1.0.3.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
src/logger.ts | Added setLogLevel function to update logger level dynamically. |
src/index.ts | Set logging level during initialization using the environment variable. |
src/env-schema.ts | Introduced LOG_LEVEL configuration in the environment schema. |
src/confluent/client-manager.ts | Updated Kafka client initialization to include kafkaLogger with explicit type cast. |
package.json | Incremented package version. |
…nction - Improved LOG_LEVEL environment variable processing to ensure case-insensitivity by using a preprocess step. - Updated setLogLevel function to accept a LogLevel type, enhancing type safety and clarity in logging level management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances application logging by adding dynamic log level configuration, integrates a custom Pino-based logger into the Kafka client, and bumps the package version.
- Introduce
LOG_LEVEL
environment variable and Zod schema support - Add
setLogLevel
function and invoke it during startup - Wire
kafkaLogger
intoKafkaJS.Kafka
initialization, with TS type adjustments - Bump version from 1.0.2 to 1.0.3
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
src/env-schema.ts | Added LOG_LEVEL to environment schema |
src/logger.ts | Introduced setLogLevel function |
src/index.ts | Set log level from env.LOG_LEVEL on startup |
src/confluent/client-manager.ts | Configured KafkaJS.Kafka to use kafkaLogger |
package.json | Updated version to 1.0.3 |
Comments suppressed due to low confidence (1)
src/confluent/client-manager.ts:122
- Add unit or integration tests to verify that the custom
kafkaLogger
is invoked correctly during Kafka client operations to ensure logging integration works as expected.
new KafkaJS.Kafka({
z.enum( | ||
Object.keys(logLevels) as [ | ||
keyof typeof logLevels, | ||
...Array<keyof typeof logLevels>, | ||
], | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using z.nativeEnum(logLevels)
instead of manually constructing z.enum(Object.keys(logLevels))
to leverage Zod's native enum support and simplify the schema.
z.enum( | |
Object.keys(logLevels) as [ | |
keyof typeof logLevels, | |
...Array<keyof typeof logLevels>, | |
], | |
), | |
z.nativeEnum(logLevels), |
Copilot uses AI. Check for mistakes.
This pull request introduces several updates to improve logging functionality, enhance Kafka client configuration, and increment the package version. The changes primarily focus on adding dynamic log level configuration, refining Kafka client initialization, and updating dependencies.
Logging Enhancements:
src/env-schema.ts
: Added a new environment variableLOG_LEVEL
to configure application logging levels dynamically. The default is set to "info."src/logger.ts
: Introduced asetLogLevel
function to update the logger's level dynamically based on the environment configuration.src/index.ts
: Integrated thesetLogLevel
function to set the logging level during application initialization using theLOG_LEVEL
environment variable. [1] [2]Kafka Client Configuration:
src/confluent/client-manager.ts
: Updated the Kafka client initialization to include a custom logger (kafkaLogger
) and addressed TypeScript type-checking issues by explicitly casting configurations.src/confluent/client-manager.ts
: AddedkafkaLogger
to the imports to support enhanced Kafka client logging.Miscellaneous:
package.json
: Incremented the package version from1.0.2
to1.0.3
.