-
Notifications
You must be signed in to change notification settings - Fork 260
A80: gRPC Metrics for TCP connection #428
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
Closed
Closed
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
dedfb16
Create A80-grpc-metrics-for-tcp-connection
nanahpang ffaeb22
Update A80-grpc-metrics-for-tcp-connection
nanahpang d413291
Update A80-grpc-metrics-for-tcp-connection
nanahpang 5b5ba3f
Update A80-grpc-metrics-for-tcp-connection
nanahpang 583e6b3
Update and rename A80-grpc-metrics-for-tcp-connection to A80-grpc-met…
nanahpang 8aa21c1
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 9f8038c
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 59ab138
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang ce27a69
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang d239c39
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 0726f6e
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 3bfe76b
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 2ccf768
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 83ac908
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 2a11aea
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang b6dc6d9
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 0aceebe
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 052d5cf
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 7e5bc86
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 092fbc1
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang bd18940
Update A80-grpc-metrics-for-tcp-connection.md
nanahpang 006fec9
Update proposal with more information
yashykt d95b572
Merge pull request #1 from yashykt/add-proposal
nanahpang File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| A80: gRPC Metrics for TCP connection | ||
| ---- | ||
| * Author(s): Yash Tibrewal (@yashykt), Nana Pang (@nanahpang), Yousuk Seung (@yousukseung) | ||
| * Approver: Craig Tiller (@ctiller), Mark Roth (@markdroth) | ||
| * Status: {Draft, In Review, Ready for Implementation, Implemented} | ||
| * language: {...} | ||
| * Last updated: 2024-04-18 | ||
| * Discussion at: https://groups.google.com/g/grpc-io/c/AyT0LVgoqFs | ||
|
|
||
| ## Abstract | ||
|
|
||
| This document proposes adding new TCP connection metrics to gRPC for improved network analysis and debugging. | ||
|
|
||
| ## Background | ||
|
|
||
| To improve the network debugging capabilities for gRPC users, we propose adding per-connection TCP metrics in gRPC. The metrics will utilize the metrics framework outlined in [A79]. | ||
|
|
||
| ### Related Proposals: | ||
| * [A79]: gRPC Non-Per-Call Metrics Framework | ||
|
|
||
| [A79]: A79-non-per-call-metrics-architecture.md | ||
|
|
||
| ## Proposal | ||
|
|
||
| This document proposes changes to the following gRPC components. | ||
|
|
||
| ### Per-Connection TCP Metrics | ||
|
|
||
| We will provide the following metrics: | ||
| - `grpc.tcp.min_rtt` | ||
| - `grpc.tcp.delivery_rate` | ||
| - `grpc.tcp.packets_sent` | ||
| - `grpc.tcp.packets_retransmitted` | ||
| - `grpc.tcp.packets_spurious_retransmitted` | ||
|
|
||
| The metrics will be exported as: | ||
|
|
||
| | Name | Type | Unit | Labels | Description | | ||
| | ------------- | ----- | ----- | ------- | ----------- | | ||
| | grpc.tcp.min_rtt | Histogram (double) | s | None | Records TCP's current estimate of minimum round trip time (RTT), typically used as an indication of the network health between two endpoints.<br /> RTT = packet acked timestamp - packet sent timestamp. | | ||
| | grpc.tcp.delivery_rate | Histogram (double) | bit/s | None | Records latest goodput measured of the TCP connection. <br /> Elapsed time = packet acked timestamp - last packet acked timestamp. <br /> Delivery rate = packet acked bytes / elapsed time. | | ||
| | grpc.tcp.packets_sent | Counter (uint64) | {packet} | None | Records total packets TCP sends in the calculation period. | | ||
yashykt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| | grpc.tcp.packets_retransmitted | Counter (uint64) | {packet} | None | Records total packets lost in the calculation period, including lost or spuriously retransmitted packets. | | ||
| | grpc.tcp.packets_spurious_retransmitted | Counter (uint64) | {packet} | None | Records total packets spuriously retransmitted packets in the calculation period. These are retransmissions that TCP later discovered unnecessary.| | ||
|
|
||
| #### Metric Collection Design | ||
|
|
||
| A high-level approach to collecting TCP metrics (on Linux) is as follows: | ||
| 1) **Enable Network Timestamps for Metric Calculation:** Enable the `SO_TIMESTAMPING` option in the kernel's TCP stack through the `setsocketopt(fd, SOL_SOCKET, SO_TIMESTAMPING, &val, sizeof(val))` system call. This enables the kernel to capture packet timestamps during transmission. | ||
| 2) **Calculate Metrics from Timestamps:** Linux kernel calculates TCP connection metrics based on the captured packet timestamps. These metrics can be retrieved using the `getsockopt(TCP_INFO)` system call. For example, the delivery_rate metric estimates the goodput—the rate of useful data transmitted—for the most recent group of outbound data packets within a single flow ([code](https://elixir.bootlin.com/linux/v5.11.1/source/net/ipv4/tcp.c#L391)). | ||
yashykt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| 3) **Periodically Collect Statistics:** At a specified time interval (e.g., every 5 minutes), gRPC aggregates the calculated metrics and updates the corresponding statistics records. | ||
yashykt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
yashykt marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| A detailed explanation of the design can be found in the Fathom documentation. | ||
|
|
||
| #### Reference: | ||
| * Fathom: https://dl.acm.org/doi/pdf/10.1145/3603269.3604815 | ||
| * Kernel TCP Timestamping: https://www.kernel.org/doc/Documentation/networking/timestamping.rst | ||
| * Delivery Rate: https://datatracker.ietf.org/doc/html/draft-cheng-iccrg-delivery-rate-estimation#name-delivery-rate | ||
|
|
||
| ### Metric Stability | ||
|
|
||
| All metrics added in this proposal will start as experimental. The long term goal will be to | ||
| de-experimentalize them and have them be on by default, but the exact | ||
| criteria for that change are TBD. | ||
|
|
||
| ### Temporary environment variable protection | ||
|
|
||
| This proposal does not include any features enabled via external I/O, so | ||
| it does not need environment variable protection. | ||
|
|
||
| ## Implementation | ||
|
|
||
| Will be implemented in C-core, and currently have no plans to implement in other languages. | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Buckets for histogram are explicitly specified for request latency in https://github.com/grpc/proposal/blob/master/A66-otel-stats.md. Do you plan to reuse the same buckets? It might be worth specifying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion, but using buckets for the
min_rttmetric wouldn't offer much significant insights. The value ofmin_rttis bound to the physical length of the path between sender and receiver or the queueing time caused by throttling and load. A step-change inmin_rttvalues usually means that traffic is being throttled or experiencing congestion, or has been re-routed through a different path. I think it's better to opt out buckets in this situation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nanahpang If you do not specify buckets, we will get the default OpenTelemetry buckets which is { 0, 5, 10, 25, 50, 75,100, 250, 500, 750, 1000, 2500, 5000, 7500, 10000}. Are there better defaults that we can suggest or do we leave it to the user to figure it out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding OpenTelemetry buckets, if you're referring to a plugin used for exporting metrics, that would be a separate concern from the metric types we're defining here. However, I agree that specifying a smaller range for the min_rtt metric buckets is the right approach, especially since we're using seconds as the unit (to align with the open-source standard) while the underlying measurements are usually in microseconds.