internal/upload: deal with files that are too new or too old

Be more careful about dates, and add tests for various
boundary conditions. In particular, the current time is
evaluated only once.

Change-Id: I23f4919d37d9a77784be2afcf9ba348ed6a223f9
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/524877
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Peter Weinberger <[email protected]>

Author: pjweinbgo

internal/counter/file.go

@@ -355,6 +355,7 @@ func Open() func() {
 	mainCounter.Add(1)
 	defaultFile.rotate()
 	return func() {
+		// Once this has been called, the defaultFile is no longer usable.
 		mf := defaultFile.current.Load()
 		if mf == nil {
 			// telemetry might have been off

internal/upload/date.go

@@ -15,8 +15,13 @@ import (
 
 // time and date handling
 
-// another name for time.Now() to use for testing
-var now = time.Now
+// all the upload processing takes place (conceptually) at
+// a single instant. Most of the time this wouldn't matter
+// but it protects against time skew if time.Now
+// increases the day between calls, as might happen (rarely) by chance
+// or if there are long scheduling delays between calls.
+var thisInstant = time.Now().UTC()
+
 var distantPast = 21 * 24 * time.Hour
 
 // reports that are too old (21 days) are not uploaded
@@ -26,7 +31,8 @@ func tooOld(date string) bool {
 		logger.Printf("tooOld: %v", err)
 		return false
 	}
-	return now().Sub(t) > distantPast
+	age := thisInstant.Sub(t)
+	return age > distantPast
 }
 
 // return the expiry date of a countfile in YYYY-MM-DD format
@@ -57,14 +63,13 @@ func expiry(fname string) time.Time {
 		logger.Printf("time.Parse: %v for %s", err, fname)
 		return farFuture // don't process it, whatever it is
 	}
-	// TODO(pjw): check for off-by-one-day?
 	return expiry
 }
 
 // stillOpen returns true if the counter file might still be active
 func stillOpen(fname string) bool {
 	expiry := expiry(fname)
-	return expiry.After(now()) // TODO(pjw): off by one day?
+	return expiry.After(thisInstant)
 }
 
 // avoid parsing count files multiple times