Clean up logging and improve comments

Author: toddkazakov

data/work/postprocess/enqueue.go

@@ -8,7 +8,6 @@ import (
 	mapset "github.com/deckarep/golang-set/v2"
 
 	"github.com/tidepool-org/platform/errors"
-	"github.com/tidepool-org/platform/log"
 	"github.com/tidepool-org/platform/metadata"
 	"github.com/tidepool-org/platform/pointer"
 	userWork "github.com/tidepool-org/platform/user/work"
@@ -17,8 +16,8 @@ import (
 
 // Enqueue creates a work item to signal a change to the data of a user to trigger the postprocessor.
 // Work is created for every change reported, rather than merged into the work already pending for the user,
-// so that reporting a change is a single insert when data is uploaded. The work pending for a user is instead
-// merged when it is processed.
+// so that reporting a change is a single insert when data is uploaded. If there are multiple pending work items
+// for the same user, they are merged during processing.
 func Enqueue(ctx context.Context, workClient work.Client, userID string, reasons ...string) error {
 	if ctx == nil {
 		return errors.New("context is missing")
@@ -42,11 +41,6 @@ func Enqueue(ctx context.Context, workClient work.Client, userID string, reasons
 		return errors.Wrap(err, "unable to create work")
 	}
 
-	log.LoggerFromContext(ctx).WithFields(log.Fields{
-		"userId":                  userID,
-		"reasons":                 reasons,
-		"processingAvailableTime": create.ProcessingAvailableTime,
-	}).Debug("created work")
 	return nil
 }
 

data/work/postprocess/processor.go

@@ -70,7 +70,7 @@ func (p *Processor) Process(ctx context.Context, wrk *work.Work, processingUpdat
 		p.absorbPending,
 		p.updateSummaries,
 		p.updateClinicSummaries,
-		p.triggerElectronicHealthRecordSync,
+		p.triggerEHRSync,
 	).Process(p.Delete)
 }
 
@@ -167,7 +167,9 @@ func (p *Processor) updateSummaries() *work.ProcessResult {
 	p.summariesUpdate, err = p.UpdateSummaries(p.Context(), *p.User().UserID)
 
 	// The changes made are recorded in the metadata before they are synced to the clinic service,
-	// so that a failure between the two retries the update
+	// so that a failure between the two retries the update. They are recorded even when the
+	// calculation fails partway (a partial update is returned alongside the error), riding along
+	// on the failing update so the retry still reports them
 	changed := p.Metadata().recordSummariesUpdate(p.summariesUpdate)
 	if err != nil {
 		return p.Failing(err)
@@ -197,11 +199,6 @@ func (p *Processor) updateClinicSummaries() *work.ProcessResult {
 			return p.Failing(errors.Wrap(err, "unable to delete patient summary"))
 		}
 	}
-	if len(workMetadata.PendingSummaryDeletes) > 0 {
-		log.LoggerFromContext(p.Context()).WithFields(log.Fields{
-			"deleted": workMetadata.PendingSummaryDeletes,
-		}).Debug("deleted clinic service summaries")
-	}
 
 	var cgm *summaryTypes.CGMSummary
 	var bgm *summaryTypes.BGMSummary
@@ -215,9 +212,6 @@ func (p *Processor) updateClinicSummaries() *work.ProcessResult {
 		if err := p.ClinicsClient.UpdatePatientSummary(p.Context(), *p.User().UserID, clinics.NewPatientSummary(cgm, bgm)); err != nil {
 			return p.Failing(errors.Wrap(err, "unable to update patient summary"))
 		}
-		log.LoggerFromContext(p.Context()).WithFields(log.Fields{
-			"updated": workMetadata.PendingSummaryUpdates,
-		}).Debug("updated clinic service summaries")
 	}
 
 	log.LoggerFromContext(p.Context()).WithFields(log.Fields{
@@ -228,9 +222,8 @@ func (p *Processor) updateClinicSummaries() *work.ProcessResult {
 	return nil
 }
 
-// triggerElectronicHealthRecordSync reports the data of the user to any electronic health record it
-// is shared with. Repeating the request reports the same data again, not twice, so retries are safe.
-func (p *Processor) triggerElectronicHealthRecordSync() *work.ProcessResult {
+// triggerEHRSync triggers report and flowsheet upload for patients with active subscriptions
+func (p *Processor) triggerEHRSync() *work.ProcessResult {
 	if !TriggersEHRSync(p.Metadata().Reasons) {
 		return nil
 	}
@@ -239,8 +232,6 @@ func (p *Processor) triggerElectronicHealthRecordSync() *work.ProcessResult {
 		return p.Failing(errors.Wrap(err, "unable to trigger EHR sync"))
 	}
 
-	log.LoggerFromContext(p.Context()).Info("triggerred EHR sync")
-
 	return nil
 }
 

work/store/structured/mongo/mongo.go

@@ -174,12 +174,13 @@ func (s *Store) Poll(ctx context.Context, poll *work.Poll) ([]*work.Work, error)
 	// Group all documents by serial id
 	pipeline = append(pipeline, bson.M{"$group": bson.M{"_id": "$serialId", "documents": bson.M{"$push": "$$ROOT"}}})
 
-	// Match any without a serial id or any serial id group that contains no member in state
-	// processing nor in state failing with retry time in future ($elemMatch binds both failing
-	// conditions to the same member). Matching the whole group rather than its head is slightly
-	// conservative: a group is also excluded when a failing member with a future retry sorts after
-	// an otherwise eligible head. With uniform priority the failing member sorts first anyway;
-	// where priorities differ, correctness wins over throughput.
+	// Match documents without a serial id, and serial id groups with no member processing or
+	// failing with a future retry time ($elemMatch binds both failing conditions to the same
+	// member). The whole group is checked, not just its head, because the sort is by priority
+	// first: a lower-priority processing or failing member can sort behind an eligible sibling,
+	// and a head-only check would dispatch that sibling — running two members of a serial group
+	// at once or retrying out of order. Blocking the group whenever such a member exists is
+	// slightly conservative, but keeps the serial guarantees unconditional.
 	pipeline = append(pipeline, bson.M{"$match": bson.M{"$or": bson.A{
 		bson.M{"_id": bson.M{"$exists": false}},
 		bson.M{"$nor": bson.A{