Android storage calculation changes (#33397)

Fixes #33379. Corrects storage calculation, and uses correct events to calculate work profile storage.
2025-09-24 08:35:14 -04:00
parent daa82a3e1d
commit 8d2b7f7c8f
2 changed files with 147 additions and 6 deletions
@@ -565,6 +565,16 @@ func (svc *Service) calculateAndroidStorageMetrics(
 	var totalAvailableBytes int64
 	var hasMeasuredEvents bool

+	// Track the latest external storage detection event to avoid accumulation
+	var latestExternalStorageBytes int64
+	var latestExternalStorageTime time.Time
+
+	// Track the latest measured events to avoid accumulation
+	var latestInternalMeasuredBytes int64
+	var latestInternalMeasuredTime time.Time
+	var latestExternalMeasuredBytes int64
+	var latestExternalMeasuredTime time.Time
+
 	for _, event := range device.MemoryEvents {
 		level.Debug(svc.logger).Log(
 			"msg", "Android memory event"+logSuffix,
@@ -572,19 +582,56 @@ func (svc *Service) calculateAndroidStorageMetrics(
 			"byte_count", event.ByteCount,
 			"create_time", event.CreateTime,
 		)
+
+		eventTime, err := time.Parse(time.RFC3339, event.CreateTime)
+		if err != nil {
+			// Log parse error but continue processing
+			level.Debug(svc.logger).Log(
+				"msg", "Failed to parse event time"+logSuffix,
+				"event_type", event.EventType,
+				"create_time", event.CreateTime,
+				"error", err,
+			)
+			continue
+		}
+
 		switch event.EventType {
 		case "EXTERNAL_STORAGE_DETECTED":
-			totalStorageBytes += event.ByteCount
-		case "INTERNAL_STORAGE_MEASURED", "EXTERNAL_STORAGE_MEASURED":
-			totalAvailableBytes += event.ByteCount
-			hasMeasuredEvents = true
+			// Only use the most recent EXTERNAL_STORAGE_DETECTED event
+			if eventTime.After(latestExternalStorageTime) {
+				latestExternalStorageBytes = event.ByteCount
+				latestExternalStorageTime = eventTime
+			}
+		case "INTERNAL_STORAGE_MEASURED":
+			// Only use the most recent INTERNAL_STORAGE_MEASURED event
+			if eventTime.After(latestInternalMeasuredTime) {
+				latestInternalMeasuredBytes = event.ByteCount
+				latestInternalMeasuredTime = eventTime
+				hasMeasuredEvents = true
+			}
+		case "EXTERNAL_STORAGE_MEASURED":
+			// Only use the most recent EXTERNAL_STORAGE_MEASURED event
+			if eventTime.After(latestExternalMeasuredTime) {
+				latestExternalMeasuredBytes = event.ByteCount
+				latestExternalMeasuredTime = eventTime
+				hasMeasuredEvents = true
+			}
 		}
 	}

+	// Add the latest external storage value (if any) to the total
+	if latestExternalStorageBytes > 0 {
+		totalStorageBytes += latestExternalStorageBytes
+	}
+
+	// Calculate total available from the latest measured events
+	totalAvailableBytes = latestInternalMeasuredBytes + latestExternalMeasuredBytes
+
 	if totalStorageBytes > 0 {
 		gigsTotalDiskSpace = float64(totalStorageBytes) / (1024 * 1024 * 1024)

-		// If we only have DETECTED events (no MEASURED events), storage measurement isn't supported
+		// If we only have DETECTED events (no MEASURED events), available space measurement isn't supported
+		// We can still report total storage capacity but not how much is free/used
 		// We use -1 as sentinel value to indicate "not supported"
 		if !hasMeasuredEvents {
 			gigsDiskSpaceAvailable = -1
@@ -421,6 +421,31 @@ func TestAndroidStorageExtraction(t *testing.T) {
 		require.Equal(t, float64(-1), createdHost.Host.GigsDiskSpaceAvailable, "should set available storage to -1 when MEASURED events are missing")
 		require.Equal(t, float64(-1), createdHost.Host.PercentDiskSpaceAvailable, "should set percent available to -1 when MEASURED events are missing")
 	})
+
+	t.Run("uses only latest EXTERNAL_STORAGE_DETECTED event", func(t *testing.T) {
+		createdHost = nil // Reset
+
+		enrollmentMessage := createEnrollmentMessageWithMultipleExternalDetectedEvents(t, androidmanagement.Device{
+			Name:                createAndroidDeviceId("multiple-external-test"),
+			EnrollmentTokenData: `{"enroll_secret": "global"}`,
+		})
+
+		err := svc.ProcessPubSubPush(context.Background(), "value", enrollmentMessage)
+		require.NoError(t, err)
+
+		require.NotNil(t, createdHost)
+		require.NotNil(t, createdHost.Host)
+
+		// Should use only the latest EXTERNAL_STORAGE_DETECTED event (120GB)
+		// Total: 1GB (internal) + 120GB (latest external) = 121GB
+		require.Equal(t, 121.0, createdHost.Host.GigsTotalDiskSpace, "should use only latest EXTERNAL_STORAGE_DETECTED event")
+
+		// Available: 96GB (from EXTERNAL_STORAGE_MEASURED)
+		require.InDelta(t, 96.0, createdHost.Host.GigsDiskSpaceAvailable, 0.1, "should calculate available storage")
+
+		// Percentage: 96/121 * 100 = 79.34%
+		require.InDelta(t, 79.34, createdHost.Host.PercentDiskSpaceAvailable, 0.1, "should calculate percentage correctly")
+	})
 }

 func createEnrollmentMessage(t *testing.T, deviceInfo androidmanagement.Device) *android.PubSubMessage {
@@ -496,7 +521,76 @@ func createEnrollmentMessageWithoutMeasuredEvents(t *testing.T, deviceInfo andro
 			CreateTime: "2024-01-15T09:00:00Z",
 		},
 		// No INTERNAL_STORAGE_MEASURED or EXTERNAL_STORAGE_MEASURED events
-		// This simulates Work Profile/BYOD device behavior
+	}
+
+	data, err := json.Marshal(deviceInfo)
+	require.NoError(t, err)
+
+	encodedData := base64.StdEncoding.EncodeToString(data)
+
+	return &android.PubSubMessage{
+		Attributes: map[string]string{
+			"notificationType": string(android.PubSubEnrollment),
+		},
+		Data: encodedData,
+	}
+}
+
+func createEnrollmentMessageWithMultipleExternalDetectedEvents(t *testing.T, deviceInfo androidmanagement.Device) *android.PubSubMessage {
+	deviceInfo.HardwareInfo = &androidmanagement.HardwareInfo{
+		EnterpriseSpecificId: strings.ToUpper(uuid.New().String()),
+		Brand:                "Google",
+		Model:                "Pixel 8a",
+		SerialNumber:         "test-serial",
+		Hardware:             "test-hardware",
+	}
+	deviceInfo.SoftwareInfo = &androidmanagement.SoftwareInfo{
+		AndroidBuildNumber: "test-build",
+		AndroidVersion:     "16",
+	}
+	deviceInfo.MemoryInfo = &androidmanagement.MemoryInfo{
+		TotalRam:             int64(8 * 1024 * 1024 * 1024), // 8GB RAM in bytes
+		TotalInternalStorage: int64(1 * 1024 * 1024 * 1024), // 1GB work profile partition (simulates PROFILE_OWNER)
+	}
+
+	// Simulate with multiple work profile EXTERNAL_STORAGE_DETECTED events
+	deviceInfo.MemoryEvents = []*androidmanagement.MemoryEvent{
+		{
+			EventType:  "INTERNAL_STORAGE_MEASURED",
+			CreateTime: "2024-01-15T09:00:00Z",
+			// No byteCount for work profiles
+		},
+		{
+			EventType:  "EXTERNAL_STORAGE_MEASURED",
+			ByteCount:  int64(96 * 1024 * 1024 * 1024), // 96GB available
+			CreateTime: "2024-01-15T09:00:01Z",
+		},
+		// Multiple EXTERNAL_STORAGE_DETECTED events (simulating the bug scenario)
+		{
+			EventType:  "EXTERNAL_STORAGE_DETECTED",
+			ByteCount:  int64(110 * 1024 * 1024 * 1024), // 110GB (older event)
+			CreateTime: "2024-01-15T09:00:02Z",
+		},
+		{
+			EventType:  "EXTERNAL_STORAGE_DETECTED",
+			ByteCount:  int64(110 * 1024 * 1024 * 1024), // 110GB
+			CreateTime: "2024-01-15T09:05:00Z",
+		},
+		{
+			EventType:  "EXTERNAL_STORAGE_DETECTED",
+			ByteCount:  int64(110 * 1024 * 1024 * 1024), // 110GB
+			CreateTime: "2024-01-15T09:10:00Z",
+		},
+		{
+			EventType:  "EXTERNAL_STORAGE_DETECTED",
+			ByteCount:  int64(120 * 1024 * 1024 * 1024), // 120GB (latest, different value)
+			CreateTime: "2024-01-15T09:15:00Z",
+		},
+		{
+			EventType:  "EXTERNAL_STORAGE_DETECTED",
+			ByteCount:  int64(110 * 1024 * 1024 * 1024), // 110GB (older timestamp than above)
+			CreateTime: "2024-01-15T09:14:00Z",
+		},
 	}

 	data, err := json.Marshal(deviceInfo)