From 8c5f00da86a689fc5b6634cd0527c3f2c606036c Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 5 Sep 2024 16:30:24 -0400 Subject: [PATCH] MM-60285 Add fresh label to channel and team switch metrics (#28100) * Changed measureAndReport to take an object as parameters * MM-60285 Add fresh label to channel and team switch metrics --- server/channels/app/metrics.go | 14 +++++++++++-- server/einterfaces/metrics.go | 4 ++-- server/einterfaces/mocks/MetricsInterface.go | 12 +++++------ server/enterprise/metrics/metrics.go | 12 +++++------ server/public/model/metrics.go | 1 + webapp/channels/src/actions/views/rhs.ts | 6 +++++- .../post_view/post_list/post_list.tsx | 20 +++++++++++++++++-- .../global_threads/global_threads.tsx | 6 +++++- .../src/utils/performance_telemetry/index.ts | 19 +++++++++++++++--- .../performance_telemetry/reporter.test.ts | 18 ++++++++++++++--- .../utils/performance_telemetry/reporter.ts | 1 + 11 files changed, 87 insertions(+), 26 deletions(-) diff --git a/server/channels/app/metrics.go b/server/channels/app/metrics.go index 4b47757487a2..9f7d961fdbb4 100644 --- a/server/channels/app/metrics.go +++ b/server/channels/app/metrics.go @@ -49,9 +49,19 @@ func (a *App) RegisterPerformanceReport(rctx request.CTX, report *model.Performa case model.ClientPageLoadDuration: a.Metrics().ObserveClientPageLoadDuration(commonLabels["platform"], commonLabels["agent"], h.Value/1000) case model.ClientChannelSwitchDuration: - a.Metrics().ObserveClientChannelSwitchDuration(commonLabels["platform"], commonLabels["agent"], h.Value/1000) + a.Metrics().ObserveClientChannelSwitchDuration( + commonLabels["platform"], + commonLabels["agent"], + h.GetLabelValue("fresh", model.AcceptedTrueFalseLabels, ""), + h.Value/1000, + ) case model.ClientTeamSwitchDuration: - a.Metrics().ObserveClientTeamSwitchDuration(commonLabels["platform"], commonLabels["agent"], h.Value/1000) + a.Metrics().ObserveClientTeamSwitchDuration( + commonLabels["platform"], + commonLabels["agent"], + h.GetLabelValue("fresh", model.AcceptedTrueFalseLabels, ""), + h.Value/1000, + ) case model.ClientRHSLoadDuration: a.Metrics().ObserveClientRHSLoadDuration(commonLabels["platform"], commonLabels["agent"], h.Value/1000) case model.ClientGlobalThreadsLoadDuration: diff --git a/server/einterfaces/metrics.go b/server/einterfaces/metrics.go index 88e414a87e83..d9e5bff26875 100644 --- a/server/einterfaces/metrics.go +++ b/server/einterfaces/metrics.go @@ -111,8 +111,8 @@ type MetricsInterface interface { ObserveClientCumulativeLayoutShift(platform, agent string, elapsed float64) IncrementClientLongTasks(platform, agent string, inc float64) ObserveClientPageLoadDuration(platform, agent string, elapsed float64) - ObserveClientChannelSwitchDuration(platform, agent string, elapsed float64) - ObserveClientTeamSwitchDuration(platform, agent string, elapsed float64) + ObserveClientChannelSwitchDuration(platform, agent, fresh string, elapsed float64) + ObserveClientTeamSwitchDuration(platform, agent, fresh string, elapsed float64) ObserveClientRHSLoadDuration(platform, agent string, elapsed float64) ObserveGlobalThreadsLoadDuration(platform, agent string, elapsed float64) ObserveMobileClientLoadDuration(platform string, elapsed float64) diff --git a/server/einterfaces/mocks/MetricsInterface.go b/server/einterfaces/mocks/MetricsInterface.go index 188ee7f96b8e..e2163767dfd3 100644 --- a/server/einterfaces/mocks/MetricsInterface.go +++ b/server/einterfaces/mocks/MetricsInterface.go @@ -298,9 +298,9 @@ func (_m *MetricsInterface) ObserveAPIEndpointDuration(endpoint string, method s _m.Called(endpoint, method, statusCode, originClient, pageLoadContext, elapsed) } -// ObserveClientChannelSwitchDuration provides a mock function with given fields: platform, agent, elapsed -func (_m *MetricsInterface) ObserveClientChannelSwitchDuration(platform string, agent string, elapsed float64) { - _m.Called(platform, agent, elapsed) +// ObserveClientChannelSwitchDuration provides a mock function with given fields: platform, agent, fresh, elapsed +func (_m *MetricsInterface) ObserveClientChannelSwitchDuration(platform string, agent string, fresh string, elapsed float64) { + _m.Called(platform, agent, fresh, elapsed) } // ObserveClientCumulativeLayoutShift provides a mock function with given fields: platform, agent, elapsed @@ -333,9 +333,9 @@ func (_m *MetricsInterface) ObserveClientRHSLoadDuration(platform string, agent _m.Called(platform, agent, elapsed) } -// ObserveClientTeamSwitchDuration provides a mock function with given fields: platform, agent, elapsed -func (_m *MetricsInterface) ObserveClientTeamSwitchDuration(platform string, agent string, elapsed float64) { - _m.Called(platform, agent, elapsed) +// ObserveClientTeamSwitchDuration provides a mock function with given fields: platform, agent, fresh, elapsed +func (_m *MetricsInterface) ObserveClientTeamSwitchDuration(platform string, agent string, fresh string, elapsed float64) { + _m.Called(platform, agent, fresh, elapsed) } // ObserveClientTimeToFirstByte provides a mock function with given fields: platform, agent, elapsed diff --git a/server/enterprise/metrics/metrics.go b/server/enterprise/metrics/metrics.go index d775d6939be5..aac0046999ec 100644 --- a/server/enterprise/metrics/metrics.go +++ b/server/enterprise/metrics/metrics.go @@ -1262,7 +1262,7 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf Name: "channel_switch", Help: "Duration of the time taken from when a user clicks on a channel in the LHS to when posts in that channel become visible (seconds)", }, - []string{"platform", "agent"}, + []string{"platform", "agent", "fresh"}, ) m.Registry.MustRegister(m.ClientChannelSwitchDuration) @@ -1273,7 +1273,7 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf Name: "team_switch", Help: "Duration of the time taken from when a user clicks on a team in the LHS to when posts in that team become visible (seconds)", }, - []string{"platform", "agent"}, + []string{"platform", "agent", "fresh"}, ) m.Registry.MustRegister(m.ClientTeamSwitchDuration) @@ -1822,12 +1822,12 @@ func (mi *MetricsInterfaceImpl) ObserveClientPageLoadDuration(platform, agent st mi.ClientPageLoadDuration.With(prometheus.Labels{"platform": platform, "agent": agent}).Observe(elapsed) } -func (mi *MetricsInterfaceImpl) ObserveClientChannelSwitchDuration(platform, agent string, elapsed float64) { - mi.ClientChannelSwitchDuration.With(prometheus.Labels{"platform": platform, "agent": agent}).Observe(elapsed) +func (mi *MetricsInterfaceImpl) ObserveClientChannelSwitchDuration(platform, agent, fresh string, elapsed float64) { + mi.ClientChannelSwitchDuration.With(prometheus.Labels{"platform": platform, "agent": agent, "fresh": fresh}).Observe(elapsed) } -func (mi *MetricsInterfaceImpl) ObserveClientTeamSwitchDuration(platform, agent string, elapsed float64) { - mi.ClientTeamSwitchDuration.With(prometheus.Labels{"platform": platform, "agent": agent}).Observe(elapsed) +func (mi *MetricsInterfaceImpl) ObserveClientTeamSwitchDuration(platform, agent, fresh string, elapsed float64) { + mi.ClientTeamSwitchDuration.With(prometheus.Labels{"platform": platform, "agent": agent, "fresh": fresh}).Observe(elapsed) } func (mi *MetricsInterfaceImpl) ObserveClientRHSLoadDuration(platform, agent string, elapsed float64) { diff --git a/server/public/model/metrics.go b/server/public/model/metrics.go index 3daae496c39b..d3691b5c4872 100644 --- a/server/public/model/metrics.go +++ b/server/public/model/metrics.go @@ -51,6 +51,7 @@ var ( "modal_content", "other", ) + AcceptedTrueFalseLabels = sliceToMapKey("true", "false") ) type MetricSample struct { diff --git a/webapp/channels/src/actions/views/rhs.ts b/webapp/channels/src/actions/views/rhs.ts index 76f969930040..4eb51ad74c7d 100644 --- a/webapp/channels/src/actions/views/rhs.ts +++ b/webapp/channels/src/actions/views/rhs.ts @@ -656,7 +656,11 @@ export function setEditChannelMembers(active: boolean) { export function measureRhsOpened() { return () => { - measureAndReport(Measure.RhsLoad, Mark.PostSelected, undefined, true); + measureAndReport({ + name: Measure.RhsLoad, + startMark: Mark.PostSelected, + canFail: true, + }); performance.clearMarks(Mark.PostSelected); }; diff --git a/webapp/channels/src/components/post_view/post_list/post_list.tsx b/webapp/channels/src/components/post_view/post_list/post_list.tsx index 240acfb79da6..a441ca5d329e 100644 --- a/webapp/channels/src/components/post_view/post_list/post_list.tsx +++ b/webapp/channels/src/components/post_view/post_list/post_list.tsx @@ -25,8 +25,24 @@ function markAndMeasureChannelSwitchEnd(fresh = false) { mark(Mark.PostListLoaded); // Send new performance metrics to server - const channelSwitch = measureAndReport(Measure.ChannelSwitch, Mark.ChannelLinkClicked, Mark.PostListLoaded, true); - const teamSwitch = measureAndReport(Measure.TeamSwitch, Mark.TeamLinkClicked, Mark.PostListLoaded, true); + const channelSwitch = measureAndReport({ + name: Measure.ChannelSwitch, + startMark: Mark.ChannelLinkClicked, + endMark: Mark.PostListLoaded, + labels: { + fresh: fresh.toString(), + }, + canFail: true, + }); + const teamSwitch = measureAndReport({ + name: Measure.TeamSwitch, + startMark: Mark.TeamLinkClicked, + endMark: Mark.PostListLoaded, + labels: { + fresh: fresh.toString(), + }, + canFail: true, + }); // Send old performance metrics to Rudder if (shouldTrackPerformance()) { diff --git a/webapp/channels/src/components/threading/global_threads/global_threads.tsx b/webapp/channels/src/components/threading/global_threads/global_threads.tsx index f2ca0316ad09..adb62348ef70 100644 --- a/webapp/channels/src/components/threading/global_threads/global_threads.tsx +++ b/webapp/channels/src/components/threading/global_threads/global_threads.tsx @@ -120,7 +120,11 @@ const GlobalThreads = () => { useEffect(() => { if (!isLoading) { - measureAndReport(Measure.GlobalThreadsLoad, Mark.GlobalThreadsLinkClicked, undefined, true); + measureAndReport({ + name: Measure.GlobalThreadsLoad, + startMark: Mark.GlobalThreadsLinkClicked, + canFail: true, + }); performance.clearMarks(Mark.GlobalThreadsLinkClicked); } }, [isLoading]); diff --git a/webapp/channels/src/utils/performance_telemetry/index.ts b/webapp/channels/src/utils/performance_telemetry/index.ts index 548ebe659d22..9e9966c0eb92 100644 --- a/webapp/channels/src/utils/performance_telemetry/index.ts +++ b/webapp/channels/src/utils/performance_telemetry/index.ts @@ -32,21 +32,34 @@ export function markAndReport(name: string): PerformanceMark { * If either the start or end mark does not exist, undefined will be returned and, if canFail is false, an error * will be logged. */ -export function measureAndReport(measureName: string, startMark: string, endMark: string | undefined, canFail = false): PerformanceMeasure | undefined { +export function measureAndReport({ + name, + startMark, + endMark, + labels, + canFail = false, +}: { + name: string; + startMark: string; + endMark?: string; + labels?: Record; + canFail?: boolean; +}): PerformanceMeasure | undefined { const options: PerformanceMeasureOptions = { start: startMark, end: endMark, detail: { + labels, report: true, }, }; try { - return performance.measure(measureName, options); + return performance.measure(name, options); } catch (e) { if (!canFail) { // eslint-disable-next-line no-console - console.error('Unable to measure ' + measureName, e); + console.error('Unable to measure ' + name, e); } return undefined; diff --git a/webapp/channels/src/utils/performance_telemetry/reporter.test.ts b/webapp/channels/src/utils/performance_telemetry/reporter.test.ts index a8ecd31fedea..40cba5d238f7 100644 --- a/webapp/channels/src/utils/performance_telemetry/reporter.test.ts +++ b/webapp/channels/src/utils/performance_telemetry/reporter.test.ts @@ -36,14 +36,26 @@ describe('PerformanceReporter', () => { performance.mark('testMarkA'); performance.mark('testMarkB'); - measureAndReport('testMeasureA', 'testMarkA', 'testMarkB'); + measureAndReport({ + name: 'testMeasureA', + startMark: 'testMarkA', + endMark: 'testMarkB', + }); await waitForObservations(); performance.mark('testMarkC'); - measureAndReport('testMeasureB', 'testMarkA', 'testMarkC'); - measureAndReport('testMeasureC', 'testMarkB', 'testMarkC'); + measureAndReport({ + name: 'testMeasureB', + startMark: 'testMarkA', + endMark: 'testMarkC', + }); + measureAndReport({ + name: 'testMeasureC', + startMark: 'testMarkB', + endMark: 'testMarkC', + }); await waitForObservations(); diff --git a/webapp/channels/src/utils/performance_telemetry/reporter.ts b/webapp/channels/src/utils/performance_telemetry/reporter.ts index e2e039458349..e4cf85b610c7 100644 --- a/webapp/channels/src/utils/performance_telemetry/reporter.ts +++ b/webapp/channels/src/utils/performance_telemetry/reporter.ts @@ -169,6 +169,7 @@ export default class PerformanceReporter { this.histogramMeasures.push({ metric: entry.name, value: entry.duration, + labels: entry.detail.labels, timestamp: Date.now(), }); }