From 7f8ac8f11c9ea78a057468379ad599da18d01f8a Mon Sep 17 00:00:00 2001
From: Jacob Bandes-Storch <jacob@foxglove.dev>
Date: Tue, 2 Aug 2022 17:07:10 -0700
Subject: [PATCH] doctor: check statistics start/end time (#507)

**Public-Facing Changes**
Updated `mcap doctor` to check the correctness of Statistics message_start_time/message_end_time.

**Description**
Fixes #501.

Reports new errors on the file from #501:
```
Examining /Users/jacob/Downloads/whattimeisit.mcap
Statistics has message start time 1659409665222474798, but the minimum chunk start time is 0
Statistics has message end time 9900000000, but the maximum chunk end time is 1659409665222474798
```
---
 go/cli/mcap/cmd/doctor.go | 47 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/go/cli/mcap/cmd/doctor.go b/go/cli/mcap/cmd/doctor.go
index e1586952d6..c113981062 100644
--- a/go/cli/mcap/cmd/doctor.go
+++ b/go/cli/mcap/cmd/doctor.go
@@ -28,6 +28,11 @@ type mcapDoctor struct {
 
 	// Map from chunk offset to chunk index
 	chunkIndexes map[uint64]*mcap.ChunkIndex
+
+	messageCount uint64
+	minLogTime   uint64
+	maxLogTime   uint64
+	statistics   *mcap.Statistics
 }
 
 func (doctor *mcapDoctor) warn(format string, v ...any) {
@@ -172,6 +177,8 @@ func (doctor *mcapDoctor) examineChunk(chunk *mcap.Chunk) {
 				maxLogTime = message.LogTime
 			}
 
+			doctor.messageCount++
+
 		default:
 			doctor.error("Illegal record in chunk: %d", tokenType)
 		}
@@ -186,6 +193,13 @@ func (doctor *mcapDoctor) examineChunk(chunk *mcap.Chunk) {
 		doctor.error("Chunk.message_end_time %d does not match the latest message log time %d",
 			chunk.MessageEndTime, maxLogTime)
 	}
+
+	if minLogTime < doctor.minLogTime {
+		doctor.minLogTime = minLogTime
+	}
+	if maxLogTime > doctor.maxLogTime {
+		doctor.maxLogTime = maxLogTime
+	}
 }
 
 func (doctor *mcapDoctor) Examine() {
@@ -219,7 +233,7 @@ func (doctor *mcapDoctor) Examine() {
 			}
 
 			if header.Library == "" {
-				doctor.warn("Header.library field should be non-empty. Its good to include a library field for reference.")
+				doctor.warn("Header.library field should be non-empty. The library field should be set to a value that identifies the software which produced the file.")
 			}
 
 			var customProfile = strings.HasPrefix(header.Profile, "x-")
@@ -272,6 +286,16 @@ func (doctor *mcapDoctor) Examine() {
 					message.LogTime, channel.Topic, lastMessageTime)
 			}
 			lastMessageTime = message.LogTime
+
+			if message.LogTime < doctor.minLogTime {
+				doctor.minLogTime = message.LogTime
+			}
+			if message.LogTime > doctor.maxLogTime {
+				doctor.maxLogTime = message.LogTime
+			}
+
+			doctor.messageCount++
+
 		case mcap.TokenChunk:
 			chunk, err := mcap.ParseChunk(data)
 			if err != nil {
@@ -304,10 +328,14 @@ func (doctor *mcapDoctor) Examine() {
 				doctor.error("Failed to parse attachment index:", err)
 			}
 		case mcap.TokenStatistics:
-			_, err := mcap.ParseStatistics(data)
+			statistics, err := mcap.ParseStatistics(data)
 			if err != nil {
 				doctor.error("Failed to parse statistics:", err)
 			}
+			if doctor.statistics != nil {
+				doctor.error("File contains multiple Statistics records")
+			}
+			doctor.statistics = statistics
 		case mcap.TokenMetadata:
 			_, err := mcap.ParseMetadata(data)
 			if err != nil {
@@ -369,6 +397,20 @@ func (doctor *mcapDoctor) Examine() {
 			doctor.error("Chunk at offset %d has uncompressed size %d, but its chunk index has uncompressed size %d", chunkOffset, chunk.UncompressedSize, chunkIndex.UncompressedSize)
 		}
 	}
+
+	if doctor.statistics != nil {
+		if doctor.messageCount > 0 {
+			if doctor.statistics.MessageStartTime != doctor.minLogTime {
+				doctor.error("Statistics has message start time %d, but the minimum message start time is %d", doctor.statistics.MessageStartTime, doctor.minLogTime)
+			}
+			if doctor.statistics.MessageEndTime != doctor.maxLogTime {
+				doctor.error("Statistics has message end time %d, but the maximum message end time is %d", doctor.statistics.MessageEndTime, doctor.maxLogTime)
+			}
+		}
+		if doctor.statistics.MessageCount != doctor.messageCount {
+			doctor.error("Statistics has message count %d, but actual number of messages is %d", doctor.statistics.MessageCount, doctor.messageCount)
+		}
+	}
 }
 
 func newMcapDoctor(reader io.ReadSeeker) *mcapDoctor {
@@ -377,6 +419,7 @@ func newMcapDoctor(reader io.ReadSeeker) *mcapDoctor {
 		channels:     make(map[uint16]*mcap.Channel),
 		schemas:      make(map[uint16]*mcap.Schema),
 		chunkIndexes: make(map[uint64]*mcap.ChunkIndex),
+		minLogTime:   math.MaxUint64,
 	}
 }