opentelemetry-ruby-contrib icon indicating copy to clipboard operation
opentelemetry-ruby-contrib copied to clipboard

feat: add public_endpoint option to rack middleware

Open dmathieu opened this issue 3 years ago • 3 comments

Public endpoints may receive requests that include an external traceparent header, making the SDK believe there is a parent span, when that span will never be found in the system, since it was emitted by whatever client made the request.

Example: A public API, where the client is using tracing as well, and therefore sends trace headers with every request.

This fix proposal is similar to what the Go SDK is doing.

It introduces a new public_endpoint option for the rack instrumentation, which is a lambda that takes two arguments: env from rack, and the context we just computed.

In our case for example, we inject some specific key/value to tracestate to identify that a request is coming from our internal systems.

dmathieu avatar Sep 28 '22 13:09 dmathieu

@dmathieu Thank you for your contribution.

We briefly discussed this PR at the Ruby SIG 2022-10-11, if you have a moment please watch the discussion here: https://youtu.be/T20fKAUgeMM?t=2766

I think that @mwear @robertlaurin had the most insight related to the use of tracestate and how this should require some additional discussion with the specification working group/SIG. I will open the floor to them to add any additional thoughts or concerns about adding this functionality to the instrumentation.

arielvalentin avatar Oct 13 '22 03:10 arielvalentin

To summarize the discussion (briefly) I believe that the intended way to solve the problem you are running into is to use the tracestate header. Essentially, you would add an entry to tracestate when a trace enters your system. You can store whatever you want there (although size is a concern to be aware of), but for practical purposes, you'd want to store at least the current span id. When your system receives a request, you'd look at both the traceparent and your tracestate entry to make a decision on how you want to report the parent span id. If the parent id from tracestate matches the parent id in traceparent, you know this is a continuation of a trace that started in your system. If you don't have an entry in tracestate, or the parent ids do not match, this trace (likely) came from outside of your system and you can decide how to record the span for your purposes. I don't know that this scenario, or its interaction with auto-instrumentation has been discussed at the specification level. I'll bring it up at the spec meeting tomorrow and report back.

mwear avatar Oct 17 '22 21:10 mwear

I brought this up at the spec SIG earlier today. Everyone was in agreement that trust boundaries for trace context need to be established at the Spec level. There was an action item for the TC to come up with some proposals. The tracestate solution I mentioned did not seem very popular. I'll keep you updated on the progress as things evolve.

mwear avatar Oct 18 '22 18:10 mwear

👋 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

github-actions[bot] avatar Apr 27 '23 01:04 github-actions[bot]