BCDA-8301: Fix large object S3 trigger + update logging #984
Conversation
kyeah
changed the title
bcda/cclf/cclf.go
Outdated
| @@ -308,7 +286,7 @@ func (importer CclfImporter) ImportCCLFDirectory(filePath string) (success, fail | |||
| } | |||
|
|
|||
| if len(cclfMap) == 0 { | |||
| log.API.Info("Failed to find any CCLF files in directory") | |||
| importer.Logger.Info("Failed to find any CCLF files in directory") | |||
There was a problem hiding this comment.
nit on the existing log message: should we update the language here to omit the word failed, since a lack of CCLF files doesn't necessarily indicate that something has gone wrong?
Possibly something like this: No CCLF files in directory
bcda/cclf/cclf.go
Outdated
| @@ -360,14 +335,12 @@ func (importer CclfImporter) ImportCCLFDirectory(filePath string) (success, fail | |||
| defer c() | |||
| return importer.FileProcessor.CleanUpCCLF(ctx, cclfMap) | |||
| }(); err != nil { | |||
| fmt.Println(err.Error()) | |||
| log.API.Error(err) | |||
| importer.Logger.Error(err) | |||
| } | |||
|
|
|||
| if failure > 0 { | |||
| err = errors.New("one or more files failed to import correctly") | |||
There was a problem hiding this comment.
another logging suggestion/non-blocking issue: would it be helpful to include the # of files that failed to import vs a generic message?
| @@ -58,11 +59,12 @@ func cclfImportHandler(ctx context.Context, sqsEvent events.SQSEvent) (string, e | |||
| // Send the entire filepath into the CCLF Importer so we are only | |||
| // importing the one file that was sent in the trigger. | |||
| filepath := fmt.Sprintf("%s/%s", e.S3.Bucket.Name, e.S3.Object.Key) | |||
| logger.Infof("Reading %s event for file %s", e.EventName, filepath) | |||
There was a problem hiding this comment.
like the addition of logging here to get more visibility!
|
LGTM! Were there any files that were too large for ingestion that were cleaned up by BFD that we may need to retrieve to try and process again? |
Good question! The timing is such that it's unlikely there were files cleaned up before I noticed, but I can double-check since we're still waiting for A* model entity files this month (last import was July 8th looking at the prod DB). |
🎫 Ticket
https://jira.cms.gov/browse/BCDA-8301
🛠 Changes
ObjectCreated:CompleteMultipartUploadeventsℹ️ Context
Noticed that most files received yesterday were quietly skipped. After investigation, when a large file is uploaded, it triggers the multipart upload event instead of the ObjectCreated:Put event.
This blog post indicates that there are several different events that can trigger an ObjectCreated:* event: https://aws.amazon.com/blogs/aws/s3-event-notification/
In addition, the error alert was triggered due to non-JSON logs being picked up, and several "warning-only" logs being logged at the
ERRORlevel. This updates that to ensure only real errors are logged at the ERROR level.🧪 Validation