PS-469 - add more logs #123
Conversation
…yment changes & payment reconciliation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/api/admin/admin.service.ts
Outdated
| this.logger.info( | ||
| `updateWinnings called by ${userId} for winningsId=${winningsId}`, | ||
| ); | ||
| this.logger.debug(`updateWinnings payload: ${JSON.stringify(body)}`); |
There was a problem hiding this comment.
[❗❗ security]
Consider using a more structured logging approach instead of JSON.stringify(body), as this can potentially log sensitive information. Ensure that sensitive fields are either omitted or masked.
|
|
||
| let needsReconciliation = false; | ||
| const winningsId = body.winningsId; | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Consider using logger.info for informational messages instead of logger.log to maintain consistency and clarity in log levels.
| this.logger.log( | ||
| `updateWinnings called by ${userId} for winningsId=${winningsId}`, | ||
| ); | ||
| this.logger.log(`updateWinnings payload: ${JSON.stringify(body)}`); |
There was a problem hiding this comment.
[performance]
Switching from logger.debug to logger.log may result in excessive logging in production environments. Ensure that the log level is appropriate for the environment to avoid performance issues and log noise.
| body.paymentId, | ||
| ); | ||
|
|
||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Changing the logging level from debug to log may result in more verbose output in production environments. Ensure that this change aligns with the desired logging strategy and does not expose sensitive information or overwhelm log storage.
| let releaseDate; | ||
| if (body.paymentStatus) { | ||
| releaseDate = await this.getPaymentReleaseDateByWinningsId(winningsId); | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Consider using this.logger.debug instead of this.logger.log if the intention is to log detailed information primarily useful for debugging. log might be too verbose for production logs and could clutter the log files.
|
|
||
| // iterate payments and build transaction list | ||
| payments.forEach((payment) => { | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Changing the log level from debug to log may result in more verbose output in production environments. Ensure this change aligns with the intended logging strategy and does not expose sensitive information or overwhelm log storage.
|
|
||
| if (body.paymentStatus === PaymentStatus.OWED) { | ||
| needsReconciliation = true; | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Consider using this.logger.debug instead of this.logger.log for this message. Since the message is about internal state changes and not critical information, debug level might be more appropriate to avoid cluttering logs with non-essential information.
| } | ||
| } | ||
|
|
||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Changing this.logger.debug to this.logger.log may result in a loss of granularity in log levels. If log is not equivalent to debug, consider whether this change might affect the ability to filter logs appropriately.
| ); | ||
| }); | ||
|
|
||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
The change from this.logger.info to this.logger.log could affect the log level semantics. Ensure that log provides the same level of detail and filtering capability as info if that's the intended behavior.
| for (const transaction of transactions) { | ||
| await transaction(tx); | ||
| for (let i = 0; i < transactions.length; i++) { | ||
| this.logger.log(`Executing transaction ${i + 1}/${transactions.length}`); |
There was a problem hiding this comment.
[maintainability]
Switching from this.logger.debug to this.logger.log might impact the log level used for transaction execution details. Verify that this change aligns with the desired logging strategy and does not reduce the ability to filter logs by severity.
| } | ||
| }); | ||
|
|
||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Replacing this.logger.info with this.logger.log could alter the intended log level. Confirm that log provides the necessary level of detail and filtering as info for successful transaction execution messages.
| }); | ||
|
|
||
| if (winning?.winner_id) { | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Consider using a more specific logging level than log, such as info or debug, to maintain consistency and clarity in log severity. This helps in filtering logs based on their importance.
| `Triggering payments reconciliation for user ${winning.winner_id}`, | ||
| ); | ||
| await this.paymentsService.reconcileUserPayments(winning.winner_id); | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Consider using a more specific logging level than log, such as info or debug, to maintain consistency and clarity in log severity. This helps in filtering logs based on their importance.
| } | ||
|
|
||
| result.data = 'Successfully updated winnings'; | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Consider using this.logger.info instead of this.logger.log to maintain consistency with the logging level used throughout the codebase. This ensures that logs are appropriately categorized and filtered based on their importance.
| }); | ||
|
|
||
| if (existingFormAssociation) { | ||
| this.logger.log( |
There was a problem hiding this comment.
[maintainability]
Changing the logging level from debug to log may result in more verbose output in production environments. Ensure that this change aligns with the logging strategy and does not expose sensitive information or overwhelm log storage.
| `Found existing association id='${existingFormAssociation.id}' status='${existingFormAssociation.tax_form_status}' date_filed='${existingFormAssociation.date_filed}'`, | ||
| ); | ||
| } else { | ||
| this.logger.log('No existing tax form association found'); |
There was a problem hiding this comment.
[maintainability]
Similar to the previous comment, changing the logging level from debug to log could increase log verbosity. Verify that this change is intentional and appropriate for the environment where this code will run.
PS-469 - add more logs in admin service & payment service to track payment changes & payment reconciliation