Skip to content

Commit

Permalink
MM-58535 Add more information to LCP and INP metrics (#27484)
Browse files Browse the repository at this point in the history
* Improve mocking of imported resources in unit tests

We have Webpack configured so that, when code imports an image or other resource, the code gets the URL of that image. Jest now matches that behaviour which is needed because React Testing Library would previously throw an error.

* Polyfill ResizeObserver in all unit tests

* Ensure haveIChannelPermission always returns a boolean value

The previous code could sometimes return undefined. While that should behave the same in practice, it can cause React to print prop type warnings

* MM-58535 Add region label to LCP metrics

* MM-58535 Upgrade web-vitals and add INP attribution

* Change new labels to use snake_case

* Remove replaceGlobalStore option from renderWithContext

I was going to add this in case any tests failed with this option set to false, but after running those tests, that's not the case. I'm going to remove this as an option since it seems more likely than not that anyone using RTL would prefer to have this on.
  • Loading branch information
hmhealey authored Jul 9, 2024
1 parent 99881b8 commit e3b2b13
Show file tree
Hide file tree
Showing 31 changed files with 346 additions and 98 deletions.
14 changes: 12 additions & 2 deletions server/channels/app/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,19 @@ func (a *App) RegisterPerformanceReport(rctx request.CTX, report *model.Performa
case model.ClientFirstContentfulPaint:
a.Metrics().ObserveClientFirstContentfulPaint(commonLabels["platform"], commonLabels["agent"], h.Value/1000)
case model.ClientLargestContentfulPaint:
a.Metrics().ObserveClientLargestContentfulPaint(commonLabels["platform"], commonLabels["agent"], h.Value/1000)
a.Metrics().ObserveClientLargestContentfulPaint(
commonLabels["platform"],
commonLabels["agent"],
h.GetLabelValue("region", model.AcceptedLCPRegions, "other"),
h.Value/1000,
)
case model.ClientInteractionToNextPaint:
a.Metrics().ObserveClientInteractionToNextPaint(commonLabels["platform"], commonLabels["agent"], h.Value/1000)
a.Metrics().ObserveClientInteractionToNextPaint(
commonLabels["platform"],
commonLabels["agent"],
h.GetLabelValue("interaction", model.AcceptedInteractions, "other"),
h.Value/1000,
)
case model.ClientCumulativeLayoutShift:
a.Metrics().ObserveClientCumulativeLayoutShift(commonLabels["platform"], commonLabels["agent"], h.Value)
case model.ClientPageLoadDuration:
Expand Down
4 changes: 2 additions & 2 deletions server/einterfaces/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ type MetricsInterface interface {

ObserveClientTimeToFirstByte(platform, agent string, elapsed float64)
ObserveClientFirstContentfulPaint(platform, agent string, elapsed float64)
ObserveClientLargestContentfulPaint(platform, agent string, elapsed float64)
ObserveClientInteractionToNextPaint(platform, agent string, elapsed float64)
ObserveClientLargestContentfulPaint(platform, agent, region string, elapsed float64)
ObserveClientInteractionToNextPaint(platform, agent, interaction string, elapsed float64)
ObserveClientCumulativeLayoutShift(platform, agent string, elapsed float64)
IncrementClientLongTasks(platform, agent string, inc float64)
ObserveClientPageLoadDuration(platform, agent string, elapsed float64)
Expand Down
12 changes: 6 additions & 6 deletions server/einterfaces/mocks/MetricsInterface.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions server/enterprise/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -1175,7 +1175,7 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf
// Extend the range of buckets for this while we get a better idea of the expected range of this metric is
Buckets: []float64{.005, .01, .025, .05, .1, .25, .5, 1, 2.5, 5, 10, 15, 20},
},
[]string{"platform", "agent"},
[]string{"platform", "agent", "region"},
)
m.Registry.MustRegister(m.ClientLargestContentfulPaint)

Expand All @@ -1186,7 +1186,7 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf
Name: "interaction_to_next_paint",
Help: "Measure of how long it takes for a user to see the effects of clicking with a mouse, tapping with a touchscreen, or pressing a key on the keyboard (seconds)",
},
[]string{"platform", "agent"},
[]string{"platform", "agent", "interaction"},
)
m.Registry.MustRegister(m.ClientInteractionToNextPaint)

Expand Down Expand Up @@ -1783,12 +1783,12 @@ func (mi *MetricsInterfaceImpl) ObserveClientFirstContentfulPaint(platform, agen
mi.ClientFirstContentfulPaint.With(prometheus.Labels{"platform": platform, "agent": agent}).Observe(elapsed)
}

func (mi *MetricsInterfaceImpl) ObserveClientLargestContentfulPaint(platform, agent string, elapsed float64) {
mi.ClientLargestContentfulPaint.With(prometheus.Labels{"platform": platform, "agent": agent}).Observe(elapsed)
func (mi *MetricsInterfaceImpl) ObserveClientLargestContentfulPaint(platform, agent, region string, elapsed float64) {
mi.ClientLargestContentfulPaint.With(prometheus.Labels{"platform": platform, "agent": agent, "region": region}).Observe(elapsed)
}

func (mi *MetricsInterfaceImpl) ObserveClientInteractionToNextPaint(platform, agent string, elapsed float64) {
mi.ClientInteractionToNextPaint.With(prometheus.Labels{"platform": platform, "agent": agent}).Observe(elapsed)
func (mi *MetricsInterfaceImpl) ObserveClientInteractionToNextPaint(platform, agent, interaction string, elapsed float64) {
mi.ClientInteractionToNextPaint.With(prometheus.Labels{"platform": platform, "agent": agent, "interaction": interaction}).Observe(elapsed)
}

func (mi *MetricsInterfaceImpl) ObserveClientCumulativeLayoutShift(platform, agent string, elapsed float64) {
Expand Down
56 changes: 31 additions & 25 deletions server/public/model/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,20 @@ var (
performanceReportVersion = semver.MustParse("0.1.0")
acceptedPlatforms = sliceToMapKey("linux", "macos", "ios", "android", "windows", "other")
acceptedAgents = sliceToMapKey("desktop", "firefox", "chrome", "safari", "edge", "other")

AcceptedInteractions = sliceToMapKey("keyboard", "pointer", "other")
AcceptedLCPRegions = sliceToMapKey(
"post",
"post_textbox",
"channel_sidebar",
"team_sidebar",
"channel_header",
"global_header",
"announcement_bar",
"center_channel",
"modal_content",
"other",
)
)

type MetricSample struct {
Expand All @@ -46,6 +60,10 @@ type MetricSample struct {
Labels map[string]string `json:"labels,omitempty"`
}

func (s *MetricSample) GetLabelValue(name string, acceptedValues map[string]any, defaultValue string) string {
return processLabel(s.Labels, name, acceptedValues, defaultValue)
}

// PerformanceReport is a set of samples collected from a client
type PerformanceReport struct {
Version string `json:"version"`
Expand Down Expand Up @@ -84,37 +102,25 @@ func (r *PerformanceReport) IsValid() error {
}

func (r *PerformanceReport) ProcessLabels() map[string]string {
var platform, agent string
var ok bool

// check if the platform is specified
platform, ok = r.Labels["platform"]
if !ok {
platform = "other"
}
platform = strings.ToLower(platform)

// check if platform is one of the accepted platforms
_, ok = acceptedPlatforms[platform]
if !ok {
platform = "other"
return map[string]string{
"platform": processLabel(r.Labels, "platform", acceptedPlatforms, "other"),
"agent": processLabel(r.Labels, "agent", acceptedAgents, "other"),
}
}

// check if the agent is specified
agent, ok = r.Labels["agent"]
func processLabel(labels map[string]string, name string, acceptedValues map[string]any, defaultValue string) string {
// check if the label is specified
value, ok := labels[name]
if !ok {
agent = "other"
return defaultValue
}
agent = strings.ToLower(agent)
value = strings.ToLower(value)

// check if agent is one of the accepted agents
_, ok = acceptedAgents[agent]
// check if the value is one that we accept
_, ok = acceptedValues[value]
if !ok {
agent = "other"
return defaultValue
}

return map[string]string{
"platform": platform,
"agent": agent,
}
return value
}
2 changes: 1 addition & 1 deletion webapp/channels/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const config = {
'<rootDir>/src/packages/mattermost-redux/test/$1',
'^mattermost-redux/(.*)$': '<rootDir>/src/packages/mattermost-redux/src/$1',
'^.+\\.(jpg|jpeg|png|apng|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$':
'identity-obj-proxy',
'<rootDir>/src/tests/image_url_mock.json',
'^.+\\.(css|less|scss)$': 'identity-obj-proxy',
'^.*i18n.*\\.(json)$': '<rootDir>/src/tests/i18n_mock.json',
},
Expand Down
2 changes: 1 addition & 1 deletion webapp/channels/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@
"tinycolor2": "1.4.2",
"turndown": "7.1.1",
"typescript": "5.3.3",
"web-vitals": "3.5.2",
"web-vitals": "4.2.0",
"zen-observable": "0.9.0"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ exports[`components/FileUploadOverlay should match snapshot when file upload is
<img
alt="Files"
className="overlay__files"
src={null}
src=""
/>
<span>
<i
Expand All @@ -28,7 +28,7 @@ exports[`components/FileUploadOverlay should match snapshot when file upload is
<img
alt="Logo"
className="overlay__logo"
src={null}
src=""
/>
</div>
</div>
Expand All @@ -48,7 +48,7 @@ exports[`components/FileUploadOverlay should match snapshot when file upload is
<img
alt="Files"
className="overlay__files"
src={null}
src=""
/>
<span>
<i
Expand All @@ -63,7 +63,7 @@ exports[`components/FileUploadOverlay should match snapshot when file upload is
<img
alt="Logo"
className="overlay__logo"
src={null}
src=""
/>
</div>
</div>
Expand All @@ -83,7 +83,7 @@ exports[`components/FileUploadOverlay should match snapshot when file upload is
<img
alt="Files"
className="overlay__files"
src={null}
src=""
/>
<span>
<i
Expand All @@ -98,7 +98,7 @@ exports[`components/FileUploadOverlay should match snapshot when file upload is
<img
alt="Logo"
className="overlay__logo"
src={null}
src=""
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ exports[`components/AddGroupsToChannelModal should match when renderOption is ca
alt="group picture"
className="more-modal__image"
height="32"
src={null}
src=""
width="32"
/>
<div
Expand Down Expand Up @@ -163,7 +163,7 @@ exports[`components/AddGroupsToChannelModal should match when renderOption is ca
alt="group picture"
className="more-modal__image"
height="32"
src={null}
src=""
width="32"
/>
<div
Expand Down Expand Up @@ -212,7 +212,7 @@ exports[`components/AddGroupsToChannelModal should match when renderOption is ca
alt="group picture"
className="more-modal__image"
height="32"
src={null}
src=""
width="32"
/>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ exports[`components/AddGroupsToTeamModal should match when renderOption is calle
alt="group picture"
className="more-modal__image"
height="32"
src={null}
src=""
width="32"
/>
<div
Expand Down Expand Up @@ -202,7 +202,7 @@ exports[`components/AddGroupsToTeamModal should match when renderOption is calle
alt="group picture"
className="more-modal__image"
height="32"
src={null}
src=""
width="32"
/>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`components/OpenIdConvert should match snapshot 1`] = `
<img
alt="OpenId Convert Image"
className="OpenIdConvert_image"
src={null}
src=""
/>
</div>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ import type {PostDraft} from 'types/store/draft';

import AdavancedTextEditor from './advanced_text_editor';

global.ResizeObserver = require('resize-observer-polyfill');

const currentUserId = 'current_user_id';
const channelId = 'current_channel_id';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import {Locations} from 'utils/constants';
import FormattingBar from './formatting_bar';
import * as Hooks from './hooks';

global.ResizeObserver = require('resize-observer-polyfill');

jest.mock('./hooks');

const {splitFormattingBarControls} = jest.requireActual('./hooks');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ exports[`components/ConfigurationBar should match snapshot, expired 1`] = `
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
<Memo(MemoizedFormattedMessage)
defaultMessage="{licenseSku} license is expired and some features may be disabled."
Expand All @@ -37,7 +37,7 @@ exports[`components/ConfigurationBar should match snapshot, expired 1`] = `
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
<Memo(MemoizedFormattedMessage)
defaultMessage="{licenseSku} license is expired and some features may be disabled."
Expand Down Expand Up @@ -66,7 +66,7 @@ exports[`components/ConfigurationBar should match snapshot, expired, in grace pe
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
<Memo(MemoizedFormattedMessage)
defaultMessage="{licenseSku} license is expired and some features may be disabled."
Expand All @@ -93,7 +93,7 @@ exports[`components/ConfigurationBar should match snapshot, expired, in grace pe
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
<Memo(MemoizedFormattedMessage)
defaultMessage="{licenseSku} license is expired and some features may be disabled."
Expand All @@ -116,7 +116,7 @@ exports[`components/ConfigurationBar should match snapshot, expired, regular use
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
<Memo(MemoizedFormattedMessage)
defaultMessage="{licenseSku} license is expired and some features may be disabled. Please contact your System Administrator for details."
Expand Down Expand Up @@ -145,7 +145,7 @@ exports[`components/ConfigurationBar should match snapshot, expiring, trial lice
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
This is the last day of your free trial. Purchase a license now to continue using Mattermost Professional and Enterprise features.
</React.Fragment>
Expand All @@ -164,7 +164,7 @@ exports[`components/ConfigurationBar should match snapshot, expiring, trial lice
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
This is the last day of your free trial. Purchase a license now to continue using Mattermost Professional and Enterprise features.
</React.Fragment>
Expand All @@ -183,7 +183,7 @@ exports[`components/ConfigurationBar should match snapshot, expiring, trial lice
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
This is the last day of your free trial.
</React.Fragment>
Expand All @@ -202,7 +202,7 @@ exports[`components/ConfigurationBar should match snapshot, expiring, trial lice
<React.Fragment>
<img
className="advisor-icon"
src={null}
src=""
/>
This is the last day of your free trial.
</React.Fragment>
Expand Down
Loading

0 comments on commit e3b2b13

Please sign in to comment.