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

Add span links for HTTP retries and redirects #2393

Conversation

denisivan0v
Copy link
Contributor

@denisivan0v denisivan0v commented Mar 2, 2022

Changes

This PR brings back the part that was temporary excluded from #2078 for semantic conventions for HTTP retries and redirects and defines a linking of corresponding spans. The corresponding discussion is here: #2078 (comment).

Usage of span links makes it possible to correlate spans created within retries or redirects scenarios. For example, in this scenario with HTTP request being made to same set of backends from several threads

request (SERVER, trace=t1, span=s1)
  |
  -- Host: example1.com
  |  GET / - 200 (CLIENT, trace=t1, span=s2)
  |  |
  |   --- server (SERVER, trace=t1, span=s3)
  |
  -- Host: example2.com
  |  GET / - 500 (CLIENT, trace=t1, span=s4)
  |   |
  |   --- server (SERVER, trace=t1, span=s5)
  |
  -- Host: example2.com
  |  GET / - 500 (CLIENT, trace=t1, span=s6, http.retry_count=1)
  |   |
  |   --- server (SERVER, trace=t1, span=s7)
  |
  -- Host: example2.com
  |  GET / - 500 (CLIENT, trace=t1, span=s8)
  |   |
  |   --- server (SERVER, trace=t1, span=s9)
  |
  -- Host: example3.com
  |  GET / - 200 (CLIENT, trace=t1, span=s10)
  |   
  |   --- server (SERVER, trace=t1, span=s11)
  |
  -- Host: example2.com
  |  GET / - 200 (CLIENT, trace=t1, span=s12, http.retry_count=2)
  |   |
  |   --- server (SERVER, trace=t1, span=s13)
  |
  -- Host: example2.com
     GET / - 200 (CLIENT, trace=t1, span=s14)
      |
      --- server (SERVER, trace=t1, span=s15)

the only way to understand the relationship between spans (so, to reconstruct the trace) is to use links:

request (SERVER, trace=t1, span=s1)
  |
  -- Host: example1.com
  |  GET / - 200 (CLIENT, trace=t1, span=s2)
  |  |
  |   --- server (SERVER, trace=t1, span=s3)
  |
  -- Host: example2.com
  |  GET / - 500 (CLIENT, trace=t1, span=s4)
  |   |
  |   --- server (SERVER, trace=t1, span=s5)
  |
  -- Host: example2.com
  |  GET / - 500 (CLIENT, trace=t1, span=s6, links=[ {trace=t1, span=s4} ], http.retry_count=1)
  |   |
  |   --- server (SERVER, trace=t1, span=s7)
  |
  -- Host: example2.com
  |  GET / - 500 (CLIENT, trace=t1, span=s8)
  |   |
  |   --- server (SERVER, trace=t1, span=s9)
  |
  -- Host: example3.com
  |  GET / - 200 (CLIENT, trace=t1, span=s10)
  |   |
  |   --- server (SERVER, trace=t1, span=s11)
  |
  -- Host: example2.com
  |  GET / - 200 (CLIENT, trace=t1, span=s12, links=[ {trace=t1, span=s6} ], http.retry_count=2)
  |   |
  |   --- server (SERVER, trace=t1, span=s13)
  |
  -- Host: example2.com
     GET / - 200 (CLIENT, trace=t1, span=s14)
      |
      --- server (SERVER, trace=t1, span=s15)

This change addresses a scenario which is in the scope for bringing the existing HTTP semantic conventions for tracing to an initial stable state, see related otep #174.

Related: #2278

/cc @bogdandrutu, @tedsuo, @open-telemetry/specs-trace-approvers

--- server (SERVER, trace=t1, span=s7)
```

Example of retries with no trace started upfront:
Copy link
Member

Choose a reason for hiding this comment

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

When there is no incoming trace, or existing trace around the retry calls, how does a retry request reference back to the previous span?

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

This is a new model which was not initially thought about to use links to connect "siblings" in case of a possible "no parent present".

I would like to discuss/document/agree on:

  1. What is the instrumentation strategy that we use? Do we write a "defensive" instrumentation? Means that we over "connect" spans for the possible case when a parent "trace" does not exists?
    • Personal opinion is to not start this path.
  2. Do we need to document the relationship between the "linked" spans?

I feel all these questions need to be discussed/documented before this PR and #2278 can be merged.

@github-actions
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Mar 11, 2022
@denisivan0v denisivan0v marked this pull request as ready for review March 15, 2022 23:18
@denisivan0v denisivan0v requested review from a team as code owners March 15, 2022 23:18
@arminru arminru added area:semantic-conventions Related to semantic conventions spec:trace Related to the specification/trace directory semconv:HTTP labels Mar 16, 2022
@github-actions
Copy link

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions semconv:HTTP spec:trace Related to the specification/trace directory Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants