Missing trace context http headers with faraday

[eric-christian]

Describe the bug

Hello

We noticed that the trace context is not forwarded to other services. The requests are traced, there is just no connection between the services. We use the auto generated OpenAPI-Client https://github.com/OpenAPITools/openapi-generator with faraday. After some digging i figured out that OpenAPI-Client sets the headers like so request.headers = open_api_headers on the yielded request object. Setting headers like this in faraday removes all other already set headers. Since the faraday spy sets the trace context headers before it yields the request object, the trace context headers will be removed.

Steps to reproduce

  require 'faraday'
  require 'elastic-apm'

  ElasticAPM.start
  Faraday.get('https://github.com') do |request|
    pp request.headers # {"User-Agent"=>"Faraday v1.10.0", "Traceparent"=>"00-b30d7f776d439bd21ad9368468d49f6e-2dcd9612313db7aa-01", "Tracestate"=>"es=s:1.0", "Elastic-Apm-Traceparent"=>  "00-b30d7f776d439bd21ad9368468d49f6e-2dcd9612313db7aa-01"}
    req.headers = { my_header: 'something' }
    pp request.headers # {"My-Header"=>"Something"}
  end

Expected behavior

I expect the trace context headers to be send. Even if the business logic sets its own headers in a kinda harsh way.

And maybe the yielded request object shouldn't contain them already, because the code block is most likely business logic and the headers might interfere with the business logic. Especially if the tracing is disabled during development and testing and on preview / production there are suddenly more headers. 🤷 Big maybe. Depends of how stealthy the spy should work. Maybe it doesn't matter. 🤷

Environment

• OS: OS X 12.2.1
• Ruby version: ruby 2.7.3p183 (2021-04-05 revision 6847ee089d) [arm64-darwin20]
• Framework and version: rails 6.1.4.7, faraday 1.10.0
• Agent version: elastic-apm 4.5.0

Additional context

I came up with two possible solutions.

1. Simply make the apply_headers after the yield. https://github.com/elastic/apm-agent-ruby/blob/main/lib/elastic_apm/spies/faraday.rb#L107

              result = super(method, url, body, headers) do |req|
                yield req if block
                trace_context.apply_headers { |k, v| req[k] = v }
              end

2. Move the spy to Faraday::RackBuilder.build_response. I just did a small prove of concept and i have no idea if this introduces other side effects or if there are cases where this method does not get called. But so far my pipelines are all green and i see the connections between the services. Also my POC assumed that we are within a span, because i did not remove the original faraday spy. This would probably also fix the problem with the possible site effects caused by yielding the request object twice. Because the url should be present here.

# frozen_string_literal: true

require 'elastic-apm'
require 'faraday'

module ElasticAPMFaradayPatch
  module Ext
    def build_response(connection, request)
      unless (span = ElasticAPM.current_span)
        return super
      end
      return super if ElasticAPM::Spies::FaradaySpy.disabled?

      span.trace_context.apply_headers { |k, v| request[k] = v }
      super(connection, request)
    end
  end

  ::Faraday::RackBuilder.prepend(Ext)
end

Have a great day! 😃

[estolfo]

Hi @eric-christian Thanks for opening this issue and for the detailed investigation! We'll review and have an update soon.