feat: integrate slogger with current logging #376
Conversation
WalkthroughThe pull request introduces new configuration parameters for a logging service named Slogerr in the Changes
Sequence DiagramsequenceDiagram
participant Logger
participant SlogerrTransport
participant HttpService
participant SlogerrAPI
Logger->>SlogerrTransport: Log message
SlogerrTransport->>ConfigService: Get log configuration
SlogerrTransport->>HttpService: Prepare log payload
HttpService->>SlogerrAPI: POST log data
SlogerrAPI-->>HttpService: Response
HttpService-->>SlogerrTransport: Log transmission status
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/api/src/common/logger/slogerr.transport.ts (3)
2-2: Consider using ES import syntax for consistency.Using
require('winston-transport')is valid, but switching to an ES import (e.g.,import TransportStream from 'winston-transport';) may help maintain consistency with the other imports in your Nest.js-based code.
12-19: Ensure clarity between "level" and "severity".Both
levelandseveritymay overlap semantically. If they serve different purposes, consider adding JSDoc or comments describing their intended use. If they serve the same purpose, you might standardize them to reduce redundancy.
65-71: Use broader success checks & rename success message.Currently, the code only checks for
response.status !== 200; consider also allowing other 2xx statuses (e.g., 201, 204). Moreover, the success log message reads “Error log successfully sent”, which might be misleading if the log is not necessarily an error.- if (response.status !== 200) { - this.logger.error( - `Failed to send log to Slogerr. Status: ${response.status}, Message: ${response.statusText}`, - ); - } else { - this.logger.log('Error log successfully sent to Slogerr', response); - } + if (response.status < 200 || response.status >= 300) { + this.logger.error( + `Failed to send log to Slogerr. Status: ${response.status}, Message: ${response.statusText}`, + ); + } else { + this.logger.log('Log successfully sent to Slogerr', response); + }apps/api/src/config/logger.config.ts (1)
13-13: Consider using Nest.js dependency injection to instantiate HttpService.Creating
HttpServicedirectly here works, but you may want to consider using Nest’s standard DI patterns, such as registering it in a module and injecting it. This approach can improve testability and consistency across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/api/.env.example(1 hunks)apps/api/src/common/logger/slogerr.transport.ts(1 hunks)apps/api/src/config/logger.config.ts(3 hunks)
🔇 Additional comments (2)
apps/api/src/config/logger.config.ts (1)
Line range hint 83-98: Validate the logging sequence and concurrency.
Adding slogerrTransport to the transportsConfig is correct for integrating with Winston. Ensure that concurrency use cases (e.g., simultaneous logs) do not require extra synchronization. Also verify that the transport is triggered only if the log level threshold is met (e.g., “info” vs. “error”).
apps/api/.env.example (1)
20-24: Confirm consistent naming for Slogger variables.
The environment variables use both SLOGGER_ and SLOGERR_. Ensure this is intentional and consistent with the naming of the service (e.g., "Slogger" or "Slogerr"). If one is a typo, correct it across your codebase and environment references.
API PR Checklist
Pre-requisites
.env.examplefile with the required values as applicable.PR Details
PR details have been updated as per the given format (see below)
feat: add admin login endpoint)Additional Information
ready for reviewshould be added if the PR is ready to be reviewed)Note: Reviewer should ensure that the checklist and description have been populated and followed correctly, and the PR should be merged only after resolving all conversations and verifying that CI checks pass.
Description:
Integrate Slogger along with the current logging mechanism , any ''error' will be sent to slogger portal through an api call
Related changes:
Screenshots:
Recieved mail
Slogger portal
Query request and response:
N/A
Documentation changes:
N/A
Test suite/unit testing output:
N/A
Pending actions:
N/A
Additional notes:
N/A
Summary by CodeRabbit
New Features
Configuration