From e9da1ee8ca3042d1c6dcb31b38139ec43acfeb64 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Fri, 13 Oct 2023 14:59:12 -0400 Subject: [PATCH] Performance metrics: Differentiate requests that are from page load (#24327) * POC page_load_context header * refactor to make this.pageLoadContext a private field, and relocate setTimeout code * add pageLoadContext to `ObserveAPIEndpointDuration` method * move to telemetry_actions * move PageContext constant in webapp * give server control of possible page_load_context values * variable name change --------- Co-authored-by: mickmister Co-authored-by: Mattermost Build --- server/channels/web/handlers.go | 9 ++++++++- server/einterfaces/metrics.go | 2 +- server/einterfaces/mocks/MetricsInterface.go | 6 +++--- webapp/channels/src/actions/telemetry_actions.jsx | 15 +++++++++++++++ webapp/channels/src/actions/websocket_actions.jsx | 6 +++++- webapp/channels/src/components/root/root.tsx | 6 ++++-- webapp/channels/src/utils/constants.tsx | 5 +++++ webapp/platform/client/src/client4.ts | 8 ++++++++ 8 files changed, 49 insertions(+), 8 deletions(-) diff --git a/server/channels/web/handlers.go b/server/channels/web/handlers.go index aa920840dd..cc981f7325 100644 --- a/server/channels/web/handlers.go +++ b/server/channels/web/handlers.go @@ -407,8 +407,15 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if r.URL.Path != model.APIURLSuffix+"/websocket" { elapsed := float64(time.Since(now)) / float64(time.Second) + + pageLoadContext := r.Header.Get("X-Page-Load-Context") + if pageLoadContext != "page_load" && pageLoadContext != "reconnect" { + pageLoadContext = "" + } + originClient := string(originClient(r)) - c.App.Metrics().ObserveAPIEndpointDuration(h.HandlerName, r.Method, statusCode, originClient, elapsed) + + c.App.Metrics().ObserveAPIEndpointDuration(h.HandlerName, r.Method, statusCode, originClient, pageLoadContext, elapsed) } } } diff --git a/server/einterfaces/metrics.go b/server/einterfaces/metrics.go index 2cd4baf970..3c347a650d 100644 --- a/server/einterfaces/metrics.go +++ b/server/einterfaces/metrics.go @@ -58,7 +58,7 @@ type MetricsInterface interface { IncrementFilesSearchCounter() ObserveFilesSearchDuration(elapsed float64) ObserveStoreMethodDuration(method, success string, elapsed float64) - ObserveAPIEndpointDuration(endpoint, method, statusCode, originClient string, elapsed float64) + ObserveAPIEndpointDuration(endpoint, method, statusCode, originClient, pageLoadContext string, elapsed float64) IncrementPostIndexCounter() IncrementFileIndexCounter() IncrementUserIndexCounter() diff --git a/server/einterfaces/mocks/MetricsInterface.go b/server/einterfaces/mocks/MetricsInterface.go index 5998bb66ae..2ee3b89ac4 100644 --- a/server/einterfaces/mocks/MetricsInterface.go +++ b/server/einterfaces/mocks/MetricsInterface.go @@ -239,9 +239,9 @@ func (_m *MetricsInterface) IncrementWebsocketReconnectEvent(eventType string) { _m.Called(eventType) } -// ObserveAPIEndpointDuration provides a mock function with given fields: endpoint, method, statusCode, originClient, elapsed -func (_m *MetricsInterface) ObserveAPIEndpointDuration(endpoint string, method string, statusCode string, originClient string, elapsed float64) { - _m.Called(endpoint, method, statusCode, originClient, elapsed) +// ObserveAPIEndpointDuration provides a mock function with given fields: endpoint, method, statusCode, originClient, pageLoadContext, elapsed +func (_m *MetricsInterface) ObserveAPIEndpointDuration(endpoint string, method string, statusCode string, originClient string, pageLoadContext string, elapsed float64) { + _m.Called(endpoint, method, statusCode, originClient, pageLoadContext, elapsed) } // ObserveClusterRequestDuration provides a mock function with given fields: elapsed diff --git a/webapp/channels/src/actions/telemetry_actions.jsx b/webapp/channels/src/actions/telemetry_actions.jsx index 793ffd093f..625bdf1275 100644 --- a/webapp/channels/src/actions/telemetry_actions.jsx +++ b/webapp/channels/src/actions/telemetry_actions.jsx @@ -19,6 +19,8 @@ const SUPPORTS_MEASURE_METHODS = isSupported([ performance.clearMeasures, ]); +const HEADER_X_PAGE_LOAD_CONTEXT = 'X-Page-Load-Context'; + export function isTelemetryEnabled(state) { const config = getConfig(state); return config.DiagnosticsEnabled === 'true'; @@ -262,3 +264,16 @@ function updateRequestCountAtMark(name) { function getRequestCountAtMark(name) { return requestCountAtMark[name] ?? 0; } + +/** + * This allows the server to know that a given HTTP request occurred during page load or reconnect. + * The server then uses this information to store metrics fields based on the request context. + * The setTimeout approach is a "best effort" approach that will produce false positives. + * A more accurate approach will result in more obtrusive code, which would add risk and maintenance cost. + */ +export const temporarilySetPageLoadContext = (pageLoadContext) => { + Client4.setHeader(HEADER_X_PAGE_LOAD_CONTEXT, pageLoadContext); + setTimeout(() => { + Client4.removeHeader(HEADER_X_PAGE_LOAD_CONTEXT); + }, 5000); +}; diff --git a/webapp/channels/src/actions/websocket_actions.jsx b/webapp/channels/src/actions/websocket_actions.jsx index 1cfbe92402..2999b3f798 100644 --- a/webapp/channels/src/actions/websocket_actions.jsx +++ b/webapp/channels/src/actions/websocket_actions.jsx @@ -118,9 +118,11 @@ import RemovedFromChannelModal from 'components/removed_from_channel_modal'; import WebSocketClient from 'client/web_websocket_client'; import {loadPlugin, loadPluginsIfNecessary, removePlugin} from 'plugins'; import {getHistory} from 'utils/browser_history'; -import {ActionTypes, Constants, AnnouncementBarMessages, SocketEvents, UserStatuses, ModalIdentifiers, WarnMetricTypes} from 'utils/constants'; +import {ActionTypes, Constants, AnnouncementBarMessages, SocketEvents, UserStatuses, ModalIdentifiers, WarnMetricTypes, PageLoadContext} from 'utils/constants'; import {getSiteURL} from 'utils/url'; +import {temporarilySetPageLoadContext} from './telemetry_actions'; + const dispatch = store.dispatch; const getState = store.getState; @@ -211,6 +213,8 @@ export function reconnect() { // eslint-disable-next-line console.log('Reconnecting WebSocket'); + temporarilySetPageLoadContext(PageLoadContext.RECONNECT); + dispatch({ type: GeneralTypes.WEBSOCKET_SUCCESS, timestamp: Date.now(), diff --git a/webapp/channels/src/components/root/root.tsx b/webapp/channels/src/components/root/root.tsx index 6137458115..a7c7a6fc7c 100644 --- a/webapp/channels/src/components/root/root.tsx +++ b/webapp/channels/src/components/root/root.tsx @@ -24,7 +24,7 @@ import type {ActionResult} from 'mattermost-redux/types/actions'; import {loadRecentlyUsedCustomEmojis} from 'actions/emoji_actions'; import * as GlobalActions from 'actions/global_actions'; -import {measurePageLoadTelemetry, trackEvent, trackSelectorMetrics} from 'actions/telemetry_actions.jsx'; +import {measurePageLoadTelemetry, temporarilySetPageLoadContext, trackEvent, trackSelectorMetrics} from 'actions/telemetry_actions.jsx'; import BrowserStore from 'stores/browser_store'; import store from 'stores/redux_store'; @@ -53,7 +53,7 @@ import webSocketClient from 'client/web_websocket_client'; import {initializePlugins} from 'plugins'; import Pluggable from 'plugins/pluggable'; import A11yController from 'utils/a11y_controller'; -import {StoragePrefixes} from 'utils/constants'; +import {PageLoadContext, StoragePrefixes} from 'utils/constants'; import {EmojiIndicesByAlias} from 'utils/emoji'; import {getSiteURL} from 'utils/url'; import * as UserAgent from 'utils/user_agent'; @@ -415,6 +415,8 @@ export default class Root extends React.PureComponent { }; componentDidMount() { + temporarilySetPageLoadContext(PageLoadContext.PAGE_LOAD); + this.mounted = true; this.initiateMeRequests(); diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index 9933af4f7b..152264d452 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -2162,4 +2162,9 @@ export const OverActiveUserLimits = { MAX: 0.1, } as const; +export const PageLoadContext = { + PAGE_LOAD: 'page_load', + RECONNECT: 'reconnect', +} as const; + export default Constants; diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index a8ad429e6d..b4c0e8ea40 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -216,6 +216,14 @@ export default class Client4 { this.defaultHeaders['Accept-Language'] = locale; } + setHeader(header: string, value: string) { + this.defaultHeaders[header] = value; + } + + removeHeader(header: string) { + delete this.defaultHeaders[header]; + } + setEnableLogging(enable: boolean) { this.enableLogging = enable; }