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

Add "group" concept to performance timeline #9

Closed
wants to merge 1 commit into from
Closed

Conversation

igrigorik
Copy link
Member

There is a need for a common concept of an "event group" in the Performance Timeline, which would allow the UA to associate one or more distinct events (of same or different types) into a logical group.

Some examples of where this is necessary:

  • Nav Timing: nav request -> one or more redirects -> destination
  • Res Timing: cors preflight -> request; multi-part/resumed fetch
  • Frame Timing: main frame -> one or more composite events
  • ... and so on.

In each of the above cases the UA can assign a common group to indicate that the events share a common parent. In turn, the developer can check the group ID of each event and fetch other associated events if necessary - e.g. follow a redirect chain; fetch render+composite events, and so on.

This commit adds the following concepts:

  • adds "group" identifier to PerformanceEntry
  • adds getEntriesByGroup() to Performance Timeline

There is a need for a common concept of an "event group" in the
Performance Timeline, which would allow the UA to associate one or more
distinct events (of same or different types) into a logical group.

Some examples of where this is necessary:
- Nav Timing: nav request -> one or more redirects -> destination
- Res Timing: cors preflight -> request; multi-part/resumed fetch
- Frame Timing: main frame -> one or more composite events
- ... and so on.

In each of the above cases the UA can assign a common group to indicate
that the events share a common parent. In turn, the developer can check
the group ID of each event and fetch other associated events if
necessary - e.g. follow a redirect chain; fetch render+composite events,
and so on.

This commit adds the following concepts:
- adds "group" identifier to PerformanceEntry
- adds getEntriesByGroup() to Performance Timeline
@nicjansma
Copy link

Another example: ServerTiming grouped with the corresponding ResourceTiming entry

@nicjansma
Copy link

Is it worth it to have a second optional parameter for getEntriesByGroup() for the specific entryType?

eg:

PerformanceEntryList getEntriesByGroup(DOMString group, optional DOMString entryType);

getEntriesByName() has this option. I could see it being useful for the ServerTiming->ResourceTiming link, though maybe not as useful for the other examples.

@natduca
Copy link

natduca commented Mar 24, 2015

I'm not sure it makes sense to merge these use cases across domains. Group means very different things, and assumes that This Group is The Only Group. Suppose we add other groupings down the road? What do we do then?

I can't help but think that the real bug here that we want a getEntriesMatching(predicate) and then we can add domain-specific fields.

@igrigorik
Copy link
Member Author

@natduca I'm not sure what you mean by "domain". The "group" is simply an arbitrary unique ID (UUID, integer counter, or some such) that can be used to link multiple records into a logical group. As such, there are no restrictions on number of groups or how they should be used to link records together -- it's just a key to group things. Perhaps "group" is not the best choice of name, but I haven't been able to come up with anything substantially better so far -- if you have any suggestions, lmk!

@nicjansma ah, right, that's another great use case - good call.

@toddreifsteck
Copy link
Member

Another possible use case would be adding group to user-timing to record relation to frame-timing or synchronous resource-timing.

@igrigorik
Copy link
Member Author

Thinking about this some more, and with benefit of lots of great feedback from various folks...


A "group" is formed by a common key between two entities. In the case of Performance objects recorded in the timeline, any two objects with the same attribute (and optionally, some predicate on the value) can be said to form a group. The simplest example are the existing getEntriesByType and getEntriesByName methods on the Performance Timeline, which filter (and "group") records by common attribute name and value:

  • getEntriesByType(DOMString entryType): filter the timeline for records that have attribute type and whose value is entryType.
  • getEntriesByName(DOMString entryName, optional DOMString entryType): filter the timeline for records that have attribute name and whose value is entryName, and optionally... records that have attribute type and whose value is entryType.

Of course, the same pattern applies to any arbitrary set of attribute name/value's. As long as any two entries share a common attribute, they can be "grouped" by applying the same filtering steps; they can also belong to "multiple groups" by having multiple attributes, and so on.

In other words, we can do much better than imposing a single "group" attribute...

  • Performance Entry objects that can be linked together should expose one or more common attributes (e.g. name, type, sourceFrameNumber, and so on).
  • The objects can then be linked together by filtering on attribute's key/value - e.g. the application can simply get all the entries in the (global or observed) timeline, and iterate through the entries by applying some predicate function (e.g. name must be X, value Y).

Maybe, as a convenience, we could/should consider exposing a simple convenience method that can make this easier, and that explains our existing methods... For the sake of an example:

getEntries(DOMString attributeName, optional DOMString attributeValue)

  • getEntriesByType(entryType) -> getEntries('type', entryType)
  • getEntriesByName(entryName) -> getEntries('name', entryName)
  • getEntriesByName(entryName, entryType) -> getEntries('name', entryName).getEntries('type', entryType)

More importantly though, we first need to ensure that existing PerformanceEntry records that ought to be linked surface some common attribute key/value's:

  1. Frame Timing: render event -> (one or more) composite events.
    • Render events already expose sourceFrameNumber which is used to link or or more Composite events to it.
  2. Nav Timing: nav request -> (one or more) redirect -> destination
    • Multiple requests are required to fulfill a navigation request. To link them we can expose a common "requestId" (or some such, don't pay attention to the name :)) attribute, whose value is shared between all objects in this sequence.
  3. Resource Timing: cors preflight -> request; multi-part/resumed fetch
    • Same as above.
  4. Nav/Resource Timing + Server Timing
    • Each Nav/Resource timing object may have a server timing entry associated with it. To link them we can expose a common "serverId" attribute, whose value is shared between the ServerEntry and Nav/ResourceEntry objects.

... and so on.

WDYT? Crazy talk? :)

@lawnsea
Copy link

lawnsea commented Apr 3, 2015

@igrigorik Given this design, I wonder if it makes sense to allow attributes to be added to User Timing entries.

@igrigorik
Copy link
Member Author

@lawnsea if we end up going down this route, it may.. but I think that's a whole separate discussion.

@lawnsea
Copy link

lawnsea commented Apr 3, 2015

@igrigorik Ok. An example usecase we have at @bazaarvoice right now where grouping User Timing entries might be convenient is Dapper/Zipkin-style tracing of client-side code.

@lawnsea
Copy link

lawnsea commented Apr 3, 2015

@igrigorik I'm happy to open a bug on the User Timing repo if that's the right place for this discussion.

@igrigorik
Copy link
Member Author

@lawnsea please do, a full example of where and how it would be used would be great to have. Also, check out #2, as I think it overlaps with what you're proposing.

@nicjansma
Copy link

@lawnsea Added an issue to UserTiming w3c/user-timing#3

@igrigorik
Copy link
Member Author

Closing, superseded by #11.

@igrigorik igrigorik closed this May 1, 2015
@igrigorik igrigorik deleted the group branch June 15, 2015 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants