New issue
Advanced search Search tips

Issue 669700 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Improve logging around socket pools

Project Member Reported by davidben@chromium.org, Nov 29 2016

Issue description

In diagnosing issue #488043 and a duplicate of it, I noticed the socket pools don't log very well. This bug tracks a bunch of stuff to improve this:

1. SSL requests make two CONNECT_JOBs. It's hard to tell which is the SSLConnectJob and which is the TransportConnectJob. Per discussion with mmenke@, split these into different source types per subclass.

2. Socket pools mint their ConnectJob parameters from a particular request and assume these parameters are the same across the group. It's pretty easy to mess this up (issue #488043). Add a link from the ConnectJob up to the request which spawned it. Note this is distinct from the request it was bound to. Add this to SOCKET_POOL_CONNECT_JOB.

3. On success, we get the following links:

  CONNECT_JOB => SOCKET to find out which socket you used (absent on idle)
  request => CONNECT_JOB to find out which ConnectJob you bound to (absent on idle)
  request => SOCKET to find out which socket you ultimately got
  SOCKET => request to find out who is using a socket

On error, we're missing a lot of these. Note that certificate errors and other errors behave differently. (It depends on whether SetSocket gets called anyway.) It would be useful to be able to answer questions like:

- This object at this layer failed with ERR_FOO. Where did ERR_FOO come from?
- This object at this layer produced (or inherited) ERR_FOO. Where did ERR_FOO get fed into?

I feel a little weird adding the full mix of edges since there's some redundancy here, but we'll see. I'll stare at that once I've done 1 and 2.
 
Hrm, doing 2 without filling in the CONNECT_JOB => request pointer makes things confusing as it looks like the CONNECT_JOB has been bound already, so that's an edge that probably always wants to be there.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 30 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b7048f096092bcb293216921c35df413258dbd57

commit b7048f096092bcb293216921c35df413258dbd57
Author: davidben <davidben@chromium.org>
Date: Wed Nov 30 21:20:26 2016

Use different source types for each ConnectJob subclass.

This makes it easier to distinguish the different CONNECT_JOB entries
that get spawned in the more complex cases. Notably, SSL produces two of
them.

BUG=669700
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2537373002
Cr-Commit-Position: refs/heads/master@{#435419}

[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/chrome/browser/resources/net_internals/source_entry.js
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/chrome/test/data/webui/net_internals/log_view_painter.js
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/cert/multi_log_ct_verifier_unittest.cc
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/docs/crash-course-in-net-internals.md
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/http/http_proxy_client_socket_pool.cc
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/log/net_log_source_type_list.h
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/socket/client_socket_pool_base_unittest.cc
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/socket/socks_client_socket_pool.cc
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/socket/ssl_client_socket_pool.cc
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/socket/transport_client_socket_pool.cc
[modify] https://crrev.com/b7048f096092bcb293216921c35df413258dbd57/net/socket/websocket_transport_client_socket_pool.cc

You started fixing this bug over two years ago. Are you still working on it? You can update the status to "archived", "wontfix", or "closed". You can remove yourself as owner and change status to "untriaged", but if this is still a real bug, please do not sit on it.
Cc: davidben@chromium.org
Owner: ----
Status: Available (was: Started)
Marking this as available in case anyone feels like doing (2) or (3).

Sign in to add a comment