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 a Start Span section to the OT Shim. #2228

Merged
merged 7 commits into from
Jan 5, 2022

Conversation

carlosalberto
Copy link
Contributor

@carlosalberto carlosalberto commented Dec 17, 2021

Addresses an item in #2147

For some reason I had lost this section, so adding it again. Namely, the important information is:

  • When specifying Span references, use the first one as parent, and all of them as Links (this is what the OT Shims do). The reference type is added as a opentracing.ref_type attribute on the Link itself.
  • Add a small Span References section, to clarify that the OT ChildOf and FollowsFrom reference types must not be confused with OTel Links.
  • Tags/attributes specified at Span creation MUST be set before the Span is actually created, in case they are passed to some internal API, e.g. the Sampling API. See this Java issue.

Please review @jmacd @yurishkuro

@carlosalberto
Copy link
Contributor Author

Hey @yurishkuro - updated the text to mention that:

  1. From the specified references, the first value with type CHILD_OF is the parent, and if no value with this type exists, use the first value. The remaining values are used as Links.
  2. The values added as Links will have a new attribute (part of the Link itself), i.e. opentracing.ref_type=child_of|follows_from. The parent does not keep any attribute - should we add one for that case too?

Let me know whether this aligns with what you have in mind.

(Semantic convention generation is not working for whatever reason, will figure that out)

Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
@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 Dec 26, 2021
@yurishkuro
Copy link
Member

keep

@yurishkuro yurishkuro removed the Stale label Dec 26, 2021
@github-actions
Copy link

github-actions bot commented Jan 3, 2022

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

@jmacd
Copy link
Contributor

jmacd commented Jan 4, 2022

The reference type itself is discarded (we MAY use it later on, if we define equivalent semantics).

I take it this refers to the context that is taken for the parent? Sounds like the shim StartSpan() could be extended to select the OTel SpanKind but we probably won't. 👍

@carlosalberto
Copy link
Contributor Author

@jmacd Actually that's an outdated part of the description (fixed now), as now we keep the reference type as a Link attribute (opentracing.ref_type).

Sounds like the shim StartSpan() could be extended to select the OTel SpanKind but we probably won't.

I think that, as the semantics are not strictly equivalent, we would have to spend some cycles to define this. Let me know if you think it's important to consider this.

@jmacd
Copy link
Contributor

jmacd commented Jan 5, 2022

Let me know if you think it's important to consider this.

I do not! 😀

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

Successfully merging this pull request may close these issues.

4 participants