This repository has been archived by the owner on Apr 2, 2024. It is now read-only.
Preserve Jaeger RefType when storing/retrieving traces. #1681
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alejandrodnm
force-pushed
the
adn/reftype
branch
3 times, most recently
from
961311f
to
5c4ea2f
Compare
arajkumar
approved these changes
Oct 4, 2022
| @@ -14,16 +14,12 @@ import ( | |||
| "fmt" | |||
There was a problem hiding this comment.
IIUC, all jaeger related tests which we own can be removed once we get jaeger storage integration tests up & running right? I feel most of the tests which we own are redundant. WDYT?
There was a problem hiding this comment.
Yeh, I think we need to review all the tests we have, specially for traces. There are some that I modified in the previous PR that counted the results, I changed them to check the values, then found out that there's another suite that runs basically the same tests and compares to some fixtures that are snappy compressed.
There was a problem hiding this comment.
The other point is that some of the tests we have also test the OTEL ingest pipeline. The thing is that the responses to compare against the fixtures are retrieved using the Jaeger endpoint.
alejandrodnm
force-pushed
the
adn/reftype
branch
2 times, most recently
from
418c80c
to
c63737c
Compare
alejandrodnm
force-pushed
the
adn/fix-traces-links-events
branch
from
5c652ac
to
c47f053
Compare
alejandrodnm
force-pushed
the
adn/reftype
branch
from
c63737c
to
db84c70
Compare
alejandrodnm
force-pushed
the
adn/reftype
branch
2 times, most recently
from
7ae0723
to
e8310d3
Compare
niksajakovljevic
approved these changes
Oct 6, 2022
2 tasks
alejandrodnm
force-pushed
the
adn/fix-traces-links-events
branch
2 times, most recently
from
2f55113
to
55f5a05
Compare
Jaeger, like OpenTracing, uses identifies the type of references between Spans. There are only two types of references `CHILD_OF` and `FOLLOWS_FROM`. OTEL doesn't have an analogous to Jaeger reference type in their link specification. An OTEL span can have only one parent, and that's set as an attributed of the span instead of an OTEL link. When using the Jaeger to OTEL translator the first reference with type `CHILD_OF` is used to set the OTEL span parent attribute, all other references are converted to links, and since there is no notion of reference type that information is lost. On the reverse transformation, OTEL to Jaeger, the OTEL Span parent is converted to a reference with `CHILD_OF` type, while all the other OTEL links are converted to `FOLLOWS_FROM` references. The problem is that Jaeger, unlike OTEL, supports the notion of multiple parents (one use case is fork/join workloads), running this multi-parent through the translator and back returns a span with a single parent with all the other parents turned into `FOLLOWS_FROM` references. There's an open PR in the translator to keep this information by adding the type as attribute to the links following the semantic convention for OpenTracing compatibility: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/compatibility/#OpenTracing open-telemetry/opentelemetry-collector-contrib#14463 While that gets merge we are going to be manually setting the reference type as an attribute on the links when writing traces. On the retrieving traces side, we'll be using those attributes to set the corresponding reference type when constructing the Jaeger traces responses.
alejandrodnm
force-pushed
the
adn/reftype
branch
from
e8310d3
to
beea06b
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: This PR is based on #1678 . I'll update the base once #1678 is merged
Jaeger, like OpenTracing, identifies the type of references between Spans. There are only two types of references
CHILD_OFandFOLLOWS_FROM.OTEL doesn't have an analogous to Jaeger reference type in their link specification. An OTEL span can have only one parent, and that's set as an attributed of the span instead of an OTEL link.
When using the Jaeger to OTEL translator the first reference with type
CHILD_OFis used to set the OTEL span parent attribute, all other references are converted to links, and since there is no notion of reference type that information is lost. On the reverse transformation, OTEL to Jaeger, the OTEL Span parent is converted to a reference withCHILD_OFtype, while all the other OTEL links are converted toFOLLOWS_FROMreferences.The problem is that Jaeger, unlike OTEL, supports the notion of multiple parents (one use case is fork/join workloads), running this multi-parent through the translator and back returns a span with a single parent with all the other parents turned into
FOLLOWS_FROMreferences.There's an open PR in the translator to keep this information by adding the type as attribute to the links following the semantic convention for OpenTracing compatibility:
https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/compatibility/#OpenTracing open-telemetry/opentelemetry-collector-contrib#14463
While that gets merge we are going to be manually setting the reference type as an attribute on the links when writing traces. On the retrieving traces side, we'll be using those attributes to set the corresponding reference type when constructing the Jaeger traces responses.
Merge requirements
Please take into account the following non-code changes that you may need to make with your PR: