Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Geckoprofiler trace with Visual metrics enabled from Browsertime gets an error #3966

Closed
soulgalore opened this issue Mar 31, 2022 · 6 comments · Fixed by #3967
Closed

Geckoprofiler trace with Visual metrics enabled from Browsertime gets an error #3966

soulgalore opened this issue Mar 31, 2022 · 6 comments · Fixed by #3967
Labels
bug Very important to fix, typically this means that the tool is broken or lying profile data Issues related to the profile format, data structure, or profile upgraders

Comments

@soulgalore
Copy link

Hi!
If I drag and drop a geckoProfile.json directly from Browsertime, it works fine, but if I enable visual metrics in Browsertime I get the following error:
Screenshot 2022-03-31 at 08 23 07

I think there are some special handling of the visual metrics in the profiler but it seems to have stopped working.

I've added the file here: https://github.com/sitespeedio/browsertime/blob/geckoprofile/test/geckoProfile-1.json

@julienw
Copy link
Contributor

julienw commented Mar 31, 2022

I believe this comes from the assumption that we'll always have all metrics.

Indeed in this code we add all 3 types of metrics:

// Add the visual progress tracks if we have visualMetrics data.
if (profile.meta && profile.meta.visualMetrics) {
globalTracks.push({ type: 'visual-progress' });
globalTracks.push({ type: 'perceptual-visual-progress' });
globalTracks.push({ type: 'contentful-visual-progress' });
}

But in the profile you shared, we only have the first one visual-progress, not the other ones perceptual-visual-progress and contentful-visual-progress.
This is where we get the data for these tracks:

export const getVisualProgress: Selector<ProgressGraphData[]> = (state) =>
getVisualMetrics(state).VisualProgress;
export const getPerceptualSpeedIndexProgress: Selector<ProgressGraphData[]> = (
state
) => getVisualMetrics(state).PerceptualSpeedIndexProgress;
export const getContentfulSpeedIndexProgress: Selector<ProgressGraphData[]> = (
state
) => getVisualMetrics(state).ContentfulSpeedIndexProgress;

This should be fairly easy to fix: we can check the presence for each values before adding the tracks.

I don't know well browsertime and its options, can you please explain how we get these metrics, and is it common that we can have one but not the others?

@julienw julienw added bug Very important to fix, typically this means that the tool is broken or lying profile data Issues related to the profile format, data structure, or profile upgraders labels Mar 31, 2022
@julienw
Copy link
Contributor

julienw commented Mar 31, 2022

@soulgalore I fixed this specific issue in a local patch, but now I see another issue: the data itself for VisualProgress looks bad, as all timestamp values are undefined. Is that expected?

Deploy preview with the fix: https://deploy-preview-3967--perf-html.netlify.app/

@julienw
Copy link
Contributor

julienw commented Mar 31, 2022

I don't know well browsertime and its options, can you please explain how we get these metrics, and is it common that we can have one but not the others?

I wonder if these options would be added only using the mozilla script at https://github.com/mozilla/browsertime/blob/3329a2c1f5b60033917f7da0a5f8298432055e8c/vendor/visualmetrics.py#L1318-L1340

I'd be interested to know the difference (I mean: how to do each one of these) between the 2 options you mention in your issue description:

  1. If I drag and drop a geckoProfile.json directly from Browsertime, it works fine
  2. but if I enable visual metrics in Browsertime

Thanks!

@soulgalore
Copy link
Author

@julienw thanks for the fix, I verified that it worked for me.

I fixed this specific issue in a local patch, but now I see another issue: the data itself for VisualProgress looks bad, as all timestamp values are undefined. Is that expected?

Thats seems wrong, maybe it copied the wrong? I never touched that code myself but I can have a look tonight and see if I can fix it.

If I drag and drop a geckoProfile.json directly from Browsertime, it works fine
but if I enable visual metrics in Browsertime

Sorry let me explain that better. You can run Browsertime with and without recording a video of the screen (= there will not be any visual metrics).

@soulgalore
Copy link
Author

I wonder if these options would be added only using the mozilla script at

About Contentful Speed Index etc, that is turned off by default and you need to enable them, that's why they was not in my trace.

@julienw
Copy link
Contributor

julienw commented Mar 31, 2022

Thanks, I think I managed to capture this in the new comments in ab64434#diff-e966a6863aad93f9d9faa8bc61f711a4afc9ec49d2dfbb8694482dd79ffbc5ac=

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Very important to fix, typically this means that the tool is broken or lying profile data Issues related to the profile format, data structure, or profile upgraders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants