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 estimatedSendRate to stats. #494

Merged
merged 3 commits into from
Apr 11, 2023

Conversation

pthatcher
Copy link
Contributor

@pthatcher pthatcher commented Mar 29, 2023

Fixes #484.


Preview | Diff

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
index.bs Outdated
This rate applies to all streams and datagrams that share a [=WebTransport session=]
and is calculated by the congestion control algorithm (potentially chosen by
{{WebTransport/congestionControl}}). If this [=WebTransport session=] is
pooled with others in a shared [=connection=], the value is undefined.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gather this is to satisfy @martinthomson's concern in #421 (comment)?

This might limit media ingest to non-pooled connections. Are we OK with that? Is there no strategy a user agent could employ instead (e.g. divide 50/50 and omit other traffic from the result)?

Or perhaps this is fine?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To start with, this speaks to the feature detection angle more strongly.

The primary concern here is with cross-origin WebTransport sessions. That is, when example.com establishes a session toward https://wt.example/upload and wants to know stuff about that connection. If that connection is used to load stuff for other purposes that wt.example might have (within the same top-level browsing context), then information about size and timing might leak through. We might assume that example.com is not already in a position to observe that much and so adding this new information would be irresponsible.

After getting more comfortable with my own understanding of this situation, I believe that it is already the case that sites that can make queries of cross-site content can learn stuff about how the connection is being used. So we're just changing the fidelity of the measurements and maybe changing/constraining how the browser might be able to act to protect that information from observation. For instance, in the current regime, a browser could isolate requests from different sources in time, so that requests never contend for capacity, at the cost of timing contention; WebTransport might make that sort of trick harder to pull off.

I'm almost at the point where I would instead say that this requirement can be removed, though perhaps we can do that in a follow-up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose that we merge this PR with this stat not being available for pooled connection, and then make a tasks to continue the discussion about pooled connections. Perhaps we solve it, as you and Martin suggest. But I'd like to not have the stat blocked as a whole while we figure it out.

pthatcher and others added 2 commits April 11, 2023 07:39
Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>
@jan-ivar
Copy link
Member

Meeting:

  • Merging this and filing a new issue to discuss pooling.

@jan-ivar jan-ivar merged commit b20068d into w3c:main Apr 11, 2023
github-actions bot added a commit that referenced this pull request Apr 11, 2023
SHA: b20068d
Reason: push, by jan-ivar

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

Add sendRateEstimate to getStats()
3 participants