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.

Sign up for GitHub

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

Jump to bottom

Add attributes request/response body size for rpc #1281

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Add attributes request/response body size for rpc #1281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2d1d094
add attribute for rpc
crossoverJie Jul 25, 2024
4328535
add changelog
crossoverJie Jul 25, 2024
ca94a14
add issues id
crossoverJie Jul 25, 2024
7f370ca
fix with cr
crossoverJie Jul 31, 2024
9b422fb
Merge branch 'main' into rpc-attr-body
crossoverJie Jul 31, 2024
a9ba7d2
add grpc body size description
crossoverJie Aug 1, 2024
f8aabcf
Merge branch 'main' into rpc-attr-body
crossoverJie Aug 1, 2024
e468b26
fix ci
crossoverJie Aug 1, 2024
e63d2f6
fix ci
crossoverJie Aug 1, 2024
e60455b
Merge branch 'main' into rpc-attr-body
crossoverJie Aug 2, 2024
47fa148
Merge master
crossoverJie Aug 2, 2024
0cfee17
Update model/registry/rpc.yaml
crossoverJie Aug 7, 2024
9aef0f4
Update model/registry/rpc.yaml
crossoverJie Aug 7, 2024
eacb4a1
Update model/trace/rpc.yaml
crossoverJie Aug 7, 2024
e06b9e1
Update model/trace/rpc.yaml
crossoverJie Aug 7, 2024
2e0e628
Merge branch 'main' into rpc-attr-body
crossoverJie Aug 7, 2024
1da2208
merge CR
crossoverJie Aug 7, 2024
228457e
fix ci
crossoverJie Aug 7, 2024
80e7bd2
fix ci
crossoverJie Aug 7, 2024
9d4c250
Merge branch 'main' into rpc-attr-body
crossoverJie Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .chloggen/rpc-attr-body.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Use this changelog template to create an entry for release notes.
#
# If your change doesn't affect end users you should instead start
# your pull request title with [chore] or use the "Skip Changelog" label.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the area of concern in the attributes-registry, (e.g. http, cloud, db)
component: rpc

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: add rpc request/response body size attributes

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
# The values here must be integers.
issues: [1281]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
10 changes: 8 additions & 2 deletions docs/attributes-registry/rpc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@ This document defines attributes for remote procedure calls.
| `rpc.message.type` | string | Whether this is a received or sent message. | `SENT`; `RECEIVED` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `rpc.message.uncompressed_size` | int | Uncompressed size of the message in bytes. | | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `rpc.method` | string | The name of the (logical) method being called, must be equal to the $method part in the span name. [6] | `exampleMethod` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `rpc.service` | string | The full (logical) name of the service being called, including its package name, if applicable. [7] | `myservice.EchoService` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `rpc.request.body.size` | int | The size of the request payload body in bytes. [7] | `3495` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `rpc.response.body.size` | int | The size of the response payload body in bytes. [8] | `3495` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `rpc.service` | string | The full (logical) name of the service being called, including its package name, if applicable. [9] | `myservice.EchoService` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| `rpc.system` | string | A string identifying the remoting system. See below for a list of well-known identifiers. | `grpc`; `java_rmi`; `dotnet_wcf` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |

**[1]:** Instrumentations SHOULD require an explicit configuration of which metadata values are to be captured. Including all request metadata values can be a security risk - explicit configuration helps avoid leaking sensitive information.
Expand All @@ -45,7 +47,11 @@ This document defines attributes for remote procedure calls.

**[6]:** This is the logical name of the method from the RPC interface perspective, which can be different from the name of any implementing method/function. The `code.function` attribute may be used to store the latter (e.g., method actually executing the call on the server side, RPC client stub method on the client side).

**[7]:** This is the logical name of the service from the RPC interface perspective, which can be different from the name of any implementing class. The `code.namespace` attribute may be used to store the latter (despite the attribute name, it may include a class name; e.g., class with method actually executing the call on the server side, RPC client stub class on the client side).
**[7]:** The request body size represents the serialized size of the message payload that is sent over the wire. In case of streaming calls, it accounts for the original message in the request and does not include size of the consequent message stream. (including unary and streaming calls).

**[8]:** The response body size represents the serialized size of the message payload that is received in the response over the wire. In case of streaming calls, it accounts for the first response payload and does not include size of the consequent message stream. (including unary and streaming calls).

**[9]:** This is the logical name of the service from the RPC interface perspective, which can be different from the name of any implementing class. The `code.namespace` attribute may be used to store the latter (despite the attribute name, it may include a class name; e.g., class with method actually executing the call on the server side, RPC client stub class on the client side).

`rpc.connect_rpc.error_code` has the following list of well-known values. If one of them applies, then the respective value MUST be used; otherwise, a custom value MAY be used.

Expand Down
14 changes: 10 additions & 4 deletions docs/rpc/grpc.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,18 @@ Below is a table of attributes that SHOULD be included on client and server gRPC
| Attribute | Type | Description | Examples | [Requirement Level](https://opentelemetry.io/docs/specs/semconv/general/attribute-requirement-level/) | Stability |
|---|---|---|---|---|---|
| [`rpc.grpc.status_code`](/docs/attributes-registry/rpc.md) | int | The [numeric status code](https://github.com/grpc/grpc/blob/v1.33.2/doc/statuscodes.md) of the gRPC request. | `0`; `1`; `2` | `Required` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.grpc.request.metadata.<key>`](/docs/attributes-registry/rpc.md) | string[] | gRPC request metadata, `<key>` being the normalized gRPC Metadata key (lowercase), the value being the metadata values. [1] | `rpc.grpc.request.metadata.my-custom-metadata-attribute=["1.2.3.4", "1.2.3.5"]` | `Opt-In` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.grpc.response.metadata.<key>`](/docs/attributes-registry/rpc.md) | string[] | gRPC response metadata, `<key>` being the normalized gRPC Metadata key (lowercase), the value being the metadata values. [2] | `rpc.grpc.response.metadata.my-custom-metadata-attribute=["attribute_value"]` | `Opt-In` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.request.body.size`](/docs/attributes-registry/rpc.md) | int | The size of the request payload body in bytes. [1] | `3495` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.response.body.size`](/docs/attributes-registry/rpc.md) | int | The size of the response payload body in bytes. [2] | `3495` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.grpc.request.metadata.<key>`](/docs/attributes-registry/rpc.md) | string[] | gRPC request metadata, `<key>` being the normalized gRPC Metadata key (lowercase), the value being the metadata values. [3] | `rpc.grpc.request.metadata.my-custom-metadata-attribute=["1.2.3.4", "1.2.3.5"]` | `Opt-In` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.grpc.response.metadata.<key>`](/docs/attributes-registry/rpc.md) | string[] | gRPC response metadata, `<key>` being the normalized gRPC Metadata key (lowercase), the value being the metadata values. [4] | `rpc.grpc.response.metadata.my-custom-metadata-attribute=["attribute_value"]` | `Opt-In` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |

**[1]:** Instrumentations SHOULD require an explicit configuration of which metadata values are to be captured. Including all request metadata values can be a security risk - explicit configuration helps avoid leaking sensitive information.
**[1]:** The request body size represents the serialized size of the message payload that is sent over the wire. In case of streaming calls, it accounts for the original message in the request and does not include size of the consequent message stream. This value can usually be obtained from protobuf API such as [AbstractMessage#getSerializedSize](https://protobuf.dev/reference/java/api-docs/com/google/protobuf/AbstractMessage.html#getSerializedSize--).
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious what gRPC team (summoning @yashykt) think about this.

I know we avoided having this value as a metric because of possible misinterpretation of behavior between streaming/unary, but having this only used for Span generation may be ok.

Is it possible that users could try to aggregate over this value and lose valuable o11y because of it? Should we have a "is_streaming" or some such flag on Spans so we can make sure we don't aggregate things and make false assumptions?

Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming we record metrics, we'd need to record direction as a part of the metric name - something like rpc.sent.message.size, rpc.received.message.size (histograms).

We could use similar naming (rpc.sent.message.size, rpc.received.message.size ) for these attributes and replace existing ones (rpc.message.*_size) populated on the events.

Unary calls then could be recorded without events - there is nothing(?) useful on the unary messages besides size anyway (direction is now part of the name and index is always 0). Having everything on one span would be easy and convenient for the simple case.

Streaming calls would not have size attributes on spans - they would always be reported on events to avoid potential ambiguity/confusion.

Would this work? @jsuereth @crossoverJie @yashykt

Copy link
Contributor

@yashykt yashykt Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The wording here that says "message payload that is sent over the wire" is kinda confusing because after a payload is serialized, we could compress it, translate it to HTTP2 using https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md and then encrypt it as well.

The part where it says "In case of streaming calls, it accounts for the original message in the request and does not include size of the consequent message stream." also seems like a big restriction.

I'm slightly confused whether this is meant for metrics or tracing, but I'll assume that this is for tracing. Refer to https://github.com/grpc/proposal/blob/master/A72-open-telemetry-tracing.md, for the gRPC team's design. Essentially, the gRPC team decided to always use events for recording message sizes irrespective of whether we are using unary or streaming. This keeps things sane and as a side-benefit, if we can record separate events for the compressed and uncompressed size, this also allows us to gain some insight on when a message was compressed/decompressed and how much time that took.


**[2]:** Instrumentations SHOULD require an explicit configuration of which metadata values are to be captured. Including all response metadata values can be a security risk - explicit configuration helps avoid leaking sensitive information.
**[2]:** The response body size represents the serialized size of the message payload that is received in the response over the wire. In case of streaming calls, it accounts for the first response payload and does not include size of the consequent message stream. This value can usually be obtained from protobuf API such as [AbstractMessage#getSerializedSize](https://protobuf.dev/reference/java/api-docs/com/google/protobuf/AbstractMessage.html#getSerializedSize--).

**[3]:** Instrumentations SHOULD require an explicit configuration of which metadata values are to be captured. Including all request metadata values can be a security risk - explicit configuration helps avoid leaking sensitive information.

**[4]:** Instrumentations SHOULD require an explicit configuration of which metadata values are to be captured. Including all response metadata values can be a security risk - explicit configuration helps avoid leaking sensitive information.



Expand Down
20 changes: 16 additions & 4 deletions docs/rpc/rpc-spans.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ Generally, a user SHOULD NOT set `peer.service` to a fully qualified RPC service
| [`network.transport`](/docs/attributes-registry/network.md) | string | [OSI transport layer](https://osi-model.com/transport-layer/) or [inter-process communication method](https://wikipedia.org/wiki/Inter-process_communication). [4] | `tcp`; `udp` | `Recommended` | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |
| [`network.type`](/docs/attributes-registry/network.md) | string | [OSI network layer](https://osi-model.com/network-layer/) or non-OSI equivalent. [5] | `ipv4`; `ipv6` | `Recommended` | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |
| [`rpc.method`](/docs/attributes-registry/rpc.md) | string | The name of the (logical) method being called, must be equal to the $method part in the span name. [6] | `exampleMethod` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.service`](/docs/attributes-registry/rpc.md) | string | The full (logical) name of the service being called, including its package name, if applicable. [7] | `myservice.EchoService` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.request.body.size`](/docs/attributes-registry/rpc.md) | int | The size of the request payload body in bytes. [7] | `3495` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.response.body.size`](/docs/attributes-registry/rpc.md) | int | The size of the response payload body in bytes. [8] | `3495` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.service`](/docs/attributes-registry/rpc.md) | string | The full (logical) name of the service being called, including its package name, if applicable. [9] | `myservice.EchoService` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |

**[1]:** May contain server IP address, DNS name, or local socket name. When host component is an IP address, instrumentations SHOULD NOT do a reverse proxy lookup to obtain DNS name and SHOULD set `server.address` to the IP address provided in the host component.

Expand All @@ -129,7 +131,11 @@ different processes could be listening on TCP port 12345 and UDP port 12345.

**[6]:** This is the logical name of the method from the RPC interface perspective, which can be different from the name of any implementing method/function. The `code.function` attribute may be used to store the latter (e.g., method actually executing the call on the server side, RPC client stub method on the client side).

**[7]:** This is the logical name of the service from the RPC interface perspective, which can be different from the name of any implementing class. The `code.namespace` attribute may be used to store the latter (despite the attribute name, it may include a class name; e.g., class with method actually executing the call on the server side, RPC client stub class on the client side).
**[7]:** The request body size represents the serialized size of the message payload that is sent over the wire. In case of streaming calls, it accounts for the original message in the request and does not include size of the consequent message stream. (including unary and streaming calls).

**[8]:** The response body size represents the serialized size of the message payload that is received in the response over the wire. In case of streaming calls, it accounts for the first response payload and does not include size of the consequent message stream. (including unary and streaming calls).

**[9]:** This is the logical name of the service from the RPC interface perspective, which can be different from the name of any implementing class. The `code.namespace` attribute may be used to store the latter (despite the attribute name, it may include a class name; e.g., class with method actually executing the call on the server side, RPC client stub class on the client side).



Expand Down Expand Up @@ -190,7 +196,9 @@ different processes could be listening on TCP port 12345 and UDP port 12345.
| [`network.transport`](/docs/attributes-registry/network.md) | string | [OSI transport layer](https://osi-model.com/transport-layer/) or [inter-process communication method](https://wikipedia.org/wiki/Inter-process_communication). [6] | `tcp`; `udp` | `Recommended` | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |
| [`network.type`](/docs/attributes-registry/network.md) | string | [OSI network layer](https://osi-model.com/network-layer/) or non-OSI equivalent. [7] | `ipv4`; `ipv6` | `Recommended` | ![Stable](https://img.shields.io/badge/-stable-lightgreen) |
| [`rpc.method`](/docs/attributes-registry/rpc.md) | string | The name of the (logical) method being called, must be equal to the $method part in the span name. [8] | `exampleMethod` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.service`](/docs/attributes-registry/rpc.md) | string | The full (logical) name of the service being called, including its package name, if applicable. [9] | `myservice.EchoService` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.request.body.size`](/docs/attributes-registry/rpc.md) | int | The size of the request payload body in bytes. [9] | `3495` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.response.body.size`](/docs/attributes-registry/rpc.md) | int | The size of the response payload body in bytes. [10] | `3495` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |
| [`rpc.service`](/docs/attributes-registry/rpc.md) | string | The full (logical) name of the service being called, including its package name, if applicable. [11] | `myservice.EchoService` | `Recommended` | ![Experimental](https://img.shields.io/badge/-experimental-blue) |

**[1]:** May contain server IP address, DNS name, or local socket name. When host component is an IP address, instrumentations SHOULD NOT do a reverse proxy lookup to obtain DNS name and SHOULD set `server.address` to the IP address provided in the host component.

Expand All @@ -212,7 +220,11 @@ different processes could be listening on TCP port 12345 and UDP port 12345.

**[8]:** This is the logical name of the method from the RPC interface perspective, which can be different from the name of any implementing method/function. The `code.function` attribute may be used to store the latter (e.g., method actually executing the call on the server side, RPC client stub method on the client side).

**[9]:** This is the logical name of the service from the RPC interface perspective, which can be different from the name of any implementing class. The `code.namespace` attribute may be used to store the latter (despite the attribute name, it may include a class name; e.g., class with method actually executing the call on the server side, RPC client stub class on the client side).
**[9]:** The request body size represents the serialized size of the message payload that is sent over the wire. In case of streaming calls, it accounts for the original message in the request and does not include size of the consequent message stream. (including unary and streaming calls).

**[10]:** The response body size represents the serialized size of the message payload that is received in the response over the wire. In case of streaming calls, it accounts for the first response payload and does not include size of the consequent message stream. (including unary and streaming calls).

**[11]:** This is the logical name of the service from the RPC interface perspective, which can be different from the name of any implementing class. The `code.namespace` attribute may be used to store the latter (despite the attribute name, it may include a class name; e.g., class with method actually executing the call on the server side, RPC client stub class on the client side).



Expand Down
Loading
Loading