feat: support context propagation through links

[gwuah]

This pull request extends the rack instrumentation library to support context propagation through links.
Several libraries have this feature, for example que and otel-http lib.

Our use case is simple, we want to connect our frontend & backend traces using links, as opposed to having the frontend be the source of the root span, as that's messing with our sampling. We have had our fork running in production for a while now, just thought to upstream it.

I'm basically brute-forcing my way through ruby, so any sort of style suggestion is welcome.

[xuan-cao-swi]

I think it should be ok as it is an option and the rationale behind it seems logical to me.

However, I do have a few formatting suggestions, which you may consider or ignore as you see fit.

if propagate_with_link&.call(request.env)
  links = prepare_span_links(parent_context)
  span = create_root_span(request, links)
else
  span = create_span(parent_context, request)
end

Additionally, I'm unsure whether option :propagate_with_link should be a callable or rather as an array of links (i.e. array of link is more restricted on options in my opinion).

[arielvalentin]

Although we already support it with URL Quantization, I am weary of adding additional extension points using code block options.

If we moved forward with this feature, I think I would prefer if this code were implemented with additional safety measures and potentially have some fallback behavior to use the parent context.

IIRC there is supposed to be something related to the Trace State that should include information about whether or not to continue a trace or start a new one, however I am not familiar with supporting Span Links for Server Span Kinds.

Is there something in the specification that states that server span kinds should support links?

[github-actions]

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot

[kaylareopelle]

@gwuah, what do you think about Ariel's comment?

[gwuah]

have some fallback behavior to use the parent context.

@arielvalentin This is already the case, unless i'm mistaken. If you decide not to propagate using links, the parent context will be passed to the root span.

Is there something in the specification that states that server span kinds should support links?

@arielvalentin Not that I'm aware of, but this is a common use-case, as demonstrated in other libraries linked in PR description.

Additionally, I'm unsure whether option :propagate_with_link should be a callable or rather as an array of links (i.e. array of link is more restricted on options in my opinion).

@xuan-cao-swi thx for the style suggestion, i'll amend it. also, it's a callable because we want to dynamically enable/disable this feature, based on stuff in headers, etc.

[dmathieu]

Related suggestion that would have offered the same feature, and has a similar implementation in other SDKs (Go and JS, maybe others):

#130

[arielvalentin]

I have asked about Parent/Child and Link propagation decisions in Slack¹ it does not seem like there is a definitive answer, however I was pointed at this issue, which I have not had time to review yet:

open-telemetry/opentelemetry-specification#3867

Related suggestion that would have offered the same feature, and has a similar implementation in other SDKs (Go and JS, maybe others):

Let me reach out in maintainers slack to get feedback from other language maintainers.

Footnotes

1. https://cloud-native.slack.com/archives/C01N7PP1THC/p1713356797068459 ↩

[kaylareopelle]

Thanks, @arielvalentin for bringing this to the maintainers slack channel. What was the conclusion/next steps drawn from your conversation?

[arielvalentin]

Thanks, @arielvalentin for bringing this to the maintainers slack channel. What was the conclusion/next steps drawn from your conversation?

I got very limited engagement on the topic with links to existing issues, which to be honest I have not read yet:

open-telemetry/opentelemetry-specification#3877
open-telemetry/opentelemetry-specification#3867

Something that is required by Golang library instrumentations is that users have to register specific endpoints to be traced or not and have more control over what handlers should ignore in the incoming trace headers and start a new trace.

That allows them more granular control of when to consider creating a new trace id as opposed to what something like Rack does where currently have a single Event Handler injected for all requests.

A lot of routing is also handled by the frameworks themselves and tend not to rely on Rack figuring out what needs to process a request. I think that is what makes this more difficult to do for Ruby library instrumentations.

[zacheryph]

Other instrumentation already use propagation_style, is there any reason this could not be done vs a callable proc?

[gwuah]

@zacheryph

it's a callable because we want to dynamically enable/disable this feature, based on stuff in headers, etc.

[github-actions]

👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot