Upgrade to use released version of Opentelemetry and also pushes the span context to gracefully handle multiple routers

config/test.exs

@@ -1,7 +1,7 @@
 import Config
 
 config :opentelemetry,
-  sampler: {:always_on, %{}},
+  # sampler: {:always_on, %{}},
   tracer: :otel_tracer_default,
   processors: [
     otel_batch_processor: %{scheduled_delay_ms: 1, exporter: :undefined}

lib/opentelemetry_plug.ex

@@ -29,7 +29,10 @@ defmodule OpentelemetryPlug do
 
     @impl true
     def call(conn, _opts) do
-      register_before_send(conn, &merge_resp_headers(&1, :otel_propagator.text_map_inject([])))
+      register_before_send(
+        conn,
+        &merge_resp_headers(&1, OpentelemetryPlug.inject_header_context([]))
+      )
     end
   end
 
@@ -43,8 +46,7 @@ defmodule OpentelemetryPlug do
 
   """
   def setup() do
-    # register the tracer. just re-registers if called for multiple repos
-    _ = OpenTelemetry.register_application_tracer(:opentelemetry_plug)
+    :ok = register_tracer()
 
     :telemetry.attach(
       {__MODULE__, :plug_router_start},
@@ -70,9 +72,13 @@ defmodule OpentelemetryPlug do
 
   @doc false
   def handle_start(_, _measurements, %{conn: conn, route: route}, _config) do
-    save_parent_ctx()
-    # setup OpenTelemetry context based on request headers
-    :otel_propagator.text_map_extract(conn.req_headers)
+    parent_ctx = save_parent_ctx()
+
+    if parent_ctx == :undefined do
+      # setup OpenTelemetry context based on request headers, but only if
+      # there's no context already
+      extract_header_context(conn.req_headers)
+    end
 
     span_name = "#{route}"
 
@@ -84,7 +90,7 @@ defmodule OpentelemetryPlug do
 
     attributes =
       [
-        "http.target": conn.request_path,
+        "http.target": http_target(conn),
         "http.host": conn.host,
         "http.scheme": conn.scheme,
         "http.flavor": http_flavor(conn.adapter),
@@ -143,7 +149,7 @@ defmodule OpentelemetryPlug do
   end
 
   defp header_or_empty(conn, header) do
-    case Plug.Conn.get_req_header(conn, header) do
+    case Plug.Conn.get_req_header(conn, String.downcase(header)) do
       [] ->
         ""
 
@@ -152,6 +158,11 @@ defmodule OpentelemetryPlug do
     end
   end
 
+  defp http_target(conn) when conn.query_string == "" or is_nil(conn.query_string),
+    do: conn.request_path
+
+  defp http_target(conn), do: conn.request_path <> "?" <> conn.query_string
+
   defp optional_attributes(conn) do
     ["http.client_ip": &client_ip/1, "http.server_name": &server_name/1]
     |> Enum.map(fn {attr, fun} -> {attr, fun.(conn)} end)
@@ -177,21 +188,73 @@ defmodule OpentelemetryPlug do
       :"HTTP/1.0" -> :"1.0"
       :"HTTP/1.1" -> :"1.1"
       :"HTTP/2.0" -> :"2.0"
+      :"HTTP/2" -> :"2.0"
       :SPDY -> :SPDY
       :QUIC -> :QUIC
-      nil -> ""
+      _ -> ""
     end
   end
 
   @ctx_key {__MODULE__, :parent_ctx}
   defp save_parent_ctx() do
     ctx = Tracer.current_span_ctx()
-    Process.put(@ctx_key, ctx)
+
+    case Process.get(@ctx_key, :undefined) do
+      list when is_list(list) ->
+        Process.put(@ctx_key, [ctx | list])
+
+      :undefined ->
+        Process.put(@ctx_key, [ctx])
+    end
+
+    ctx
   end
 
   defp restore_parent_ctx() do
-    ctx = Process.get(@ctx_key, :undefined)
-    Process.delete(@ctx_key)
+    ctx =
+      case Process.get(@ctx_key, :undefined) do
+        [ctx | rest] ->
+          Process.put(@ctx_key, rest)
+          ctx
+
+        _ ->
+          Process.delete(@ctx_key)
+          :undefined
+      end
+
     Tracer.set_current_span(ctx)
   end
+
+  # OpenTelemetry 1.0.0-rc.3 removed the need for registering application tracers.
+  if Code.ensure_loaded?(OpenTelemetry) do
+    if function_exported?(OpenTelemetry, :register_application_tracer, 1) do
+      def register_tracer do
+        _ = OpenTelemetry.register_application_tracer(:opentelemetry_plug)
+        :ok
+      end
+    else
+      def register_tracer, do: :ok
+    end
+  end
+
+  # OpenTelemetry 1.0.0-rc.3 changed the text_map propagator API
+  if Code.ensure_loaded?(:otel_propagator) do
+    if function_exported?(:otel_propagator, :text_map_extract, 1) do
+      def extract_header_context(headers) do
+        :otel_propagator.text_map_extract(headers)
+      end
+
+      def inject_header_context(headers) do
+        :otel_propagator.text_map_inject(headers)
+      end
+    else
+      def extract_header_context(headers) do
+        :otel_propagator_text_map.extract(headers)
+      end
+
+      def inject_header_context(headers) do
+        :otel_propagator_text_map.inject(headers)
+      end
+    end
+  end
 end

mix.exs

@@ -22,11 +22,10 @@ defmodule OpentelemetryPlug.MixProject do
   defp deps do
     [
       {:hackney, "~> 1.0", only: :test, runtime: false},
-      {:opentelemetry_api, "~> 1.0.0-rc"},
-      {:opentelemetry, "~> 1.0.0-rc", only: :test},
-      {:plug, "~> 1.13"},
-      {:plug_cowboy, "~> 2.5.2", only: :test, runtime: false, override: true},
-      {:telemetry, "~> 1.0"}
+      {:opentelemetry_api, "~> 1.0"},
+      {:opentelemetry, "~> 1.0", only: :test},
+      {:plug, ">= 1.10.1"},
+      {:plug_cowboy, "~> 2.2", only: :test, runtime: false}
     ]
   end
 end

mix.lock

@@ -1,17 +1,17 @@
 %{
-  "certifi": {:hex, :certifi, "2.6.1", "dbab8e5e155a0763eea978c913ca280a6b544bfa115633fa20249c3d396d9493", [:rebar3], [], "hexpm", "524c97b4991b3849dd5c17a631223896272c6b0af446778ba4675a1dff53bb7e"},
+  "certifi": {:hex, :certifi, "2.9.0", "6f2a475689dd47f19fb74334859d460a2dc4e3252a3324bd2111b8f0429e7e21", [:rebar3], [], "hexpm", "266da46bdb06d6c6d35fde799bcb28d36d985d424ad7c08b5bb48f5b5cdd4641"},
   "cowboy": {:hex, :cowboy, "2.9.0", "865dd8b6607e14cf03282e10e934023a1bd8be6f6bacf921a7e2a96d800cd452", [:make, :rebar3], [{:cowlib, "2.11.0", [hex: :cowlib, repo: "hexpm", optional: false]}, {:ranch, "1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "2c729f934b4e1aa149aff882f57c6372c15399a20d54f65c8d67bef583021bde"},
   "cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"},
   "cowlib": {:hex, :cowlib, "2.11.0", "0b9ff9c346629256c42ebe1eeb769a83c6cb771a6ee5960bd110ab0b9b872063", [:make, :rebar3], [], "hexpm", "2b3e9da0b21c4565751a6d4901c20d1b4cc25cbb7fd50d91d2ab6dd287bc86a9"},
-  "hackney": {:hex, :hackney, "1.17.4", "99da4674592504d3fb0cfef0db84c3ba02b4508bae2dff8c0108baa0d6e0977c", [:rebar3], [{:certifi, "~>2.6.1", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~>6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~>1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~>1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "de16ff4996556c8548d512f4dbe22dd58a587bf3332e7fd362430a7ef3986b16"},
+  "hackney": {:hex, :hackney, "1.18.1", "f48bf88f521f2a229fc7bae88cf4f85adc9cd9bcf23b5dc8eb6a1788c662c4f6", [:rebar3], [{:certifi, "~>2.9.0", [hex: :certifi, repo: "hexpm", optional: false]}, {:idna, "~>6.1.0", [hex: :idna, repo: "hexpm", optional: false]}, {:metrics, "~>1.0.0", [hex: :metrics, repo: "hexpm", optional: false]}, {:mimerl, "~>1.1", [hex: :mimerl, repo: "hexpm", optional: false]}, {:parse_trans, "3.3.1", [hex: :parse_trans, repo: "hexpm", optional: false]}, {:ssl_verify_fun, "~>1.1.0", [hex: :ssl_verify_fun, repo: "hexpm", optional: false]}, {:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "a4ecdaff44297e9b5894ae499e9a070ea1888c84afdd1fd9b7b2bc384950128e"},
   "idna": {:hex, :idna, "6.1.1", "8a63070e9f7d0c62eb9d9fcb360a7de382448200fbbd1b106cc96d3d8099df8d", [:rebar3], [{:unicode_util_compat, "~>0.7.0", [hex: :unicode_util_compat, repo: "hexpm", optional: false]}], "hexpm", "92376eb7894412ed19ac475e4a86f7b413c1b9fbb5bd16dccd57934157944cea"},
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
   "mime": {:hex, :mime, "2.0.2", "0b9e1a4c840eafb68d820b0e2158ef5c49385d17fb36855ac6e7e087d4b1dcc5", [:mix], [], "hexpm", "e6a3f76b4c277739e36c2e21a2c640778ba4c3846189d5ab19f97f126df5f9b7"},
   "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
-  "opentelemetry": {:hex, :opentelemetry, "1.0.0-rc.2", "d3e1fd9debfd73e00b0241cac464be7cd6ca6ac2bd38ab2ebe0c92401c76a342", [:rebar3], [{:opentelemetry_api, "~> 1.0.0-rc.2", [hex: :opentelemetry_api, repo: "hexpm", optional: false]}], "hexpm", "2f810e2eed70a9ea0c9b6943969b59e37f96a2f9e10920045a6c7676c2ab8181"},
-  "opentelemetry_api": {:hex, :opentelemetry_api, "1.0.0-rc.2", "a0ec5b242bb7ce7563b4891e77dcfa529defc9e42c19a5a702574c5ac3d0c6e7", [:mix, :rebar3], [], "hexpm", "426a969c8ee2afa8ab55b58e6e40e81c1f934c064459a1acb530f54042f9a9a3"},
+  "opentelemetry": {:hex, :opentelemetry, "1.0.5", "f0cd36ac8b30b68e8d70cec5bb88801ed7f3fe79aac67597054ed5490542e810", [:rebar3], [{:opentelemetry_api, "~> 1.0", [hex: :opentelemetry_api, repo: "hexpm", optional: false]}], "hexpm", "3b17f8933a58e1246f42a0c215840fd8218aebbcabdb0aac62b0c766fe85542e"},
+  "opentelemetry_api": {:hex, :opentelemetry_api, "1.0.3", "77f9644c42340cd8b18c728cde4822ed55ae136f0d07761b78e8c54da46af93a", [:mix, :rebar3], [], "hexpm", "4293e06bd369bc004e6fad5edbb56456d891f14bd3f9f1772b18f1923e0678ea"},
   "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"},
-  "plug": {:hex, :plug, "1.13.3", "93b299039c21a8b82cc904d13812bce4ced45cf69153e8d35ca16ffb3e8c5d98", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "98c8003e4faf7b74a9ac41bee99e328b08f069bf932747d4a7532e97ae837a17"},
+  "plug": {:hex, :plug, "1.13.6", "187beb6b67c6cec50503e940f0434ea4692b19384d47e5fdfd701e93cadb4cc2", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4.3 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "02b9c6b9955bce92c829f31d6284bf53c591ca63c4fb9ff81dfd0418667a34ff"},
   "plug_cowboy": {:hex, :plug_cowboy, "2.5.2", "62894ccd601cf9597e2c23911ff12798a8a18d237e9739f58a6b04e4988899fe", [:mix], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:cowboy_telemetry, "~> 0.3", [hex: :cowboy_telemetry, repo: "hexpm", optional: false]}, {:plug, "~> 1.7", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "ea6e87f774c8608d60c8d34022a7d073bd7680a0a013f049fc62bf35efea1044"},
   "plug_crypto": {:hex, :plug_crypto, "1.2.2", "05654514ac717ff3a1843204b424477d9e60c143406aa94daf2274fdd280794d", [:mix], [], "hexpm", "87631c7ad914a5a445f0a3809f99b079113ae4ed4b867348dd9eec288cecb6db"},
   "ranch": {:hex, :ranch, "1.8.0", "8c7a100a139fd57f17327b6413e4167ac559fbc04ca7448e9be9057311597a1d", [:make, :rebar3], [], "hexpm", "49fbcfd3682fab1f5d109351b61257676da1a2fdbe295904176d5e521a2ddfe5"},

test/opentelemetry_plug_test.exs

@@ -2,13 +2,12 @@ defmodule OpentelemetryPlugTest do
   use ExUnit.Case, async: false
   require Record
 
-  Record.defrecord(:span, Record.extract(:span, from_lib: "opentelemetry/include/otel_span.hrl"))
+  for {name, spec} <- Record.extract_all(from_lib: "opentelemetry/include/otel_span.hrl") do
+    Record.defrecord(name, spec)
+  end
 
-  for r <- [:event, :status] do
-    Record.defrecord(
-      r,
-      Record.extract(r, from_lib: "opentelemetry_api/include/opentelemetry.hrl")
-    )
+  for {name, spec} <- Record.extract_all(from_lib: "opentelemetry_api/include/opentelemetry.hrl") do
+    Record.defrecord(name, spec)
   end
 
   setup_all do
@@ -47,11 +46,35 @@ defmodule OpentelemetryPlugTest do
     assert List.keymember?(headers, "traceparent", 0)
     assert_receive {:span, span(name: "/hello/:foo", attributes: attrs)}, 5000
 
+    attrs = :otel_attributes.map(attrs)
+
     for attr <- @default_attrs do
-      assert List.keymember?(attrs, attr, 0)
+      assert Map.has_key?(attrs, attr)
     end
   end
 
+  test "basic http attributes are set" do
+    # no query string
+    assert {200, _, "Hello world"} = request(:get, "/hello/world")
+    assert_receive {:span, span(name: "/hello/:foo", attributes: attrs)}, 5000
+
+    attrs = :otel_attributes.map(attrs)
+
+    assert "GET" == attrs[:"http.method"]
+    assert :http == attrs[:"http.scheme"]
+    assert host() == attrs[:"http.host"]
+    assert "/hello/world" == attrs[:"http.target"]
+    assert "hackney" <> _ = attrs[:"http.user_agent"]
+
+    # query string
+    assert {200, _, "Hello world"} = request(:get, "/hello/world?param=one&other=42")
+    assert_receive {:span, span(name: "/hello/:foo", attributes: attrs)}, 5000
+
+    attrs = :otel_attributes.map(attrs)
+
+    assert "/hello/world?param=one&other=42" == attrs[:"http.target"]
+  end
+
   test "adds optional attributes when available" do
     Application.put_env(:opentelemetry_plug, :server_name, "example.com")
 
@@ -60,35 +83,58 @@ defmodule OpentelemetryPlugTest do
 
     assert_receive {:span, span(attributes: attrs)}, 5000
 
-    assert List.keymember?(attrs, :"http.client_ip", 0)
-    assert List.keymember?(attrs, :"http.server_name", 0)
+    assert %{
+             "http.client_ip": "1.1.1.1",
+             "http.server_name": "example.com"
+           } = :otel_attributes.map(attrs)
   end
 
   test "records exceptions" do
     assert {500, _, _} = request(:get, "/hello/crash")
     assert_receive {:span, span(attributes: attrs, status: span_status, events: events)}, 5000
 
-    assert {:"http.status_code", 500} = List.keyfind(attrs, :"http.status_code", 0)
+    attrs = :otel_attributes.map(attrs)
+
+    assert %{"http.status_code": 500} = attrs
     assert status(code: :error, message: _) = span_status
+
+    events = :otel_events.list(events)
     assert [event(name: "exception", attributes: evt_attrs)] = events
 
+    evt_attrs = :otel_attributes.map(evt_attrs)
+
     for key <- ~w(exception.type exception.message exception.stacktrace) do
-      assert List.keymember?(evt_attrs, key, 0)
+      assert Map.has_key?(evt_attrs, key)
     end
   end
 
   test "sets span status on non-successful status codes" do
     assert {400, _, _} = request(:get, "/hello/bad-request")
     assert_receive {:span, span(attributes: attrs, status: span_status)}, 5000
-    assert {:"http.status_code", 400} = List.keyfind(attrs, :"http.status_code", 0)
+    attrs = :otel_attributes.map(attrs)
+    assert %{"http.status_code": 400} = attrs
     assert status(code: :error, message: _) = span_status
   end
 
+  test "gracefully handles nested routers" do
+    assert {200, _, _} = request(:get, "/hello/nested")
+
+    assert_receive {:span,
+                    span(name: "/hello/nested/*glob/", parent_span_id: parent, trace_id: trace_id)},
+                   5000
+
+    assert_receive {:span,
+                    span(name: "/hello/nested/*glob", span_id: ^parent, trace_id: ^trace_id)},
+                   5000
+  end
+
+  defp host do
+    :ranch.info(MyRouter.HTTP) |> Keyword.fetch!(:ip) |> :inet.ntoa() |> to_string()
+  end
+
   defp base_url do
-    info = :ranch.info(MyRouter.HTTP)
-    port = Keyword.fetch!(info, :port)
-    ip = Keyword.fetch!(info, :ip)
-    "http://#{:inet.ntoa(ip)}:#{port}"
+    port = :ranch.info(MyRouter.HTTP) |> Keyword.fetch!(:port)
+    "http://#{host()}:#{port}"
   end
 
   defp request(:head = verb, path) do
@@ -109,13 +155,26 @@ defmodule OpentelemetryPlugTest do
   end
 end
 
+defmodule MyRouter.NestedRouter do
+  use Plug.Router
+
+  plug :match
+  plug :dispatch
+
+  match "/" do
+    send_resp(conn, 200, "Hello from nested")
+  end
+end
+
 defmodule MyRouter do
   use Plug.Router
 
   plug :match
   plug OpentelemetryPlug.Propagation
   plug :dispatch
 
+  forward "/hello/nested", to: MyRouter.NestedRouter
+
   match "/hello/crash" do
     _ = conn
     raise ArgumentError