Add Azure SDK semantic conventions #2233
Conversation
lmolkova
force-pushed
the
azure-sdk-tracing-conventions
branch
2 times, most recently
from
3ed4a27
to
d7ae2a3
Compare
arminru
added
area:semantic-conventions
lmolkova
force-pushed
the
azure-sdk-tracing-conventions
branch
from
d7ae2a3
to
28a1f0f
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
lmolkova
force-pushed
the
azure-sdk-tracing-conventions
branch
2 times, most recently
from
b019d45
to
0008ead
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@open-telemetry/specs-trace-approvers can someone please take a look? |
|
@arminru thanks for the review! I think I addressed your comments. |
| # Version: 0.0.0 | ||
| # Status: Experimental |
There was a problem hiding this comment.
Are those metadata fields used by internal tooling or intended to be used by the MD and code generator in https://github.com/open-telemetry/build-tools?
There was a problem hiding this comment.
those are only intended for MD generator at this point, they might also be used by Java code generator in the future. The main intent is formal documentation though.
There was a problem hiding this comment.
I think it would make sense to agree on the format and content first and have that specified in the syntax.md for the tool (but it looks good to me in general).
| # Status: Experimental | ||
| # This document describes Azure SDK trace semantic conventions. For more details refer to https://azure.github.io/azure.sdk/distributed_tracing_conventions.html | ||
|
|
||
| groups: |
There was a problem hiding this comment.
All of these define an id but no prefix. I assume you'd want to set prefix to match your ids.
There was a problem hiding this comment.
I'm not aware of any requirement for id and prefix to match. Did I miss something?
AFAIK id is only relevant for tooling, while prefix is attribute prefix.
I don't see anything here and if I check existing specs, some of them have different prefix and id. Moreover, with things like HTTP, we don't add any new prefixes to attributes.
There was a problem hiding this comment.
That is correct. Currently you have request_id and service_request_id as standalone top-level attributes though. I assume those should be put under an azure prefix / namespace (e.g. azure.sdk.http.request_id), or is this intentional?
| - id: peer.address | ||
| type: string | ||
| brief: 'Fully qualified messaging service name.' | ||
| required: always | ||
| examples: ['myEventHubNamespace.servicebus.windows.net'] |
There was a problem hiding this comment.
How about referencing net.peer.name from the general conventions here?
There was a problem hiding this comment.
once we'll switch to net.peer.name I'll use it and add a reference. But this is instrumentation exists for years and it can't be changed overnight.
There was a problem hiding this comment.
Ah so that's not a new suggestion that just borrows from OpenTracing semconvs but it's documenting an existing instrumentation that's already in use?
specification/trace/semantic_conventions/instrumentation/azure-sdk.md
Outdated
Show resolved
Hide resolved
lmolkova
force-pushed
the
azure-sdk-tracing-conventions
branch
from
2bf134f
to
378087d
Compare
| brief: 'Describes Azure SDK API calls that wrap an Azure service call(s).' | ||
|
|
||
| # http | ||
| - id: azure.sdk.http |
There was a problem hiding this comment.
Just confirming if you meant to remove .sdk in all the cases too, assuming a possibility of filling in on the server.
There was a problem hiding this comment.
this specific section describes the client side of things (e.g. populates http.url), if we add server, I assume it would go under azure.<server>.http, so I kept sdk intentionally. Do you see any problem with this?
I assume if we add common attributes between server and client and declare them in azure.http this would still be a backward-compatible change.
There was a problem hiding this comment.
For HTTP, we chose the same names for client and server, since the overlapping attributes (like http.url) do have the same meaning. We use the span kind to distinguish.
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Adds yml spec for Azure SDK trace semantic conventions. This doc only documents the current state, it is not stable and has experimental status.
As Otel conventions stabilize, Azure SDKs will gradually onboard onto them.