Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Highlight H2 push in the waterfall #198

Closed
soulgalore opened this issue Apr 11, 2017 · 13 comments
Closed

Highlight H2 push in the waterfall #198

soulgalore opened this issue Apr 11, 2017 · 13 comments
Assignees

Comments

@soulgalore
Copy link
Collaborator

@tobli has an idea of how we can show pushes directly inside the waterfall graph. @worenga provided us a couple of H2 push URLs that we can use for testing:

https://www.nghttp2.org https://www.filamentgroup.com https://www.middagsfrid.se https://www.forrent.com https://www.mnot.net https://www.benediktwolters.de https://www.eaglesecurity.com https://yeswebsites.net https://www.travelground.com

@soulgalore
Copy link
Collaborator Author

soulgalore commented Apr 11, 2017

See #119 (comment):

push

This is roughly my idea for moving the push arrow closer to where the server actually issues the push. Saves space to the left, and maybe makes slightly more semantical sense. The arrow is slightly lighter as well.

@micmro micmro self-assigned this Jun 26, 2017
@micmro
Copy link
Owner

micmro commented Jun 28, 2017

Options to check if a response was pushed:

  • _was_pushed (WPT, browsertime) - already added to the left legend by @tobli
  • x-http2-push header (added by e.g. h2o, nghttp2, caddy)

Perhaps we can add some more info about the push when hovering over the icon (looks like WPT only)?

...
   "_http2_stream_id": 39,
   "_http2_stream_dependency": 11,
   "_http2_stream_weight": 2,
   "_http2_stream_exclusive": 0,
...

For future reference: Another good source for http2 sites would be this httpArchive BigQuery Script

WPT and browsertime HARs for filamentgroup.com

@micmro micmro closed this as completed Jun 28, 2017
@micmro micmro reopened this Jun 28, 2017
@soulgalore
Copy link
Collaborator Author

Yep we should pickup the header too, good idea. I think @tobli added a screenshot somewhere on how we can highlight it in the waterfall.

@micmro
Copy link
Owner

micmro commented Jun 28, 2017

Added the screenshot and @tobli's comment as quote to your initial reference.

@worenga
Copy link

worenga commented Jun 28, 2017

The x-http2-push header is in no shape or Form standard. E.g. CDNs such as Cloudflare are using different ones such as cf-h2-pushed.

Instead of incorporating this zoo of possibilities we should concentrate on Browser feedback

@micmro
Copy link
Owner

micmro commented Jun 28, 2017

I'm aware it's not a standard, but better than nothing.

What exactly do you mean with "Browser feedback"?

@soulgalore
Copy link
Collaborator Author

soulgalore commented Jun 28, 2017

Yes think we should add those extra WPT only, we haven't had the chance to add it browsertime but when we do, we can follow the same standard.

I think it good for us to catch what we can and then when HAR spec 2.0 comes out we can follow the new standard :D

@micmro
Copy link
Owner

micmro commented Jun 30, 2017

Still working on it, but here's a quick preview:
screen shot 2017-06-30 at 3 36 49 pm

Looks a bit funny with no block size though.
Any thoughts?

@tobli
Copy link
Collaborator

tobli commented Jun 30, 2017

I like it (but I'm biased ;-). Good work!
Are you thinking this should be the default, or available as an option? Having push arrows both inline and on the left looks a bit strange to me.

@soulgalore
Copy link
Collaborator Author

@micmro looks great! I agree with @tobli we should disable the ones to the left when we got the ones inline.

@micmro
Copy link
Owner

micmro commented Jun 30, 2017

Thanks. I meant to da that, but have not gotten around to do so :).
@tobli I'd say we keep the inline ones default and make the ones on the left ones switchable: show, showBoth, hide (default) or so.
I also want to play a bit with the colour to make it a bit more subtle, and perhaps use the font's arrows.

micmro pushed a commit that referenced this issue Feb 23, 2018
@micmro
Copy link
Owner

micmro commented Feb 23, 2018

Finally (after a very long time) I got around to work on this issue again.
Replaced the arrow with font arrows and removed them form the icon list, rather than making in configurable. Just the hover info does not show up unfortunately.

You can checkout branch 198 for the code changes.

screen shot 2018-02-24 at 12 06 49 am

micmro pushed a commit that referenced this issue Apr 1, 2018
micmro pushed a commit that referenced this issue Apr 1, 2018
micmro added a commit that referenced this issue Apr 9, 2018
#198 add inline h2 push icons
@micmro
Copy link
Owner

micmro commented Apr 9, 2018

Released via v2.5.0

@micmro micmro closed this as completed Apr 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants