Closed Bug 1251767 Opened 4 years ago Closed 4 years ago

Add Websocket filter to Network Monitor

Categories

(DevTools :: Netmonitor, enhancement)

enhancement
Not set

Tracking

(firefox48 fixed)

RESOLVED FIXED
Firefox 48
Tracking Status
firefox48 --- fixed

People

(Reporter: dev+mozbugzilla, Assigned: dev+mozbugzilla, NeedInfo)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 3 obsolete files)

Attached patch ws-filter.patch (obsolete) — Splinter Review
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).
Severity: normal → enhancement
Attached patch bug1251767-ws-filter.patch (obsolete) — Splinter Review
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
Flags: needinfo?(odvarko)
Attached image ws-filter.png
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 [1] 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.

[1] https://github.com/firebug/websocket-monitor

Honza
Flags: needinfo?(clarkbw)
(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
Flags: needinfo?(clarkbw)
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
Flags: needinfo?(dev+mozbugzilla)
Attached patch bug1251767-ws-filter-2.patch (obsolete) — Splinter Review
@Honza

Of course

I have created a new patch from the most recent source. Could you follow up from here?
Flags: needinfo?(dev+mozbugzilla)
Attachment #8727929 - Attachment is obsolete: true
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)
Flags: 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
https://hg.mozilla.org/mozilla-central/rev/4ad5b5dfe6e9
https://hg.mozilla.org/mozilla-central/rev/f7311cafabcb
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
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!
Flags: needinfo?(dev+mozbugzilla)
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?
Product: Firefox → DevTools

Currently we cannot see the WebSocket traffic in network tab of Firefox console, right? Is there already an issue opened for this or should we create a new one?

(In reply to baptx from comment #16)

Currently we cannot see the WebSocket traffic in network tab of Firefox console, right? Is there already an issue opened for this or should we create a new one?

Support for monitoring WS traffic is covered by bug 1493147

Honza

You need to log in before you can comment on or make changes to this bug.