MM-58281 Allow client metrics to be floats and round timestamps (#27027)

* MM-58281 Allow client metrics to be floats and round timestamps

* MM-58281 Fix report version

* Ensure reports can contain a single timestamp

* Round timestamps in unit tests

server/channels/app/metrics.go

@@ -18,7 +18,7 @@ func (a *App) RegisterPerformanceReport(rctx request.CTX, report *model.Performa
 	for _, c := range report.Counters {
 		switch c.Metric {
 		case model.ClientLongTasks:
-			a.Metrics().IncrementClientLongTasks(commonLabels["platform"], commonLabels["agent"], float64(c.Value))
+			a.Metrics().IncrementClientLongTasks(commonLabels["platform"], commonLabels["agent"], c.Value)
 		default:
 			// we intentionally skip unknown metrics
 		}
@@ -27,21 +27,21 @@ func (a *App) RegisterPerformanceReport(rctx request.CTX, report *model.Performa
 	for _, h := range report.Histograms {
 		switch h.Metric {
 		case model.ClientTimeToFirstByte:
-			a.Metrics().ObserveClientTimeToFirstByte(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientTimeToFirstByte(commonLabels["platform"], commonLabels["agent"], h.Value)
 		case model.ClientFirstContentfulPaint:
-			a.Metrics().ObserveClientFirstContentfulPaint(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientFirstContentfulPaint(commonLabels["platform"], commonLabels["agent"], h.Value)
 		case model.ClientLargestContentfulPaint:
-			a.Metrics().ObserveClientLargestContentfulPaint(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientLargestContentfulPaint(commonLabels["platform"], commonLabels["agent"], h.Value)
 		case model.ClientInteractionToNextPaint:
-			a.Metrics().ObserveClientInteractionToNextPaint(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientInteractionToNextPaint(commonLabels["platform"], commonLabels["agent"], h.Value)
 		case model.ClientCumulativeLayoutShift:
-			a.Metrics().ObserveClientCumulativeLayoutShift(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientCumulativeLayoutShift(commonLabels["platform"], commonLabels["agent"], h.Value)
 		case model.ClientChannelSwitchDuration:
-			a.Metrics().ObserveClientChannelSwitchDuration(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientChannelSwitchDuration(commonLabels["platform"], commonLabels["agent"], h.Value)
 		case model.ClientTeamSwitchDuration:
-			a.Metrics().ObserveClientTeamSwitchDuration(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientTeamSwitchDuration(commonLabels["platform"], commonLabels["agent"], h.Value)
 		case model.ClientRHSLoadDuration:
-			a.Metrics().ObserveClientRHSLoadDuration(commonLabels["platform"], commonLabels["agent"], float64(h.Value))
+			a.Metrics().ObserveClientRHSLoadDuration(commonLabels["platform"], commonLabels["agent"], h.Value)
 		default:
 			// we intentionally skip unknown metrics
 		}

server/public/model/metrics.go

@@ -35,7 +35,7 @@ var (
 
 type MetricSample struct {
 	Metric    MetricType        `json:"metric"`
-	Value     int64             `json:"value"`
+	Value     float64           `json:"value"`
 	Timestamp int64             `json:"timestamp,omitempty"`
 	Labels    map[string]string `json:"labels,omitempty"`
 }
@@ -65,7 +65,7 @@ func (r *PerformanceReport) IsValid() error {
 		return fmt.Errorf("report version is not supported: server version: %s, report version: %s", performanceReportVersion.String(), r.Version)
 	}
 
-	if r.Start >= r.End {
+	if r.Start > r.End {
 		return fmt.Errorf("report timestamps are erroneous")
 	}
 

webapp/channels/src/utils/performance_telemetry/reporter.test.ts

@@ -52,23 +52,23 @@ describe('PerformanceReporter', () => {
         expect(sendBeacon.mock.calls[0][0]).toEqual(siteUrl + '/api/v4/client_perf');
         const report = JSON.parse(sendBeacon.mock.calls[0][1]);
         expect(report).toMatchObject({
-            start: performance.timeOrigin + testMarkA.startTime,
-            end: performance.timeOrigin + testMarkB.startTime,
+            start: Math.round(performance.timeOrigin + testMarkA.startTime),
+            end: Math.round(performance.timeOrigin + testMarkB.startTime),
             histograms: [
                 {
                     metric: 'testMeasureA',
                     value: testMarkB.startTime - testMarkA.startTime,
-                    timestamp: performance.timeOrigin + testMarkA.startTime,
+                    timestamp: Math.round(performance.timeOrigin + testMarkA.startTime),
                 },
                 {
                     metric: 'testMeasureB',
                     value: testMarkC.startTime - testMarkA.startTime,
-                    timestamp: performance.timeOrigin + testMarkA.startTime,
+                    timestamp: Math.round(performance.timeOrigin + testMarkA.startTime),
                 },
                 {
                     metric: 'testMeasureC',
                     value: testMarkC.startTime - testMarkB.startTime,
-                    timestamp: performance.timeOrigin + testMarkB.startTime,
+                    timestamp: Math.round(performance.timeOrigin + testMarkB.startTime),
                 },
             ],
         });
@@ -94,7 +94,7 @@ describe('PerformanceReporter', () => {
 
         expect(reporter.handleObservations).toHaveBeenCalled();
 
-        const timestamp = performance.timeOrigin + performance.now();
+        const timestamp = Math.round(performance.timeOrigin + performance.now());
 
         await waitForReport();
 

webapp/channels/src/utils/performance_telemetry/reporter.ts

@@ -17,13 +17,28 @@ import type {PlatformLabel, UserAgentLabel} from './platform_detection';
 import {getPlatformLabel, getUserAgentLabel} from './platform_detection';
 
 type PerformanceReportMeasure = {
+
+    /**
+     * metric is the name of a counter or histogram metric which must match a MetricType constant as defined in
+     * model/metrics.go on the server.
+     */
     metric: string;
+
+    /**
+     * value is the floating point value of the metric. It's often a millisecond duration, but it's meaning depends
+     * on which metric this is.
+     */
     value: number;
+
+    /**
+     * timestamp is an integer value representing when the metric was measured as a millisecond value. Some browsers
+     * use floating point numbers for performance timestamps, so we need to make sure to round this.
+     */
     timestamp: number;
 }
 
 type PerformanceReport = {
-    version: '1.0';
+    version: '0.1.0';
 
     labels: {
         platform: PlatformLabel;
@@ -126,7 +141,7 @@ export default class PerformanceReporter {
         this.histogramMeasures.push({
             metric: entry.name,
             value: entry.duration,
-            timestamp: performance.timeOrigin + entry.startTime,
+            timestamp: Math.round(performance.timeOrigin + entry.startTime),
         });
     }
 
@@ -151,7 +166,7 @@ export default class PerformanceReporter {
         this.histogramMeasures.push({
             metric: metric.name,
             value: metric.value,
-            timestamp: performance.timeOrigin + performance.now(),
+            timestamp: Math.round(performance.timeOrigin + performance.now()),
         });
     }
 
@@ -212,7 +227,7 @@ export default class PerformanceReporter {
         const counterMeasures = this.countersToMeasures(now, counters);
 
         return {
-            version: '1.0',
+            version: '0.1.0',
 
             labels: {
                 platform: this.platformLabel,
@@ -252,7 +267,7 @@ export default class PerformanceReporter {
             counterMeasures.push({
                 metric: name,
                 value,
-                timestamp: now,
+                timestamp: Math.round(now),
             });
         }