Add 14 metrics for TCP connections created for tunnels. #9403

ywkaras · 2023-02-13T04:06:11Z

Add current and total metrics for TCP connetions towards clients for blind TCP tunnels, and TLS tunnel, forward, and partial blind tunnel SNI-based tunnels.

Add current and total metrics for TCP connetions towards servers, for blind TCP tunnels and TLS tunnels. Only partial blind tunnel SNI-based tunnels are counted as TLS tunnels on the outgoing side, because they are only SNI-based tunnels where ATS termitates the TLS connection form the client and originates a new one towards the server.

zwoop

There's a number of refactoring / class changes as well here. Even if that is necessary for the new metrics, can we maybe break that out into two separate PRs?

ywkaras · 2023-02-14T00:56:39Z

Fine, this is why we can't have nice class hierarchies.

ywkaras · 2023-02-18T02:23:39Z

Need to describes these in documentation before this will be ready to merge.

ywkaras · 2023-02-26T02:44:57Z

proxy/http/HttpSM.cc

  c_os = tunnel.add_consumer(server_entry->vc, ua_entry->vc, &HttpSM::tunnel_handler_ssl_consumer, HT_HTTP_SERVER,
                             "http server - tunnel");

+  ua_entry->vc->make_tunnel_endpoint();


ua_entry->vc is a VConnection pointer, which is why make_tunnel_endpoint() has to be virtual member function of VConnection.

ywkaras · 2023-02-26T02:45:21Z

proxy/http/HttpSM.cc

                             "http server - tunnel");

+  ua_entry->vc->make_tunnel_endpoint();
+  server_entry->vc->make_tunnel_endpoint();


server_entry->vc is a VConnection pointer, which is why make_tunnel_endpoint() has to be virtual member function of VConnection.

ywkaras · 2023-03-02T04:18:39Z

It turns out that SSL connections to servers made for tunnels are not marked with an SNI tunnel type. I decided not to add this, unless someone feels there's a need for it. This is why the number of new metrics dropped from 20 to 14. 8 metrics for SSL connections to servers for tunnels collaped to 2 (current and total).

ywkaras · 2023-05-29T15:24:41Z

This change has been running in Yahoo prod for over a month.

ywkaras · 2023-06-26T23:03:25Z

Use "mark as" when more appropriate than "make".

ywkaras · 2023-06-27T19:14:49Z

@masaori335 ready for review.

masaori335 · 2023-06-28T06:13:17Z

iocore/eventsystem/I_VConnection.h

+  // This function should be called when the VConnection is a tunnel endpoint.  By default, a VConnection does not care if it
+  // is a tunnel endpoint.
+  virtual void
+  mark_as_tunnel_endpoint()


Why do we need this virtual function in the VConnection class? As you commented, VConnection doesn't care about it's tunnel or something else.

Because it's called through a VConnection pointer at certain places.

This one?

HttpSM::setup_blind_tunnel(bool send_response_hdr, IOBufferReader *initial) { ... ua_entry->vc->mark_as_tunnel_endpoint(); server_entry->vc->mark_as_tunnel_endpoint();

Can we use ua_txn->get_netvc() and server_txn->get_netvc() instead of ua_entry->vc and server_entry->vc here? If possible, I prefer NetVConnection to have this virtual function.

OK I just tried that. server_txn is null at that point in the code, so it doesn't work.

Fair enough, thanks for checking.

masaori335 · 2023-06-28T06:40:21Z

doc/admin-guide/monitoring/statistics/core/ssl.en.rst

+   :type: counter
+
+   Total number of TLS connections for tunnels where the far end is the client
+   initiated with an HTTP request.


This covers "HTTP CONNECT" over TLS connection?

Does that include QUIC connection once we support it?

Not currently. QUICNetVConnection would have to override _in_context_tunnel(). It could use the same metrics or different metrics.

masaori335 · 2023-06-28T06:51:45Z

doc/admin-guide/monitoring/statistics/core/ssl.en.rst

+   :type: counter
+
+   Total number of TCP connections for TLS tunnels where the far end is the server
+   created based on a ``partial_blind_route`` key in a table in the :file:`sni.yaml` file.


Do we need similar server-side metrics for forward_route?

A partial blind tunnel is the only one for which the outgoing connection corresponds to an SSLNetVConnection object rather than a UnixNetVConnection object. It's not straight forward how to implement separate counters for outgoing/server connections for the other TLS tunnel types, but it may be possible if they're needed.

ywkaras · 2023-08-17T16:11:28Z

@masaori335 this has been waiting more than a month for review.

ywkaras · 2023-08-17T16:12:46Z

[approve ci]

ywkaras · 2023-09-14T00:09:27Z

Rebased to remove merge conflicts.

Add current and total metrics for TCP connetions towards clients for blind TCP tunnels, and TLS tunnel, forward, and partial blind tunnel SNI-based tunnels. Add current and total metrics for TCP connetions towards servers, for blind TCP tunnels and TLS tunnels. Only partial blind tunnel SNI-based tunnels are counted as TLS tunnels on the outgoing side, because they are only SNI-based tunnels where ATS termitates the TLS connection form the client and originates a new one towards the server.

Due to change in usage of HttpTransact::State::method.

iocore/net/Net.cc iocore/net/SSLNetVConnection.cc iocore/net/UnixNetVConnection.cc proxy/http/HttpTransact.cc

ywkaras · 2023-10-14T01:20:36Z

Rebased, ready for review.

masaori335

Let's ship this. At least this gives us more clue of what's going on with tunnel.

ywkaras self-assigned this Feb 13, 2023

ywkaras added Tunneling Metrics labels Feb 13, 2023

ywkaras added this to the 10.0.0 milestone Feb 13, 2023

ywkaras force-pushed the tunnel_stats branch from 9e31835 to 13079d5 Compare February 13, 2023 22:12

zwoop reviewed Feb 13, 2023

View reviewed changes

ywkaras force-pushed the tunnel_stats branch from 13079d5 to 322189f Compare February 14, 2023 03:40

ywkaras force-pushed the tunnel_stats branch 3 times, most recently from 907de5f to e559370 Compare February 26, 2023 02:33

ywkaras commented Feb 26, 2023

View reviewed changes

ywkaras force-pushed the tunnel_stats branch 2 times, most recently from b9540ac to 597e442 Compare February 26, 2023 22:31

ywkaras changed the title ~~Add 16 metrics for TCP connections created for tunnels.~~ Add 20 metrics for TCP connections created for tunnels. Feb 26, 2023

ywkaras force-pushed the tunnel_stats branch from 1299568 to 6c362de Compare March 1, 2023 02:29

ywkaras changed the title ~~Add 20 metrics for TCP connections created for tunnels.~~ Add 14 metrics for TCP connections created for tunnels. Mar 1, 2023

ywkaras force-pushed the tunnel_stats branch from 7fe4eb9 to 4cab30e Compare March 1, 2023 23:39

ywkaras force-pushed the tunnel_stats branch 4 times, most recently from 600ce3a to b4db5f2 Compare March 13, 2023 00:39

ywkaras force-pushed the tunnel_stats branch from b4db5f2 to 2257644 Compare May 5, 2023 15:31

ywkaras force-pushed the tunnel_stats branch 2 times, most recently from e5a1505 to e0f71fd Compare May 29, 2023 03:39

ywkaras marked this pull request as ready for review May 29, 2023 15:23

ywkaras mentioned this pull request Jun 14, 2023

proxy.process.tunnel.current_active_connections metric only seems to count TLS tunnels. #9317

Closed

bryancall requested a review from masaori335 June 26, 2023 23:02

ywkaras marked this pull request as draft June 27, 2023 14:42

ywkaras force-pushed the tunnel_stats branch from e0f71fd to d62d5aa Compare June 27, 2023 15:06

ywkaras marked this pull request as ready for review June 27, 2023 19:13

masaori335 reviewed Jun 28, 2023

View reviewed changes

ywkaras force-pushed the tunnel_stats branch from 092df97 to 36299ce Compare August 17, 2023 16:20

ywkaras force-pushed the tunnel_stats branch from 4778ddc to 68b1090 Compare September 13, 2023 22:47

ywkaras marked this pull request as draft October 9, 2023 22:31

ywkaras added 5 commits October 14, 2023 00:23

VConnection::make_tunnel_endpoint() -> mark_as_tunnel_endpoint().

fc95473

Fix error in proxy.process.http.connect_requests.

a7379ba

Due to change in usage of HttpTransact::State::method.

Rebase changes for PR 9869.

c7ef93a

Rebase changes.

70239b5

iocore/net/Net.cc iocore/net/SSLNetVConnection.cc iocore/net/UnixNetVConnection.cc proxy/http/HttpTransact.cc

ywkaras force-pushed the tunnel_stats branch from 68b1090 to 70239b5 Compare October 14, 2023 00:48

ywkaras marked this pull request as ready for review October 14, 2023 01:19

masaori335 approved these changes Oct 17, 2023

View reviewed changes

ywkaras merged commit 124fd70 into apache:master Oct 17, 2023

zwoop added the New Feature label Apr 14, 2025

Add 14 metrics for TCP connections created for tunnels. #9403

Add 14 metrics for TCP connections created for tunnels. #9403

Uh oh!

Conversation

ywkaras commented Feb 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zwoop left a comment

Choose a reason for hiding this comment

Uh oh!

ywkaras commented Feb 14, 2023

Uh oh!

ywkaras commented Feb 18, 2023

Uh oh!

ywkaras Feb 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywkaras commented Mar 2, 2023

Uh oh!

ywkaras commented May 29, 2023

Uh oh!

ywkaras commented Jun 26, 2023

Uh oh!

ywkaras commented Jun 27, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

masaori335 Jun 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ywkaras commented Aug 17, 2023

Uh oh!

ywkaras commented Aug 17, 2023

Uh oh!

ywkaras commented Sep 14, 2023

Uh oh!

ywkaras commented Oct 14, 2023

Uh oh!

masaori335 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ywkaras commented Feb 13, 2023 •

edited

Loading

ywkaras Feb 26, 2023 •

edited

Loading

masaori335 Jun 29, 2023 •

edited

Loading