Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Self Referenced Span #411

Merged
merged 11 commits into from
Aug 7, 2019

Conversation

dm03514
Copy link
Contributor

@dm03514 dm03514 commented Aug 3, 2019

Resolves #397

Which problem is this PR solving?

  • This allows client to initialize a span from an already identified ("offline") context.

Short description of the changes

  • Introduces a Jaeger specific span reference type "self"
  • Allows client to pass in an already established SpanContext, if so, will use the passed in SpanContext for the span instead of generating span/trace ids

TODO

  • remove extra pointer, e.g. store ref.ReferencedContext instead
  • self reference should continue the loop because a span may have multiple happens-before ancestors
  • self reference unit tests
  • needs documentation in the comments and a section in the README
  • move self ref constants.go is a better place

@yurishkuro
Copy link
Member

lgtm, pending changes in the comments

@codecov
Copy link

codecov bot commented Aug 4, 2019

Codecov Report

Merging #411 into master will increase coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
+ Coverage   88.38%   88.42%   +0.03%     
==========================================
  Files          55       55              
  Lines        3092     3102      +10     
==========================================
+ Hits         2733     2743      +10     
  Misses        255      255              
  Partials      104      104
Impacted Files Coverage Δ
tracer.go 97.05% <100%> (+0.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 944042e...0890a2c. Read the comment docs.

refs jaegertracing#397

Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
@dm03514 dm03514 changed the title WIP - POC: Self Referenced Span Self Referenced Span Aug 4, 2019
@dm03514
Copy link
Contributor Author

dm03514 commented Aug 5, 2019

@yurishkuro updated! Thank you. Are there any other test cases that I should add?

tracer.go Outdated Show resolved Hide resolved
tracer_test.go Outdated Show resolved Hide resolved
…nces.

Provides a selfRef factory method hidningn constant/implementation details

Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
README.md Outdated Show resolved Hide resolved
Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
Signed-off-by: Daniel Mican <dm03514@gmail.com>
@dm03514
Copy link
Contributor Author

dm03514 commented Aug 7, 2019

@yurishkuro updated, thank you for all your help with this

@yurishkuro yurishkuro merged commit 201832f into jaegertracing:master Aug 7, 2019
@yurishkuro
Copy link
Member

thanks 🎉

@yurishkuro
Copy link
Member

please let us know how it works out end to end

yurishkuro added a commit that referenced this pull request Feb 17, 2020
@yurishkuro
Copy link
Member

@dm03514 Do you think it's correct that the sampling decision is also inherited from the Self span reference? It makes it difficult to use this mechanism for people who want to provide their own trace IDs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support externally-provided trace/span IDs
2 participants