#2180 - Additional log improvement #2326
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
Backend Unit Tests Coverage Report
|
E2E Queue Consumers Coverage Report
|
E2E Workflow Workers Coverage Report
|
andrewsignori-aot
requested review from
dheepak-aot,
ann-aot,
guru-aot,
andrepestana-aot,
JasonCTang and
sh16011993
E2E SIMS API Coverage Report
|
| // In case an unexpected error happen the finally block will still be able to | ||
| // output the partial information captured by the processSummary. | ||
| const serviceProcessSummary = new ProcessSummary(); | ||
| processSummary.children(serviceProcessSummary); |
There was a problem hiding this comment.
Let me know if I am missing something. this part can be done inside process summary class constructor right?
There was a problem hiding this comment.
Sorry, I am not following the comment. Do you mean the association of a child process?
Here we have a parent log and the creating of a child log to capture the service-related logs. I am not sure what could be done inside the ProcessSummary constructor.
There was a problem hiding this comment.
I get it now. it is for the first level of indentation.
My understanding is better now.
With that in mind, just wondering if service process summary could be retuned by a getter in the class like this. So that we can avoid doing this assignment at processor level.
await this.workflowEnqueuerService.enqueueStartAssessmentWorkflows(
processSummary.getServiceProcessSummary(),
);There was a problem hiding this comment.
The idea of having it created at the processor lever and passing it as a reference is to allow access to it in case some unhandled exception happens inside the service. Having it at the processor level allows us to dump any information added to it in the final block. Does it make sense?
| @@ -19,6 +18,7 @@ export interface QueueProcessSummaryResult { | |||
| /** | |||
| * Allow the logs aggregation for a process summary and optionally | |||
| * allow the same logs to be saved to different outputs. | |||
| * @deprecated please use ProcessSummary from sims/utilities/logger. | |||
| * Allows a log entry that represents either a single log | ||
| * or a new collection of child entries. | ||
| */ | ||
| type LogInfoEntry = LogEntry | ProcessSummary[]; |
| @@ -94,6 +94,9 @@ export class ApplicationService { | |||
| ); | |||
| }), | |||
| ) | |||
| .andWhere("application.applicationStatus IN (:...status)", { | |||
andrewsignori-aot
deleted the
feature/#2180-additional-log-improvement
branch
andrewsignori-aot
temporarily deployed
to
DEV
GitHub Actions
Inactive
andrewsignori-aot
temporarily deployed
to
DEV
GitHub Actions
Inactive
andrewsignori-aot
temporarily deployed
to
DEV
GitHub Actions
Inactive
andrewsignori-aot
temporarily deployed
to
DEV
GitHub Actions
Inactive
andrewsignori-aot
temporarily deployed
to
DEV
GitHub Actions
Inactive
andrewsignori-aot
temporarily deployed
to
DEV
GitHub Actions
Inactive
andrewsignori-aot
temporarily deployed
to
DEV
GitHub Actions
Inactive
Log improvements
Created a new method to inspect the summary message to indicate if there is something that needs attention in the logs.
Scheduler Enqueuer Query Change
The query was not filtering by application status and I thought that it would be better to ensure that only
SubmittedandCompletedapplications would be considered to have a workflow queued.Refactor
Adjusted the logs for the
assessment-workflow-enqueuerandstart-application-assessmentto exclusively use the newProcessSummaryclass instead of theQueueProcessSummary. The suggestion is intended to be a POC to have theQueueProcessSummarymarked as deprecated.