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

Interface for request tracing #3533

Closed
gordon-y-lai opened this issue Jul 15, 2021 · 6 comments
Closed

Interface for request tracing #3533

gordon-y-lai opened this issue Jul 15, 2021 · 6 comments
Assignees
Labels
priority: P4 Nice to have / wishful thinking status: archived Archived and locked; will not be updated type: enhancement New feature or request

Comments

@gordon-y-lai
Copy link

In our use case it is important to be able to collect some key information regarding the player's network requests (for example, the download request for a media segment). In particular, we would like to measure the start time, header arrival time, completion time, and the response status code for each request individually.

For now we are able to achieve the goals above by registering our own networking plugin (XHR, http/https) through shaka.net.NetworkingEngine.registerScheme(). However, the solution isn't ideal because it will not work properly if there are more than one Shaka player instances on the same page. This is due to the fact that the registration of the networking plugin is handled globally for all player instances, there is no way for us to know which request is associated with which player instance in more complex scenarios.

I believe there are several possible ways Shaka can help to make it easier to trace network requests:

  1. When invoking the networking plugin method parse(uri, request, requestType, progressUpdated), pass an extra argument "player" to reference the associated Shaka player instance. This way, we will be able to know which player a request belongs to.

  2. Introduce a new networking plugin or request interceptor which can be registered on a player instance-by-instance basis.

  3. Introduce new Shaka player events which are triggered whenever a request started/progressed/ended with corresponding status code.

Thanks!

@theodab
Copy link
Contributor

theodab commented Jul 22, 2021

Would request filters and response filters work for your use case? They aren't registered globally. They are mentioned in a few of the tutorials, such as for example the license wrapping tutorial.
The shaka.extern.Request and shaka.extern.Response don't directly link to each other, but you could figure out which response corresponds to which request by checking the originalUri field of the response, and looking for a request with that same URI in its uris array. If you're using range requests, you'll also have to check to see if the range of the request and the Content-Range of the response match up.
And the type field in the filters lets you figure out if the request/response is a segment, a header, etc.

@gordon-y-lai
Copy link
Author

@theodab - Thanks for the information. I tried the request/response filters approach as you suggested. I think it should work (since the filters can be registered for a specific player instance) to an extent while there were some shortcomings I observed. In particular:

  • For our use case, the response timing metrics include two check points: (1) when the response header arrives (to capture the end-to-end latency since request was made), and (2) when the response body becomes fully available (to estimate the actual download bandwidth).
  • It seems that the response filter is invoked after the response body becomes fully available, and there is a timeMs field in the response argument which looks like the elapsed time since the corresponding request was made. If I understand it correctly, we won't be able to measure the timing of check point # 1 as described above with this new approach.
  • It is equally important to our use case that failed downloads (and the corresponding HTTP status code) are captured. In my quick tests with bad segment URLs, it appears that the response filters will not be involved in these error conditions. So we will need to find other ways to get notified when requests run into errors.
    Please let me know if I'm missing something.

@theodab
Copy link
Contributor

theodab commented Jul 22, 2021

Ah, I see what you mean. The time that headers become available is something we could be measuring, but we don't at the moment. And we don't really inform the user of a failed download, directly; they only know if a failed download leads to the entire player erroring, but they won't hear if a failed download leads to a retry or similar.

I'll mark this as an enhancement request, then.

To me, adding new player events sounds like the best solution. How would this set of events look to you?

  1. 'downloadheadersreceived': Fired when the headers for a download are received. Extra data: the request object, the request type, and the headers (or maybe the response object, if it's available at that stage?).
  2. 'downloadfailed': Fired when a download fails. Extra data: the request object, the request type, the error code, and a boolean that indicates whether or not the request was purposefully aborted.

@theodab theodab added type: enhancement New feature or request priority: P2 Smaller impact or easy workaround and removed needs triage labels Jul 22, 2021
@shaka-bot shaka-bot added this to the Backlog milestone Jul 22, 2021
@gordon-y-lai
Copy link
Author

The player events sound reasonable. I like to add a few comments:

  • If you also add 'downloadrequested' and 'downloadcompleted' events in a similar fashion, then technically it will be possible for us to achieve our goal without registering request/response filters. I guess that's nice but it's not a must have.
  • I also like to make sure that the original HTTP response status code (aside from the Shaka error code) is available to the 'downloadfailed' event handler, since the status code can be a crucial piece of information to link the error with what's been reported on the CDN side for example.

@theodab theodab self-assigned this Jul 23, 2021
shaka-bot pushed a commit that referenced this issue Jul 29, 2021
This adds two new player events, 'downloadheadersreceived' and
'downloadfailed', to allow users to measure network performance
in greater detail.

Issue #3533

Change-Id: I33a3bd411d815e926d4bea2184e8d3ea69e2bb49
@theodab
Copy link
Contributor

theodab commented Jul 29, 2021

Okay. I've added 'downloadheadersreceived' and 'downloadfailed'.
For now, I decided to not add your proposed 'downloadrequested' and 'downloadcompleted' events; since those are replicable using our existing interfaces, they are lower priority features. In case you're still interested I'll leave this issue open to track them.

@theodab theodab added priority: P4 Nice to have / wishful thinking and removed priority: P2 Smaller impact or easy workaround labels Jul 29, 2021
@avelad avelad added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Jul 5, 2023
@github-actions
Copy link
Contributor

Closing due to inactivity. If this is still an issue for you or if you have further questions, the OP can ask shaka-bot to reopen it by including @shaka-bot reopen in a comment.

@github-actions github-actions bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Jul 12, 2023
@avelad avelad removed this from the Backlog milestone Aug 28, 2023
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Sep 10, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Sep 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P4 Nice to have / wishful thinking status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants