Add yaml semantic conventions for RPC #925
Conversation
|
#547 is closed. Maybe you want to open a new tracking issue for these? |
| <!-- semconv rpc --> | ||
| | Attribute | Type | Description | Example | Required | | ||
| |---|---|---|---|---| | ||
| | `rpc.system` | string | A string identifying the remoting system. | `grpc`<br>`java_rmi`<br>`wcf` | Yes | |
There was a problem hiding this comment.
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 like
| | `rpc.system` | string | A string identifying the remoting system. | `grpc`<br>`java_rmi`<br>`wcf` | Yes | | |
| | `rpc.system` | string | A string identifying the remoting system. | `"grpc"`, `"java_rmi"`, `"wcf"` | Yes | |
but just adding the quotes would also be good.
| |---|---|---|---|---| | ||
| | `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.
"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.
I also noticed this problem. Imho the tool should be updated to only emit the specified text without the Conditional prefix.
There was a problem hiding this comment.
That sounds also fine, although it might lead to mis-use. Like "Required: Yes, because bla"
There was a problem hiding this comment.
+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.
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.
Oberon00
added
area:semantic-conventions
There was a problem hiding this comment.
LGTM, given that the formatting will be fixed by open-telemetry/build-tools#7
|
@open-telemetry/specs-approvers please have a look |
Changes
Updated RPC semantic convention to the YAML format.
Related issues #547