Skip to content

Refactor background Segment usage#9509

Merged
rekmarks merged 9 commits intodevelopfrom
segment-background-refactor
Oct 8, 2020
Merged

Refactor background Segment usage#9509
rekmarks merged 9 commits intodevelopfrom
segment-background-refactor

Conversation

@rekmarks
Copy link
Copy Markdown
Member

@rekmarks rekmarks commented Oct 8, 2020

In the background, we currently only use Segment to track data about swaps. However, will soon have cause to record other background metrics using Segment.

This PR refactors our use of Segment in the background such that the caller only has to concern themselves with information relevant to the event being tracked, not metadata that's common to all background events. It also cleans up the metrics functionality in the transactions controller and moves it to its own function. The calculation of the various values sent to Segment is unchanged.

@rekmarks rekmarks requested a review from a team as a code owner October 8, 2020 06:39
@rekmarks rekmarks requested a review from Gudahtt October 8, 2020 06:39
@rekmarks rekmarks changed the title Refactor background segment usage Refactor background Segment usage Oct 8, 2020
}

_trackSwapsMetrics (txMeta) {
if (this._getParticipateInMetrics() && txMeta.swapMetaData) {
Copy link
Copy Markdown
Member Author

@rekmarks rekmarks Oct 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We check if MetaMetrics are enabled here to save us from unnecessarily computing the metrics data. The trackSegmentEvent function also performs this check as a safety measure.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [157e9df]
Page Load Metrics (581 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint309443168
domContentLoaded3507345799646
load3527355819646
domInteractive3507345799646

@rekmarks rekmarks requested review from Gudahtt and brad-decker and removed request for brad-decker October 8, 2020 15:39
brad-decker
brad-decker previously approved these changes Oct 8, 2020
Gudahtt
Gudahtt previously approved these changes Oct 8, 2020
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! One nit but looks good aside from that

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
@rekmarks rekmarks dismissed stale reviews from Gudahtt and brad-decker via 51ab74b October 8, 2020 16:09
@rekmarks rekmarks merged commit 30d6ad8 into develop Oct 8, 2020
@rekmarks rekmarks deleted the segment-background-refactor branch October 8, 2020 16:41
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants