Skip to content

Add TCP accept metric which tracks the total number of TCP connections accepted.#1456

Merged
SolidWallOfCode merged 1 commit intoapache:masterfrom
SolidWallOfCode:yts-502
Apr 23, 2017
Merged

Add TCP accept metric which tracks the total number of TCP connections accepted.#1456
SolidWallOfCode merged 1 commit intoapache:masterfrom
SolidWallOfCode:yts-502

Conversation

@SolidWallOfCode
Copy link
Copy Markdown
Member

proxy.process.net.tcp.accept

This is useful to track the rate of inbound connections accepted. This can be compared to transactions, open connections, or other OS network metrics to better identify performance issues.

@SolidWallOfCode SolidWallOfCode self-assigned this Feb 16, 2017
@SolidWallOfCode SolidWallOfCode added this to the 7.2.0 milestone Feb 16, 2017
@atsci
Copy link
Copy Markdown

atsci commented Feb 16, 2017

Linux build successful! See https://ci.trafficserver.apache.org/job/linux-github/1457/ for details.

@atsci
Copy link
Copy Markdown

atsci commented Feb 16, 2017

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/freebsd-github/1563/ for details.

@atsci
Copy link
Copy Markdown

atsci commented Feb 16, 2017

clang-analyzer build successful! See https://ci.trafficserver.apache.org/job/clang-analyzer-github/128/ for details.

@bryancall
Copy link
Copy Markdown
Contributor

Why are there tsconfig changes with this?

@SolidWallOfCode
Copy link
Copy Markdown
Member Author

Bad merge, I'll fix the TSConfig stuff.

@atsci
Copy link
Copy Markdown

atsci commented Feb 16, 2017

Intel CC build successful! See https://ci.trafficserver.apache.org/job/icc-github/6/ for details.

@atsci
Copy link
Copy Markdown

atsci commented Feb 16, 2017

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/freebsd-github/1573/ for details.

@atsci
Copy link
Copy Markdown

atsci commented Feb 16, 2017

Linux build successful! See https://ci.trafficserver.apache.org/job/linux-github/1467/ for details.

@atsci
Copy link
Copy Markdown

atsci commented Feb 17, 2017

clang-analyzer build successful! See https://ci.trafficserver.apache.org/job/clang-analyzer-github/138/ for details.

@zwoop
Copy link
Copy Markdown
Contributor

zwoop commented Feb 17, 2017

Hmmm, how is this different from proxy.process.http.total_incoming_connections ? If it is, then maybe we need to have a proxy.process."something".total_incoming_connections, and then have a Lua metric that adds up all the different types of connections into a "total". That was always the intent of the synthetic metrics.

@SolidWallOfCode
Copy link
Copy Markdown
Member Author

The http stat measures only HTTP 1.X connections and only those that are successfully created. If, for example, there is a connection that fails the TLS handshake, this stat will record it but http.total_incoming_connections will not. Tunnels will also be tracked by tcp_accept and not by the http stat.

It also looks like HTTP 2 connections are not tracked by that stat either, which is also distinct. It might be reasonable to either make the existing stat track both or track them independently and sum.

@jacksontj
Copy link
Copy Markdown
Contributor

It seems like we should have some sort of layered approach, where we keep track of tcp (l4) http (L5?) and TLS (L7). We have metrics for some of this it sounds, but IIRC they are a bit patchy.

So I'm in favor of a raw "tcp accepted", and IMO the http incoming_connections should probably be 1/2 agnostic (if we want an http_stream_accepted-- we can do that too).

@zwoop
Copy link
Copy Markdown
Contributor

zwoop commented Feb 22, 2017

Yes, if there are subtle differences like this, we should break them out accordingly, and then make an aggregated "sum" metrics in the Lua metrics.config.

It almost sounds like a bug / misfeature that the H2 connection is not included in the proxy.process.http.total_incoming_connections. And it's very inconsistent with other metrics; Whereas these two connection metrics works as you describe (independent), the request metrics are not. E.g.

Sending 4 HTTP/2 request, I see this:

./bin/traffic_ctl metric get proxy.process.http2.total_client_streams proxy.process.https.incoming_requests proxy.process.http.incoming_requests
proxy.process.http2.total_client_streams 4
proxy.process.https.incoming_requests 4
proxy.process.http.incoming_requests 4

Note how each H2 stream is counted as an HTTPS request, as well as an HTTP request (I'm guessing because of how H2 uses HTTP internally). This seems rather inconsistent to me, but does make some sense I guess

@shinrich
Copy link
Copy Markdown
Member

shinrich commented Mar 6, 2017

http2 connections are tracked by proxy.process.http2.current_client_sessions and proxy.process.http2.total_client_connections.

All successfully negotiated SSL/TCP connections would be the sum of proxy.process.http2.total_client_connections and proxy.process.http.total_client_connections

@bryancall
Copy link
Copy Markdown
Contributor

We should have a metric that tracks the total number of accepts at the TCP layer. I recommend that the name be inline with the current metrics proxy.process.net.total_accepts

@SolidWallOfCode
Copy link
Copy Markdown
Member Author

@bryancall - do you mean you want proxy.process.net.tcp.total_accepts instead of proxy.process.net.tcp.accepts?

@atsci
Copy link
Copy Markdown

atsci commented Apr 22, 2017

@atsci
Copy link
Copy Markdown

atsci commented Apr 22, 2017

@atsci
Copy link
Copy Markdown

atsci commented Apr 22, 2017

RAT check successful! https://ci.trafficserver.apache.org/job/RAT-github/297/

@atsci
Copy link
Copy Markdown

atsci commented Apr 22, 2017

Linux build successful! https://ci.trafficserver.apache.org/job/linux-github/1870/

@atsci
Copy link
Copy Markdown

atsci commented Apr 22, 2017

Intel CC build successful! https://ci.trafficserver.apache.org/job/icc-github/409/

@atsci
Copy link
Copy Markdown

atsci commented Apr 22, 2017

FreeBSD11 build successful! https://ci.trafficserver.apache.org/job/freebsd-github/1978/

@atsci
Copy link
Copy Markdown

atsci commented Apr 22, 2017

clang-analyzer build failed! https://ci.trafficserver.apache.org/job/clang-analyzer-github/542/

@SolidWallOfCode SolidWallOfCode merged commit 0444182 into apache:master Apr 23, 2017
@zwoop zwoop modified the milestones: 7.2.0, 8.0.0 Apr 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants