-
Notifications
You must be signed in to change notification settings - Fork 872
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
Add yaml semantic conventions for RPC #925
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
groups: | ||
- id: rpc | ||
prefix: rpc | ||
brief: 'This document defines semantic conventions for remote procedure calls.' | ||
attributes: | ||
- id: system | ||
type: string | ||
required: always | ||
brief: 'A string identifying the remoting system.' | ||
examples: ["grpc", "java_rmi", "wcf"] | ||
- id: service | ||
type: string | ||
required: | ||
conditional: "No, but recommended" | ||
brief: 'The full name of the service being called, including its package name, if applicable.' | ||
examples: "myservice.EchoService" | ||
- id: method | ||
type: string | ||
required: | ||
conditional: "No, but recommended" | ||
brief: 'The name of the method being called, must be equal to the $method part in the span name.' | ||
examples: "exampleMethod" | ||
- ref: net.peer.ip | ||
- ref: net.peer.name | ||
- ref: net.peer.port | ||
required: | ||
conditional: "See below" | ||
- ref: net.transport | ||
required: | ||
conditional: "See below" | ||
constraints: | ||
- any_of: | ||
- net.peer.ip | ||
- net.peer.name | ||
- include: network |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,17 +48,23 @@ Examples of span names: | |
|
||
### Attributes | ||
|
||
| Attribute name | Notes and examples | Required? | | ||
| -------------- | ---------------------------------------------------------------------- | --------- | | ||
| `rpc.system` | A string identifying the remoting system, e.g., `"grpc"`, `"java_rmi"` or `"wcf"`. | Yes | | ||
| `rpc.service` | The full name of the service being called, including its package name, if applicable. | No, but recommended | | ||
| `rpc.method` | The name of the method being called, must be equal to the $method part in the span name. | No, but recommended | | ||
| `net.peer.ip` | See [network attributes][]. | See below | | ||
| `net.peer.name` | See [network attributes][]. | See below | | ||
| `net.peer.port` | See [network attributes][]. | See below | | ||
| `net.transport` | See [network attributes][]. | See below | | ||
|
||
At least one of [network attributes][] `net.peer.name` or `net.peer.ip` is required. | ||
<!-- semconv rpc --> | ||
| Attribute | Type | Description | Example | Required | | ||
|---|---|---|---|---| | ||
| `rpc.system` | string | A string identifying the remoting system. | `grpc`<br>`java_rmi`<br>`wcf` | Yes | | ||
| `rpc.service` | string | The full name of the service being called, including its package name, if applicable. | `myservice.EchoService` | Conditional<br>No, but recommended | | ||
| `rpc.method` | string | The name of the method being called, must be equal to the $method part in the span name. | `exampleMethod` | Conditional<br>No, but recommended | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "Conditional. No" So it's not conditionally required? That sounds a bit odd. Maybe the tool should just not emit "conditional" but just put the note in? Or there should be an additional level between not required and required named "recommended". Or this should be just not required. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also noticed this problem. Imho the tool should be updated to only emit the specified text without the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds also fine, although it might lead to mis-use. Like "Required: Yes, because bla" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to what @Oberon00 said. I am not sure how to read the "Conditional No, but recommended" phrase. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I filed open-telemetry/build-tools#7 for having this fixed in the MD generator. @thisthat will work on this soon so I would be fine with merging this PR and re-generate the updated markdown files in a follow-up. |
||
| [`net.peer.ip`](span-general.md) | string | Remote address of the peer (dotted decimal for IPv4 or [RFC5952](https://tools.ietf.org/html/rfc5952) for IPv6) | `127.0.0.1` | See below | | ||
| [`net.peer.name`](span-general.md) | string | Remote hostname or similar, see note below. | `example.com` | See below | | ||
| [`net.peer.port`](span-general.md) | number | Remote port number. | `80`<br>`8080`<br>`443` | Conditional<br>See below | | ||
| [`net.transport`](span-general.md) | string enum | Transport protocol used. See note below. | `IP.TCP` | Conditional<br>See below | | ||
|
||
**Additional attribute requirements:** At least one of the following sets of attributes is required: | ||
|
||
* [`net.peer.ip`](span-general.md) | ||
* [`net.peer.name`](span-general.md) | ||
<!-- endsemconv --> | ||
|
||
For client-side spans `net.peer.port` is required if the connection is IP-based and the port is available (it describes the server port they are connecting to). | ||
For server-side spans `net.peer.port` is optional (it describes the port the client is connecting from). | ||
Furthermore, setting [net.transport][] is required for non-IP connection like named pipe bindings. | ||
|
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.
I think the examples should be quoted. Also I don't know if we want to use
<br>
by default. In this case,
would be enough. So I think ideally this should look likebut just adding the quotes would also be good.