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.

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

Clarify semantic of SpanKind regarding parent/child relationships #3172

Closed
Tracked by #659
pyohannes opened this issue Feb 2, 2023 · 2 comments · Fixed by #4178
Closed
Tracked by #659

Clarify semantic of SpanKind regarding parent/child relationships #3172

pyohannes opened this issue Feb 2, 2023 · 2 comments · Fixed by #4178
Assignees
Labels
semconv:messaging sig-issue A specific SIG should look into this before discussing at the spec spec:trace Related to the specification/trace directory

Comments

@pyohannes
Copy link
Contributor

What are you trying to achieve?

The specification of SpanKind describes "logical" parent/child relationships between spans:

The first property described by SpanKind reflects whether the Span is a "logical" remote child or parent. By "logical", we mean that the span is logically a remote child or parent, from the point of view of the library that is being instrumented. Spans with a remote parent are interesting because they are sources of external load. Spans with a remote child are interesting because they reflect a non-local system dependency.

It also talks about consumer spans in particular:

CONSUMER Indicates that the span describes a child of an asynchronous PRODUCER request.

The concept of a "logical" parent/child relationships created confusion during discussions in the messaging workgroup, in particular in relation to relationships between producer and consumer spans, which are often modelled as links (e. g. in existing messaging examples regarding batch receiving or batch processing).

The specification should make clear that a "logical" parent/child relationship also applies to linked spans, and ideally use unambiguous terms for describing relationships between spans that can be either parent/child relationships or links.

@pyohannes pyohannes added the spec:trace Related to the specification/trace directory label Feb 2, 2023
@github-project-automation github-project-automation bot moved this to V1 - Stable Semantics in Spec: Messaging Semantics Feb 2, 2023
@blumamir
Copy link
Member

blumamir commented Feb 4, 2023

related isssue: #526

@blumamir
Copy link
Member

blumamir commented Feb 4, 2023

To me, the kind is very useful to understand if a span is incoming or outgoing to the current process. This helps create boundaries for display and provide context to end users, similar to what open-telemetry/oteps#182 is trying to achieve. The use of parent-child-link relationships make it so that messaging spans, network spans, framework spans etc need to sometimes be marked as "internal" because they have no remote children / parent or links, but for end users this is not very helpful.

I think that the operation itself carries some "logical" direction which makes sense to me to go in the kind. So if we open a TCP socket, it might not inject or extract remote context, but it still plays the role of a CLIENT in the conversation. Similarly in messaging, we might not always have the remote context available to go into the parent or the link (redis pubsub, socket.io), but I still think it makes sense to mark spans that describe async messages entering the application as "CONSUMER".

I wonder if the wording can describe something else other than parent-child-link, which relies on the operation semantics and not the trace structure.

One edge case I can think of: for polling of messages from a remote server - "receive" span, it describes both a "client" request to a server to fetch messages and both "consumer" as it receives a batch of remote async messages to consume. In this case, the kind is not well defined.

@pyohannes pyohannes assigned pyohannes and unassigned jmacd May 4, 2023
@dyladan dyladan added the sig-issue A specific SIG should look into this before discussing at the spec label May 21, 2024
lmolkova added a commit that referenced this issue Sep 3, 2024
Fixes #3172

(Built on top of #4088)

## Changes

- Explains kinds without assuming presence of parent/children 
- Adds links as another correlation mechanism
- Normalizes nested client spans even further - database, messaging,
RPC, and LLM semantic conventions require CLIENT kind for logical client
operation.
- Does not touch INTERNAL kind yet -
#4179

* [x] Related issues #3172,
open-telemetry/semantic-conventions#674,
open-telemetry/oteps#172,
open-telemetry/semantic-conventions#1315
* ~~[ ] Related [OTEP(s)](https://github.com/open-telemetry/oteps) #~~
* ~~[ ] Links to the prototypes (when adding or changing features)~~
* [x]
[`CHANGELOG.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CHANGELOG.md)
file updated for non-trivial changes
* ~~[ ]
[`spec-compliance-matrix.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/spec-compliance-matrix.md)
updated if necessary~~

---------

Co-authored-by: Tigran Najaryan <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Co-authored-by: Trask Stalnaker <[email protected]>
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this issue Oct 31, 2024
Fixes open-telemetry#3172

(Built on top of open-telemetry#4088)

## Changes

- Explains kinds without assuming presence of parent/children 
- Adds links as another correlation mechanism
- Normalizes nested client spans even further - database, messaging,
RPC, and LLM semantic conventions require CLIENT kind for logical client
operation.
- Does not touch INTERNAL kind yet -
open-telemetry#4179

* [x] Related issues open-telemetry#3172,
open-telemetry/semantic-conventions#674,
open-telemetry/oteps#172,
open-telemetry/semantic-conventions#1315
* ~~[ ] Related [OTEP(s)](https://github.com/open-telemetry/oteps) #~~
* ~~[ ] Links to the prototypes (when adding or changing features)~~
* [x]
[`CHANGELOG.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/CHANGELOG.md)
file updated for non-trivial changes
* ~~[ ]
[`spec-compliance-matrix.md`](https://github.com/open-telemetry/opentelemetry-specification/blob/main/spec-compliance-matrix.md)
updated if necessary~~

---------

Co-authored-by: Tigran Najaryan <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]>
Co-authored-by: Trask Stalnaker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semconv:messaging sig-issue A specific SIG should look into this before discussing at the spec spec:trace Related to the specification/trace directory
Projects
Status: V1 - Stable Semantics
4 participants