Improve ecto span/transaction

Author: solnic

lib/sentry/client.ex

@@ -271,6 +271,7 @@ defmodule Sentry.Client do
 
   @spec render_transaction(%Transaction{}) :: map()
   def render_transaction(%Transaction{} = transaction) do
+    # IO.inspect(transaction, label: "transaction")
     transaction
     |> Transaction.to_map()
     |> Map.merge(%{

lib/sentry/opentelemetry/span_processor.ex

@@ -47,10 +47,7 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
     })
   end
 
-  defp build_transaction(
-         %SpanRecord{attributes: attributes, origin: "opentelemetry_ecto"} = root_span,
-         child_spans
-       ) do
+  defp build_transaction(%SpanRecord{origin: "opentelemetry_ecto"} = root_span, child_spans) do
     Transaction.new(%{
       transaction: root_span.name,
       start_timestamp: root_span.start_time,
@@ -61,18 +58,7 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
       contexts: %{
         trace: build_trace_context(root_span)
       },
-      data: %{
-        "db.system" => attributes[:"db.system"],
-        "db.name" => attributes[:"db.name"],
-        "db.instance" => attributes[:"db.instance"],
-        "db.type" => attributes[:"db.type"],
-        "db.url" => attributes[:"db.url"],
-        "total_time_microseconds" => attributes[:total_time_microseconds],
-        "idle_time_microseconds" => attributes[:idle_time_microseconds],
-        "decode_time_microseconds" => attributes[:decode_time_microseconds],
-        "queue_time_microseconds" => attributes[:queue_time_microseconds],
-        "query_time_microseconds" => attributes[:query_time_microseconds]
-      },
+      data: root_span.attributes,
       measurements: %{},
       spans: Enum.map(child_spans, &build_span(&1))
     })
@@ -83,7 +69,7 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
          child_spans
        ) do
     Transaction.new(%{
-      transaction: "#{attributes[:"phoenix.plug"]}##{attributes[:"phoenix.action"]}",
+      transaction: "#{attributes["phoenix.plug"]}##{attributes["phoenix.action"]}",
       start_timestamp: root_span.start_time,
       timestamp: root_span.end_time,
       transaction_info: %{
@@ -93,23 +79,23 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
         trace: build_trace_context(root_span)
       },
       request: %{
-        url: attributes[:"http.target"],
-        method: attributes[:"http.method"],
+        url: attributes["http.target"],
+        method: attributes["http.method"],
         headers: %{
-          "User-Agent" => attributes[:"http.user_agent"]
+          "User-Agent" => attributes["http.user_agent"]
         },
         env: %{
-          "SERVER_NAME" => attributes[:"net.host.name"],
-          "SERVER_PORT" => attributes[:"net.host.port"]
+          "SERVER_NAME" => attributes["net.host.name"],
+          "SERVER_PORT" => attributes["net.host.port"]
         }
       },
       data: %{
-        "http.response.status_code" => attributes[:"http.status_code"],
-        "method" => attributes[:"http.method"],
-        "path" => attributes[:"http.target"],
+        "http.response.status_code" => attributes["http.status_code"],
+        "method" => attributes["http.method"],
+        "path" => attributes["http.target"],
         "params" => %{
-          "controller" => attributes[:"phoenix.plug"],
-          "action" => attributes[:"phoenix.action"]
+          "controller" => attributes["phoenix.plug"],
+          "action" => attributes["phoenix.action"]
         }
       },
       measurements: %{},
@@ -121,30 +107,30 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
          %SpanRecord{attributes: attributes, origin: "opentelemetry_bandit"} = root_span,
          child_spans
        ) do
-    %Sentry.Transaction{
+    Transaction.new(%{
       start_timestamp: root_span.start_time,
       timestamp: root_span.end_time,
-      transaction: attributes[:"http.target"],
+      transaction: attributes["http.target"],
       transaction_info: %{
         source: "url"
       },
       contexts: %{
         trace: build_trace_context(root_span)
       },
       request: %{
-        url: attributes[:"http.url"],
-        method: attributes[:"http.method"],
+        url: attributes["http.url"],
+        method: attributes["http.method"],
         headers: %{
-          "User-Agent" => attributes[:"http.user_agent"]
+          "User-Agent" => attributes["http.user_agent"]
         },
         env: %{
-          "SERVER_NAME" => attributes[:"net.peer.name"],
-          "SERVER_PORT" => attributes[:"net.peer.port"]
+          "SERVER_NAME" => attributes["net.peer.name"],
+          "SERVER_PORT" => attributes["net.peer.port"]
         }
       },
       measurements: %{},
       spans: Enum.map(child_spans, &build_span(&1))
-    }
+    })
   end
 
   defp build_trace_context(
@@ -157,19 +143,21 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
       op: "http.server",
       origin: root_span.origin,
       data: %{
-        "http.response.status_code" => attributes[:"http.status_code"]
+        "http.response.status_code" => attributes["http.status_code"]
       }
     }
   end
 
-  defp build_trace_context(%SpanRecord{origin: "opentelemetry_ecto"} = root_span) do
+  defp build_trace_context(
+         %SpanRecord{origin: "opentelemetry_ecto", attributes: attributes} = root_span
+       ) do
     %{
       trace_id: root_span.trace_id,
       span_id: root_span.span_id,
       parent_span_id: root_span.parent_span_id,
-      op: "db",
+      op: "db.#{attributes["db.type"]}.ecto",
       origin: root_span.origin,
-      data: root_span.attributes
+      data: attributes
     }
   end
 
@@ -187,7 +175,7 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
   defp build_span(
          %SpanRecord{origin: "opentelemetry_phoenix", attributes: attributes} = span_record
        ) do
-    op = "#{attributes[:"phoenix.plug"]}##{attributes[:"phoenix.action"]}"
+    op = "#{attributes["phoenix.plug"]}##{attributes["phoenix.action"]}"
 
     %Span{
       op: op,
@@ -196,7 +184,7 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
       trace_id: span_record.trace_id,
       span_id: span_record.span_id,
       parent_span_id: span_record.parent_span_id,
-      description: attributes[:"http.route"],
+      description: attributes["http.route"],
       origin: span_record.origin
     }
   end
@@ -228,16 +216,14 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
   defp build_span(%SpanRecord{origin: "opentelemetry_ecto", attributes: attributes} = span_record) do
     %Span{
       trace_id: span_record.trace_id,
-      op: span_record.name,
-      start_timestamp: span_record.start_time,
-      timestamp: span_record.end_time,
       span_id: span_record.span_id,
       parent_span_id: span_record.parent_span_id,
+      op: "db.#{attributes["db.type"]}.ecto",
+      description: attributes["db.statement"] || span_record.name,
       origin: span_record.origin,
-      data: %{
-        "db.system" => attributes[:"db.system"],
-        "db.name" => attributes[:"db.name"]
-      }
+      start_timestamp: span_record.start_time,
+      timestamp: span_record.end_time,
+      data: attributes
     }
   end
 

lib/sentry/opentelemetry/span_record.ex

@@ -30,13 +30,20 @@ defmodule Sentry.Opentelemetry.SpanRecord do
         origin: origin,
         start_time: cast_timestamp(otel_attrs[:start_time]),
         end_time: cast_timestamp(otel_attrs[:end_time]),
-        attributes: attributes
+        attributes: normalize_attributes(attributes)
       )
       |> Map.new()
 
     struct(__MODULE__, attrs)
   end
 
+  defp normalize_attributes(attributes) do
+    Enum.map(attributes, fn {key, value} ->
+      {to_string(key), value}
+    end)
+    |> Map.new()
+  end
+
   defp cast_span_id(nil), do: nil
   defp cast_span_id(:undefined), do: nil
   defp cast_span_id(span_id), do: bytes_to_hex(span_id, 16)

lib/sentry/transaction.ex

@@ -10,8 +10,6 @@ defmodule Sentry.Transaction do
     :transaction,
     :transaction_info,
     :contexts,
-    :platform,
-    :sdk,
     :request,
     :measurements,
     spans: [],

test_integrations/phoenix_app/lib/phoenix_app/application.ex

@@ -15,7 +15,7 @@ defmodule PhoenixApp.Application do
 
     OpentelemetryBandit.setup()
     OpentelemetryPhoenix.setup()
-    OpentelemetryEcto.setup([:phoenix_app, :repo])
+    OpentelemetryEcto.setup([:phoenix_app, :repo], db_statement: :enabled)
 
     children = [
       PhoenixAppWeb.Telemetry,

test_integrations/phoenix_app/test/phoenix_app/repo_test.ex

@@ -0,0 +1,27 @@
+defmodule PhoenixApp.RepoTest do
+  use PhoenixApp.DataCase
+
+  alias PhoenixApp.{Repo, User}
+
+  import Sentry.TestHelpers
+
+  setup do
+    put_test_config(dsn: "http://public:secret@localhost:8080/1")
+
+    Sentry.Test.start_collecting_sentry_reports()
+  end
+
+  test "instrumented top-level ecto transaction span" do
+    Repo.all(User) |> Enum.map(& &1.id)
+
+    transactions = Sentry.Test.pop_sentry_transactions()
+
+    assert length(transactions) == 1
+
+    assert [transaction] = transactions
+
+    assert transaction.transaction_info == %{source: "db"}
+    assert transaction.contexts.trace.op == "db.sql.ecto"
+    assert transaction.contexts.trace.data["db.system"] == :sqlite
+  end
+end

test_integrations/phoenix_app/test/phoenix_app_web/controllers/transaction_test.exs

@@ -60,7 +60,8 @@ defmodule Sentry.Integrations.Phoenix.TransactionTest do
 
     assert [span] = transaction.spans
 
-    assert span.op == "phoenix_app.repo.query:users"
+    assert span.op == "db.sql.ecto"
+    assert String.starts_with?(span.description, "SELECT ")
     assert span.trace_id == trace.trace_id
     assert span.parent_span_id == trace.span_id
   end