Fix Bug 1468271, make ie performance metrics more reliable #4935

schalkneethling · 2018-08-16T10:51:48Z

@jwhitlock I have tested this change with the various editors. I also tested using various connection profiles (Online, Fast 3G, Slow 3G) in combination with 6x CPU throttling.

From my tests I no longer see the error regarding interactive-editor-loading mark not existing. I also confirmed between production and local that all GA calls are made, and sent.

For the inlined code I also use ga('send' directly as apposed to the utils in analytics.js.

PRODUCTION

ga("send", {hitType: "timing", timingCategory: "RUM - Interactive Examples", timingVar: "ie-time-to-interactive", timingValue: 137})
ga("send", {hitType: "timing", timingCategory: "RUM - Interactive Examples", timingVar: "ie-time-to-complete", timingValue: 179})
ga("send", {hitType: "event", eventCategory: "Interactive Examples", eventAction: "Time to iframe fetch start", eventLabel: "mtPQN-1534413401413", eventValue: 127, hitCallback: null})
ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar: "Time to iframe fetch start", timingValue: 127})
ga("send", {hitType: "event", eventCategory: "Interactive Examples", eventAction: "JS editor load time", eventLabel: "2Cxzy-1534413401652", eventValue: 705, hitCallback: null})
ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar: "JS editor load time", timingValue: 705})
ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar: "js-ie-load-event-end-measure", timingValue: 1040})

LOCAL

ga("send", {hitType: "timing", timingCategory: "RUM - Interactive Examples", timingVar: "ie-time-to-interactive", timingValue: 191})
ga("send", {hitType: "event", eventCategory: "Interactive Examples", eventAction: "Time to iframe fetch start", eventLabel: "wz1no-1534412668571", eventValue: 340, hitCallback: null})
ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar: "Time to iframe fetch start", timingValue: 340})
ga("send", {hitType: "timing", timingCategory: "RUM - Interactive Examples", timingVar: "ie-time-to-complete", timingValue: 253})
ga("send", {hitType: "event", eventCategory: "Interactive Examples", eventAction: "JS editor load time", eventLabel: "ihxea-1534412668872", eventValue: 1079, hitCallback: null})
ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar: "JS editor load time", timingValue: 1079})
ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar: "js-ie-load-event-end-measure", timingValue: 1398})

Let me know if you find anything that does not match our expectation. Thanks!

jwhitlock · 2018-08-16T13:48:28Z

You need to add these to .eslintignore:

kuma/static/js/utils/perf.min.js
kuma/static/js/utils/perf-post-message-handler.min.js

Did the husky precommit hook warn you about these?

schalkneethling · 2018-08-16T13:50:26Z

Did the husky precommit hook warn you about these?

Yup, but I wanted to get the PR in for review ;) Still need to update the ignore patterns and update the PR.

jwhitlock

JS Linting should run without errors.

I'm not thrilled with the magic values being used ('ie-load-event-end', 'https://interactive-examples.mdn.mozilla.net') which means I need to have Javascript from both projects loaded into memory to review. I don't have any suggestions at this time for improving it.

This took me some time to get running locally:

I replaced https://developer.mozilla.org with http://localhost:8000 in js/editor-libs/analytics.js, js/editor-libs/events.js, js/editor-libs/mce-utils.js, and js/editor-libs/perf.js. I ran npm run build to rebuild, and npm run server-start to run the server.
The enviroment-based configuration for INTERACTIVE_EXAMPLES_BASE is broken, and needed fixing.
I sprinkled in some console.log and debugger to figure out issues.
I used the non-minified versions of js/utils/perf.js and js/utils/perf-post-message-listener.js. I didn't confirm that the minified versions are the same.

I used the CSS border page, so my events were:

Running command: ga("send", {hitType: "timing", timingCategory: "RUM - Interactive Examples", timingVar: "ie-time-to-interactive", timingValue: 907.999999995809})

Running command: ga("send", {hitType: "timing", timingCategory: "RUM - Interactive Examples", 

Running command: ga("send", {hitType: "event", eventCategory: "Interactive Examples", eventAction: 

Running command: ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar: 

Running command: ga("send", {hitType: "timing", timingCategory: "Interactive Examples", timingVar:

I seem to be missing the "Time to iframe fetch start" event and timing. I may be missing something in my local config.

jwhitlock · 2018-08-16T15:25:03Z

kuma/static/js/utils/perf-post-message-handler.min.js

@@ -0,0 +1 @@
+function handlePerfMarks(e){e.measureName?(window.mdn.perf.setMark(e.markName),window.mdn.perf.setMeasure({measureName:e.measureName,startMark:e.startMark,endMark:e.endMark}),ga&&ga.create&&ga("send",{hitType:"timing",timingCategory:"RUM - Interactive Examples",timingVar:e.measureName,timingValue:window.mdn.perf.getDuration(e.measureName)||0})):window.mdn.perf.setMark(e.markName)}function perfMsgHandler(e){"use strict";var a=window.mdn.interactiveEditor.editorUrl||"https://interactive-examples.mdn.mozilla.net",r=e.data;if(e.origin!==a)return!1;r.markName&&-1<r.markName.indexOf("interactive-editor-")&&handlePerfMarks(r)}window.addEventListener("message",perfMsgHandler,!1);


It seems this should be called perf-post-message-listener.min.js, since it appears to be the minified version of perf-post-message-listener.js.

Whoops, my bad. You are correct.

schalkneethling · 2018-08-16T16:40:28Z

I'm not thrilled with the magic values being used ('ie-load-event-end', 'https://interactive-examples.mdn.mozilla.net') which means I need to have Javascript from both projects loaded into memory to review. I don't have any suggestions at this time for improving it.

I agree that this is a problem. Seeing @hobinjk is actively working on getting this out of the iframe, perhaps this is temporary for now, and not something we need to fix soon?

I seem to be missing the "Time to iframe fetch start" event and timing. I may be missing something in my local config.

It may be because this happens in Kuma and is not something sent from the iframe?
https://github.com/mozilla/kuma/blob/master/kuma/static/js/interactive.js#L65

jwhitlock · 2018-08-16T18:07:15Z

I'm not thrilled with the magic values being used ('ie-load-event-end', 'https://interactive-examples.mdn.mozilla.net') which means I need to have Javascript from both projects loaded into memory to review. I don't have any suggestions at this time for improving it.

I agree that this is a problem. Seeing @hobinjk is actively working on getting this out of the iframe, perhaps this is temporary for now, and not something we need to fix soon?

I think it is long overdue to work on this, but not in the context of this PR. There's no deadline for @hobinjk's work, and no guarantee that inlining will be appropriate.

I seem to be missing the "Time to iframe fetch start" event and timing. I may be missing something in my local config.

It may be because this happens in Kuma and is not something sent from the iframe?
https://github.com/mozilla/kuma/blob/master/kuma/static/js/interactive.js#L65

Thanks, that got me to the right place. The issue is that interactive-examples.mdn.mozilla.net is hardcoded here as well:

https://github.com/mozilla/kuma/blob/61b9a0d7c77afb6ad6c2999786fe851e89dd2d75/kuma/static/js/interactive.js#L20-L22

If this is changed to look for targetOrigin, then the iframe events are fired.

@schalkneethling

…4936) This addresses some issues when reviewing PR #4935 and getting my Kuma dev environment to talk to a local checkout of https://github.com/mdn/interactive-examples/ * I gave @schalkneethling the wrong command in #4404 (comment) to override ``INTERACTIVE_EXAMPLES_BASE`` from the environment, this corrects it. * The code that looks for the interactive example iframe events was always looking for the production domain, this changes it to look for ``INTERACTIVE_EXAMPLES_BASE`` via ``mdn.interactiveEditor.editorUrl``.

schalkneethling · 2018-08-17T10:25:03Z

@jwhitlock files added to eslintignore, merged your PR on Kuma as well as IE and rebased.

jwhitlock · 2018-08-17T13:40:33Z

@schalkneethling can you match the names of kuma/static/js/utils/perf-post-message-handler.min.js and kuma/static/js/utils/perf-post-message-listener.js?

schalkneethling · 2018-08-17T16:32:34Z

@schalkneethling can you match the names of kuma/static/js/utils/perf-post-message-handler.min.js and kuma/static/js/utils/perf-post-message-listener.js?

Whoops, completely forgot about that.

schalkneethling · 2018-08-17T16:42:00Z

@jwhitlock Filename updated.

schalkneethling added the comp: frontend Component: Frontend label Aug 16, 2018

schalkneethling requested a review from jwhitlock August 16, 2018 10:51

jwhitlock suggested changes Aug 16, 2018

View reviewed changes

jwhitlock mentioned this pull request Aug 16, 2018

bug 1308630, bug 1468271: Fixes for custom Interactive Examples URLs #4936

Merged

Fix Bug 1468271, make ie performance metrics more reliable

cc217e2

jwhitlock approved these changes Aug 17, 2018

View reviewed changes

jwhitlock merged commit 26b889c into mdn:master Aug 17, 2018

jwhitlock mentioned this pull request Aug 21, 2018

Adds check for startMark before measure and beacon, bug 1468271 #4875

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix Bug 1468271, make ie performance metrics more reliable #4935

Fix Bug 1468271, make ie performance metrics more reliable #4935

Uh oh!

schalkneethling commented Aug 16, 2018

Uh oh!

jwhitlock commented Aug 16, 2018

Uh oh!

schalkneethling commented Aug 16, 2018

Uh oh!

jwhitlock left a comment

Uh oh!

jwhitlock Aug 16, 2018

Uh oh!

schalkneethling Aug 16, 2018

Uh oh!

schalkneethling commented Aug 16, 2018

Uh oh!

jwhitlock commented Aug 16, 2018

Uh oh!

schalkneethling commented Aug 17, 2018

Uh oh!

jwhitlock commented Aug 17, 2018

Uh oh!

schalkneethling commented Aug 17, 2018

Uh oh!

schalkneethling commented Aug 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -0,0 +1 @@
		function handlePerfMarks(e){e.measureName?(window.mdn.perf.setMark(e.markName),window.mdn.perf.setMeasure({measureName:e.measureName,startMark:e.startMark,endMark:e.endMark}),ga&&ga.create&&ga("send",{hitType:"timing",timingCategory:"RUM - Interactive Examples",timingVar:e.measureName,timingValue:window.mdn.perf.getDuration(e.measureName)\|\|0})):window.mdn.perf.setMark(e.markName)}function perfMsgHandler(e){"use strict";var a=window.mdn.interactiveEditor.editorUrl\|\|"https://interactive-examples.mdn.mozilla.net",r=e.data;if(e.origin!==a)return!1;r.markName&&-1<r.markName.indexOf("interactive-editor-")&&handlePerfMarks(r)}window.addEventListener("message",perfMsgHandler,!1);

Fix Bug 1468271, make ie performance metrics more reliable #4935

Fix Bug 1468271, make ie performance metrics more reliable #4935

Uh oh!

Conversation

schalkneethling commented Aug 16, 2018

PRODUCTION

LOCAL

Uh oh!

jwhitlock commented Aug 16, 2018

Uh oh!

schalkneethling commented Aug 16, 2018

Uh oh!

jwhitlock left a comment

Choose a reason for hiding this comment

Uh oh!

jwhitlock Aug 16, 2018

Choose a reason for hiding this comment

Uh oh!

schalkneethling Aug 16, 2018

Choose a reason for hiding this comment

Uh oh!

schalkneethling commented Aug 16, 2018

Uh oh!

jwhitlock commented Aug 16, 2018

Uh oh!

schalkneethling commented Aug 17, 2018

Uh oh!

jwhitlock commented Aug 17, 2018

Uh oh!

schalkneethling commented Aug 17, 2018

Uh oh!

schalkneethling commented Aug 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants