Add support for w3c Server-timing headers

NEW
Unassigned

Status

enhancement
P3
normal
2 years ago
2 months ago

People

(Reporter: zac.spitzer, Unassigned)

Tracking

({dev-doc-needed})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [webpagetest])

Attachments

(1 attachment)

Reporter

Description

2 years ago
Server-timing headers is a new w3c standard for including statistics in the headers of a http response

https://w3c.github.io/server-timing/
https://ma.ttias.be/server-timings-chrome-devtools/
https://gist.github.com/paulirish/a76ac17fc211b019e538c09d8d827691

It just requires a header to be parsed and values to be displayed in the timings panel
Thanks for the report!

Honza
Severity: normal → enhancement
Priority: -- → P3

Comment 2

2 years ago
I've tried to add this feature (first bug).
Confusing things:
* Netmonitor could not start after a global component rename (app -> App)
* Netmonitor could not properly render after a recent connector refactoring (sending the props as a fifth argument is not supported in the devtools-launchpad@0.0.96)
* Firebug theme have missed colors in timing, or I just can't build it properly
Attachment #8920871 - Flags: ui-review+
Attachment #8920871 - Flags: feedback?(odvarko)
Comment on attachment 8920871 [details] [diff] [review]
Bug1403051.patch

Review of attachment 8920871 [details] [diff] [review]:
-----------------------------------------------------------------

The patch looks good. I think it goes in the right direction.

* Netmonitor could not start after a global component rename (app -> App)
* Netmonitor could not properly render after a recent connector refactoring (sending the props as a fifth argument is not supported in the devtools-launchpad@0.0.96)
This should be fixed for OSX, see bug 1410782

* Firebug theme have missed colors in timing, or I just can't build it properly
Firebug theme will be removed soon see bug 1378108


Couple of questions:
* Do you have an online test page that is sending some server-timings?
* Do you see any potential performance issues (e.g. parsing headers?)

Thanks for working on this!

Honza
Attachment #8920871 - Flags: feedback?(odvarko) → feedback+

Comment 4

2 years ago
> Do you have an online test page that is sending some server-timings?
My actual project uses server-timing only in dev environment, so I don't know any online page with server-timing.
I've tested patch on that dev version and simple node.js test server.

> Do you see any potential performance issues (e.g. parsing headers?)
Maybe I'm missing something, but I've seen only one bad thing here - sort with toLowerCase call in each comparison.
Did you find any other thing that should fixed?

Also seems this is a duplicate bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1337684

> Thanks for working on this!
Thanks for review :)

Comment 5

2 years ago
Note that since Server-Timing is an HTTP *trailer* (at end of the payload instead of the beginning), it will need some special support from necko.  I've filed bug 1413999 for that.

It does look like bug 1337684 is a dupe of this one?  If not, perhaps it's the one that needs to depend on bug 1413999?
Depends on: 1413999
Hi Honza,

I am currently working on bug 1413999, which is adding support for server timing in necko.
In bug 1413999, I tend to implement the header parser in necko code, but I also see that the code for parsing header is also appeared in the patch in this bug. I am just wondering is this common to parse http headers in js? Do you need only a header string or the parsed data?

Thanks.
Flags: needinfo?(odvarko)
(In reply to Kershaw Chang [:kershaw] from comment #6)
> Hi Honza,
> 
> I am currently working on bug 1413999, which is adding support for server
> timing in necko.
> In bug 1413999, I tend to implement the header parser in necko code, but I
> also see that the code for parsing header is also appeared in the patch in
> this bug. I am just wondering is this common to parse http headers in js?
I think so, DevTools are parsing header e.g. to get content type,
encoding, etc.

> Do you need only a header string or the parsed data?

It's probably useful to have access to both. For example, one of the side
panels in the Network monitor panel shows parsed (and nicely formatted)
Headers as well as raw data (to see what *exactly* came over the wire
from the server).

Honza
Flags: needinfo?(odvarko)
> > Do you need only a header string or the parsed data?
> 
> It's probably useful to have access to both. For example, one of the side
> panels in the Network monitor panel shows parsed (and nicely formatted)
> Headers as well as raw data (to see what *exactly* came over the wire
> from the server).
> 

Could you let me know how do you want to get the data and what is the format you expected?
In bug 1413999, I plan to add nsIServerTiming interface and a nsIArray of nsIServerTiming in nsITimedChannel. The idl would be like:

interface nsIServerTiming : nsISupports {
  [must_use] readonly attribute ACString name;
  [must_use] readonly attribute double duration;
  [must_use] readonly attribute ACString description;
};

interface nsITimedChannel : nsISupports {
  readonly attribute nsIArray serverTiming;
};

Is this enough to you?
Flags: needinfo?(odvarko)
(In reply to Kershaw Chang [:kershaw] from comment #8)
> Could you let me know how do you want to get the data and what is the format
> you expected?
> In bug 1413999, I plan to add nsIServerTiming interface and a nsIArray of
> nsIServerTiming in nsITimedChannel. The idl would be like:
> 
> interface nsIServerTiming : nsISupports {
>   [must_use] readonly attribute ACString name;
>   [must_use] readonly attribute double duration;
>   [must_use] readonly attribute ACString description;
> };
> 
> interface nsITimedChannel : nsISupports {
>   readonly attribute nsIArray serverTiming;
> };
> 
> Is this enough to you?
This looks good and nicely corresponds to the spec [1], so yes should be enough.

Since the server-timings are transferred using HTTP trailer not a header, is there any plan to have API that can be used (from JS) to access raw representation of all HTTP trailers? This could be also visualized for the user in DevTools Toolbox. I understand that server-timings is the only one supported, correct?

Honza


[1] https://w3c.github.io/server-timing/#dfn-server-timing-header-field
Flags: needinfo?(odvarko)
(In reply to Jan Honza Odvarko [:Honza] from comment #9)
> (In reply to Kershaw Chang [:kershaw] from comment #8)
> > Could you let me know how do you want to get the data and what is the format
> > you expected?
> > In bug 1413999, I plan to add nsIServerTiming interface and a nsIArray of
> > nsIServerTiming in nsITimedChannel. The idl would be like:
> > 
> > interface nsIServerTiming : nsISupports {
> >   [must_use] readonly attribute ACString name;
> >   [must_use] readonly attribute double duration;
> >   [must_use] readonly attribute ACString description;
> > };
> > 
> > interface nsITimedChannel : nsISupports {
> >   readonly attribute nsIArray serverTiming;
> > };
> > 
> > Is this enough to you?
> This looks good and nicely corresponds to the spec [1], so yes should be
> enough.
> 
> Since the server-timings are transferred using HTTP trailer not a header, is
> there any plan to have API that can be used (from JS) to access raw
> representation of all HTTP trailers? 

I think I can file another bug for accessing HTTP trailers.

> This could be also visualized for the
> user in DevTools Toolbox. I understand that server-timings is the only one
> supported, correct?
> 

Yes, we will only put server-timings in trailers for now.
(In reply to Kershaw Chang [:kershaw] from comment #10)
> I think I can file another bug for accessing HTTP trailers.
Excellent!

Please cc me (or cross-link here) the bug, so I can track the progress.

Thanks,
Honza
Depends on: 1428250

Comment 12

Last year
FYI: the non-devtools work for server-timing is getting close, so we should look into moving this bug forward again soon.
Flags: needinfo?(odvarko)

Comment 13

Last year
Just to followup on comment 11, we have HttpBaseChannel::GetResponseTrailers for the purpose of getting the trailer array from the channel, so necko side should be good to go here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(odvarko)
@garret: It's now possible to get HTTP trailers (see comment # 13),
and so we can move forward with this bug.

Are you still interested in it?
Should I assign the bug to you to finish it?

Honza
Flags: needinfo?(garret.rus)

Updated

Last year
Product: Firefox → DevTools

Garret wasn't active on Bugzilla for a long time, so I clear the ni.

And for what it's worth, somebody on Discourse was wondering why this feature isn't available yet.

Sebastian

Flags: needinfo?(garret.rus)

Would this be a good first bug?

Flags: needinfo?(odvarko)

Perhaps good first bug for upcoming internship (Outreachy #18)

Honza

Flags: needinfo?(odvarko)
You need to log in before you can comment on or make changes to this bug.