Return tracing header in Rack/Rails HTTP response to support passing trace information via JS apps #16
Conversation
…e information via JS apps
LarsFronius
changed the title
|
Thank you for the contribution. You are correct the trace header should be returned on the response header by default. This is a miss on Ruby SDK. The standard of returning trace header is actually only returning the trace id portion. Parent id is not necessary and the SDK should not expose sampling rate to external caller. See the Go SDK implementation https://github.com/aws/aws-xray-sdk-go/blob/master/xray/handler.go#L134-L138 |
|
Incorporated your feedback in a subsequent commit @haotianw465 👍 |
lib/aws-xray-sdk/facets/rack.rb
Outdated
| @@ -49,6 +49,8 @@ def call(env) | |||
| resp_meta[:content_length] = len | |||
| end | |||
| segment.merge_http_response response: resp_meta | |||
| trace_header = {TRACE_HEADER => TraceHeader.from_entity(entity: XRay.recorder.current_segment).root_string} | |||
There was a problem hiding this comment.
Nitpick. Why passing XRay.recorder.current_segment instead of segment?
There was a problem hiding this comment.
I think I have written XRay.recorder.current_segment too many times to retrieve the current segment, that I didn't realize I actually have a segment object available in this scope. 🤔 Muscle memory, haha. Good catch and corrected! 🙂
In our case, we have a Rails application, that the user has the first interaction with. This is where the trace is started. From here a Javascript app in the browser takes over any further interaction and calls different API subsystems. In order to pass the tracing information to these API subsystems and continue the original trace, this PR introduces responding with the
X-Amzn-Trace-Idheader in the Rack/Rails response.I had noticed that e.g. https://github.com/aws/aws-xray-sdk-go is responding with the
X-Amzn-Trace-Idheader by default once the http middleware is configured, which makes my think it's wanted default behaviour.Looking forward to hearing your feedback about whether this being the right approach. I am also wondering if it's worth making this configurable in any way or can be default behaviour as observed in the Go SDK.
Best, Lars
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.