Closed Bug 1251767 Opened 7 years ago Closed 7 years ago
Add Websocket filter to Network Monitor
My first patch on Bugzilla \o/ We are a couple of guys working on making the developer experience for working with WebSockets better via an extension called websocket-monitor. One of our desired features is to filter the connections in the Network Monitor down to only Websocket upgrades, but it turns out the only good way to implement this feature is by adding it to the network monitor directly. If we try to add it as part of the extension, we run into all kinds of issues with state, which is no good. Today websocket upgrades are shown as XHR, but can be rather tedious to find the right one when there are a lot of XHR. It would therefore be better to be able to filter Websocket upgrades as well. Chrome has had this feature for a while. This proposed patch adds this feature as one of the filter buttons to the network monitor. When clicked, this button makes the list only shows connections that have a Upgrade=websocket header. It also removes said connections from the XHR filter, as that would be redundant (this is the way Chrome does it).
Looks great! I rebased the patch on latest source and attached new version. The feature works nice for me. Here is a try push to make sure tests are passing. https://treeherder.mozilla.org/#/jobs?repo=try&revision=af699bf09f42 Let's see how it goes... (I am NI me so, it's in my review queue) Honza
Attachment #8724275 - Attachment is obsolete: true
Bryan, this patch introduces a new WS (web sockets) filter for the Net panel. Check out the attached screenshot that shows: 1) A new WS button in Net panel's toolbar 2) Only 101 requests in the panel (these represents web socket upgrade/ws connection requests) since the filter is active. Does that sound ok to you? This is also inline with the WebSocket Monitor extension  planning. * Support for WS monitoring should be integrated directly with the Net panel eventually (the extension is currently having its own WebSockets panel). * We want to land the 'web socket monitoring' feature in the tree and ship built-in.  https://github.com/firebug/websocket-monitor Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #2) > Does that sound ok to you? Looks good to me.
Assignee: nobody → dev+mozbugzilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
G(In reply to Bryan Clark (Firefox PM) [:clarkbw] from comment #3) > (In reply to Jan Honza Odvarko [:Honza] from comment #2) > > Does that sound ok to you? > > Looks good to me. Excellent @Espen: can you please rebase the patch on the latest head? (there are changes mostly introduced by making the code eslint clean, bug 1252807) Honza
@Honza Of course I have created a new patch from the most recent source. Could you follow up from here?
Great, thanks, one last thing. The commit message should be: Bug 1251767 - Add WS filter button to net panel; r=Honza (the reviewer part is missing) Honza
Flags: needinfo?(odvarko) → needinfo?(dev+mozbugzilla)
@Honza Yeah, my bad. Here's an updated patch with the commit message you requested
Attachment #8732313 - Attachment is obsolete: true
Flags: needinfo?(dev+mozbugzilla) → needinfo?(odvarko)
Attachment #8732944 - Flags: review+
Pushed a follow-up https://hg.mozilla.org/integration/fx-team/rev/f7311cafabcb Use multiple lines for code if necessary to not have more than 80 characters on a line and make eslint happy
I've updated the bits on the toolbar: https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#Toolbar ...and filtering requests: https://developer.mozilla.org/en-US/docs/Tools/Network_Monitor#Filtering_requests ...to mention this. Let me know if this covers it, please!
Looks good to me! You might want to also mention WebSocket Monitor extension. https://github.com/firebug/websocket-monitor/wiki https://addons.mozilla.org/cs/firefox/addon/websocket-monitor/ Not sure if on this page or any other MDN page that is more suitable for DevTools extensions... Honza
(In reply to Jan Honza Odvarko [:Honza] from comment #13) > Looks good to me! > Thanks Honza! > You might want to also mention WebSocket Monitor extension. > https://github.com/firebug/websocket-monitor/wiki > https://addons.mozilla.org/cs/firefox/addon/websocket-monitor/ > > Not sure if on this page or any other MDN page that is > more suitable for DevTools extensions... It's a good idea. I've added a link from the Network Monitor page, as that seems to be a place people will look. I've also started https://developer.mozilla.org/en-US/docs/Tools/Add-ons. I'll figure out where to link to this page when I reorganize the devtools homepage (much overdue task). > > Honza
Since WebSocket Monitor legacy extension is no more supported in Firefox 57, Would not it be an opportunity to add it natively to the network panel like Chrome?
You need to log in before you can comment on or make changes to this bug.