spandex-project/spandex

`distributed_context` takes a `Plug.Conn` rather than `headers()`

jfmyers9 opened this issue · 6 comments

Currently there are two methods that Spandex provides to do distributed tracing.

inject_context:

  @spec inject_context(headers(), SpanContext.t(), Tracer.opts()) :: headers()

distributed_context:

  @spec distributed_context(Plug.Conn.t(), Tracer.opts()) ::
          {:ok, SpanContext.t()}
          | {:error, :disabled}

Given that these two methods are intended to be counterparts of each other, would it make more sense if the distributed_context interface was actually:

  @spec distributed_context(headers(), Tracer.opts()) ::
          {:ok, SpanContext.t()}
          | {:error, :disabled}

An example use case would be injecting and extracting tracing contexts from a GRPC request which does not rely on Plug.Conn.

Thoughts?

Probably, yeah. In the interest of backwards compatibility, though, we could just do:

def distributed_context(headers, opts) when is_list(headers)

And just make it accept a conn or headers

Makes sense. This would require Adapters to update their interfaces as well. What would be the expected behavior if a developer used a new version of Spandex with an older version of an Adapter? Would a failure in this situation be ok?

They wouldn't have to update if we accept a list of headers or a conn, and return the same result, right? Everything should be hunky-dory.

Unfortunately at the moment we just forward the Plug.Conn to the adapter. I'm not sure how we would be able to do this without updating the adapters without doing something odd like creating an empty Plug.Conn that contains the headers.

Yeah, you're right. Right now, there is only one supported adapter (unless someone made a custom one). So:
1.) We release the change we discussed here to mainline spandex as a major release
2.) We release the change to spandex_datadog as a major release, and bump the dependency in mix.exs to the new major release in spandex

This should solve for the case that someone updates spandex or spandex_datadog w/o updating the other one. I don't mind doing the releases, if you're interested in PR-ing the changes to the two projects 👍

Sounds like a good plan. I should be able to whip up a PR soon. Thanks for the input.