Refactor OTel (#835)

* Add environment to transaction

* Make root span the transaction

* Generic transactions spans parsing

Author: danschultzer

lib/sentry/opentelemetry/span_processor.ex

@@ -361,7 +361,7 @@ defmodule Sentry.Test do
 
       iex> Sentry.Test.start_collecting_sentry_reports()
       :ok
-      iex> Sentry.send_transaction(%Sentry.Transaction{})
+      iex> Sentry.send_transaction(Sentry.Transaction.new(%{span_id: "123", spans: []}))
       {:ok, ""}
       iex> [%Sentry.Transaction{}] = Sentry.Test.pop_sentry_transactions()
 

lib/sentry/test.ex

@@ -1,50 +1,60 @@
 defmodule Sentry.Transaction do
   @type t() :: %__MODULE__{}
 
-  alias Sentry.{UUID}
+  alias Sentry.{Config, UUID}
+
+  @enforce_keys ~w(event_id span_id spans)a
 
   defstruct [
     :event_id,
-    :start_timestamp,
-    :timestamp,
+    :environment,
+    :span_id,
     :transaction,
     :transaction_info,
-    :status,
     :contexts,
-    :request,
     :measurements,
-    spans: [],
+    :spans,
     type: "transaction"
   ]
 
   def new(attrs) do
-    struct(__MODULE__, Map.put(attrs, :event_id, UUID.uuid4_hex()))
+    struct!(
+      __MODULE__,
+      attrs
+      |> Map.put(:event_id, UUID.uuid4_hex())
+      |> Map.put(:environment, Config.environment_name())
+    )
   end
 
   # Used to then encode the returned map to JSON.
   @doc false
   def to_map(%__MODULE__{} = transaction) do
-    Map.put(
-      Map.from_struct(transaction),
-      :spans,
-      Enum.map(transaction.spans, &Sentry.Span.to_map(&1))
-    )
+    transaction_attrs = Map.take(transaction, [:event_id, :environment, :transaction, :transaction_info, :contexts, :measurements, :type])
+    {[root_span], child_spans} = Enum.split_with(transaction.spans, &is_nil(&1.parent_span_id))
+
+    root_span
+    |> Sentry.Span.to_map()
+    |> Map.put(:spans, Enum.map(child_spans, &Sentry.Span.to_map/1))
+    |> Map.drop([:description])
+    |> Map.merge(transaction_attrs)
   end
 end
 
 defmodule Sentry.Span do
+  @enforce_keys ~w(span_id trace_id start_timestamp timestamp)a
+
   defstruct [
-    :op,
+    :trace_id,
+    :span_id,
+    :parent_span_id,
     :start_timestamp,
     :timestamp,
     :description,
-    :span_id,
-    :parent_span_id,
-    :trace_id,
+    :op,
+    :status,
     :tags,
     :data,
-    :origin,
-    :status
+    :origin
   ]
 
   # Used to then encode the returned map to JSON.

lib/sentry/transaction.ex

@@ -115,7 +115,8 @@ defmodule Sentry.Mixfile do
 
       # Required by Tracing
       {:opentelemetry, "~> 1.5"},
-      {:opentelemetry_api, "~> 1.3"}
+      {:opentelemetry_api, "~> 1.3"},
+      {:opentelemetry_semantic_conventions, "~> 1.0"}
     ]
   end
 

mix.exs

@@ -34,7 +34,7 @@
   "oban": {:hex, :oban, "2.18.3", "1608c04f8856c108555c379f2f56bc0759149d35fa9d3b825cb8a6769f8ae926", [:mix], [{:ecto_sql, "~> 3.10", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:ecto_sqlite3, "~> 0.9", [hex: :ecto_sqlite3, repo: "hexpm", optional: true]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.16", [hex: :postgrex, repo: "hexpm", optional: true]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "36ca6ca84ef6518f9c2c759ea88efd438a3c81d667ba23b02b062a0aa785475e"},
   "opentelemetry": {:hex, :opentelemetry, "1.5.0", "7dda6551edfc3050ea4b0b40c0d2570423d6372b97e9c60793263ef62c53c3c2", [:rebar3], [{:opentelemetry_api, "~> 1.4", [hex: :opentelemetry_api, repo: "hexpm", optional: false]}], "hexpm", "cdf4f51d17b592fc592b9a75f86a6f808c23044ba7cf7b9534debbcc5c23b0ee"},
   "opentelemetry_api": {:hex, :opentelemetry_api, "1.4.0", "63ca1742f92f00059298f478048dfb826f4b20d49534493d6919a0db39b6db04", [:mix, :rebar3], [], "hexpm", "3dfbbfaa2c2ed3121c5c483162836c4f9027def469c41578af5ef32589fcfc58"},
-  "opentelemetry_semantic_conventions": {:hex, :opentelemetry_semantic_conventions, "0.2.0", "b67fe459c2938fcab341cb0951c44860c62347c005ace1b50f8402576f241435", [:mix, :rebar3], [], "hexpm", "d61fa1f5639ee8668d74b527e6806e0503efc55a42db7b5f39939d84c07d6895"},
+  "opentelemetry_semantic_conventions": {:hex, :opentelemetry_semantic_conventions, "1.27.0", "acd0194a94a1e57d63da982ee9f4a9f88834ae0b31b0bd850815fe9be4bbb45f", [:mix, :rebar3], [], "hexpm", "9681ccaa24fd3d810b4461581717661fd85ff7019b082c2dff89c7d5b1fc2864"},
   "parse_trans": {:hex, :parse_trans, "3.4.1", "6e6aa8167cb44cc8f39441d05193be6e6f4e7c2946cb2759f015f8c56b76e5ff", [:rebar3], [], "hexpm", "620a406ce75dada827b82e453c19cf06776be266f5a67cff34e1ef2cbb60e49a"},
   "phoenix": {:hex, :phoenix, "1.7.17", "2fcdceecc6fb90bec26fab008f96abbd0fd93bc9956ec7985e5892cf545152ca", [:mix], [{:castore, ">= 0.0.0", [hex: :castore, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.1", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_template, "~> 1.0", [hex: :phoenix_template, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 2.0", [hex: :phoenix_view, repo: "hexpm", optional: true]}, {:plug, "~> 1.14", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.7", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2 or ~> 2.0", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}, {:websock_adapter, "~> 0.5.3", [hex: :websock_adapter, repo: "hexpm", optional: false]}], "hexpm", "50e8ad537f3f7b0efb1509b2f75b5c918f697be6a45d48e49a30d3b7c0e464c9"},
   "phoenix_html": {:hex, :phoenix_html, "4.1.1", "4c064fd3873d12ebb1388425a8f2a19348cef56e7289e1998e2d2fa758aa982e", [:mix], [], "hexpm", "f2f2df5a72bc9a2f510b21497fd7d2b86d932ec0598f0210fed4114adc546c6f"},

mix.lock

@@ -117,14 +117,14 @@ defmodule Sentry.EnvelopeTest do
     test "works with transactions" do
       put_test_config(environment_name: "test")
 
-      spans = [
+      root_span =
         %Sentry.Span{
           start_timestamp: 1_588_601_261.481_961,
           timestamp: 1_588_601_261.488_901,
           description: "GET /sockjs-node/info",
           op: "http",
           span_id: "b01b9f6349558cd1",
-          parent_span_id: "b0e6f15b45c36b12",
+          parent_span_id: nil,
           trace_id: "1e57b752bc6e4544bbaa246cd1d05dee",
           tags: %{"http.status_code" => "200"},
           data: %{
@@ -133,23 +133,27 @@ defmodule Sentry.EnvelopeTest do
             "type" => "xhr",
             "method" => "GET"
           }
-        },
-        %Sentry.Span{
-          start_timestamp: 1_588_601_261.535_386,
-          timestamp: 1_588_601_261.544_196,
-          description: "Vue <App>",
-          op: "update",
-          span_id: "b980d4dec78d7344",
-          parent_span_id: "9312d0d18bf51736",
-          trace_id: "1e57b752bc6e4544bbaa246cd1d05dee"
         }
-      ]
 
-      transaction = %Sentry.Transaction{
-        start_timestamp: System.system_time(:second),
-        timestamp: System.system_time(:second),
-        spans: spans
-      }
+      child_spans =
+        [
+          %Sentry.Span{
+            start_timestamp: 1_588_601_261.535_386,
+            timestamp: 1_588_601_261.544_196,
+            description: "Vue <App>",
+            op: "update",
+            span_id: "b980d4dec78d7344",
+            parent_span_id: "9312d0d18bf51736",
+            trace_id: "1e57b752bc6e4544bbaa246cd1d05dee"
+          }
+        ]
+
+      transaction =
+        Sentry.Transaction.new(%{
+          span_id: root_span.span_id,
+          spans: [root_span | child_spans],
+          transaction: "test-transaction"
+        })
 
       envelope = Envelope.from_transaction(transaction)
 
@@ -159,16 +163,13 @@ defmodule Sentry.EnvelopeTest do
 
       assert {:ok, decoded_transaction} = Jason.decode(transaction_line)
       assert decoded_transaction["type"] == "transaction"
-      assert decoded_transaction["start_timestamp"] == transaction.start_timestamp
-      assert decoded_transaction["timestamp"] == transaction.timestamp
-
-      assert [span1, span2] = decoded_transaction["spans"]
+      assert decoded_transaction["start_timestamp"] == root_span.start_timestamp
+      assert decoded_transaction["timestamp"] == root_span.timestamp
 
-      assert span1["start_timestamp"] == List.first(spans).start_timestamp
-      assert span1["timestamp"] == List.first(spans).timestamp
+      assert [span] = decoded_transaction["spans"]
 
-      assert span2["start_timestamp"] == List.last(spans).start_timestamp
-      assert span2["timestamp"] == List.last(spans).timestamp
+      assert span["start_timestamp"] == List.first(child_spans).start_timestamp
+      assert span["timestamp"] == List.first(child_spans).timestamp
     end
   end
 

test/envelope_test.exs

@@ -4,7 +4,7 @@ defmodule Sentry.ClientReportTest do
   import Sentry.TestHelpers
 
   alias Sentry.ClientReport.Sender
-  alias Sentry.{Event, Transaction}
+  alias Sentry.{Event, Transaction, Span}
 
   setup do
     original_retries =
@@ -19,6 +19,8 @@ defmodule Sentry.ClientReportTest do
     %{bypass: bypass}
   end
 
+  @span_id Sentry.UUID.uuid4_hex()
+
   describe "record_discarded_events/2 + flushing" do
     test "succefully records the discarded event to the client report", %{bypass: bypass} do
       start_supervised!({Sender, name: :test_client_report})
@@ -28,10 +30,16 @@ defmodule Sentry.ClientReportTest do
           event_id: Sentry.UUID.uuid4_hex(),
           timestamp: "2024-10-12T13:21:13"
         },
-        %Transaction{
-          event_id: Sentry.UUID.uuid4_hex(),
-          timestamp: "2024-10-12T13:21:13"
-        }
+        Transaction.new(%{
+          span_id: @span_id,
+          transaction: "test-transaction",
+          spans: [%Span{
+            span_id: @span_id,
+            trace_id: Sentry.UUID.uuid4_hex(),
+            start_timestamp: "2024-10-12T13:21:13",
+            timestamp: "2024-10-12T13:21:13"
+          }]
+        })
       ]
 
       assert :ok = Sender.record_discarded_events(:before_send, events, :test_client_report)

test/sentry/client_report/sender_test.exs

@@ -44,16 +44,18 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
 
     assert [%Sentry.Transaction{} = transaction] = Sentry.Test.pop_sentry_transactions()
 
-    assert_valid_iso8601(transaction.timestamp)
-    assert_valid_iso8601(transaction.start_timestamp)
-    assert transaction.timestamp > transaction.start_timestamp
-    assert length(transaction.spans) == 1
-
-    assert_valid_trace_id(transaction.contexts.trace.trace_id)
-
-    assert [span] = transaction.spans
-
-    assert span.op == "child_instrumented_function_one"
+    transaction_data = Sentry.Transaction.to_map(transaction)
+
+    assert transaction_data.event_id
+    assert transaction_data.environment == "test"
+    assert transaction_data.type == "transaction"
+    assert transaction_data.op == "child_instrumented_function_one"
+    assert transaction_data.transaction_info == %{source: :custom}
+    assert_valid_iso8601(transaction_data.timestamp)
+    assert_valid_iso8601(transaction_data.start_timestamp)
+    assert transaction_data.timestamp > transaction_data.start_timestamp
+    assert_valid_trace_id(transaction_data.contexts.trace.trace_id)
+    assert length(transaction_data.spans) == 0
   end
 
   test "sends captured spans as transactions with child spans" do
@@ -64,12 +66,16 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
     TestEndpoint.instrumented_function()
 
     assert [%Sentry.Transaction{} = transaction] = Sentry.Test.pop_sentry_transactions()
-    assert_valid_iso8601(transaction.timestamp)
-    assert_valid_iso8601(transaction.start_timestamp)
-    assert transaction.timestamp > transaction.start_timestamp
-    assert length(transaction.spans) == 3
 
-    [root_span, child_span_one, child_span_two] = transaction.spans
+    transaction_data = Sentry.Transaction.to_map(transaction)
+
+    assert transaction_data.op == "instrumented_function"
+    assert_valid_iso8601(transaction_data.timestamp)
+    assert_valid_iso8601(transaction_data.start_timestamp)
+    assert transaction_data.timestamp > transaction_data.start_timestamp
+    assert length(transaction_data.spans) == 2
+
+    [child_span_one, child_span_two] = transaction_data.spans
     assert child_span_one.op == "child_instrumented_function_one"
     assert child_span_two.op == "child_instrumented_function_two"
     assert child_span_one.parent_span_id == transaction.contexts.trace.span_id
@@ -82,10 +88,10 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
 
     assert child_span_one.timestamp > child_span_one.start_timestamp
     assert child_span_two.timestamp > child_span_two.start_timestamp
-    assert root_span.timestamp >= child_span_one.timestamp
-    assert root_span.timestamp >= child_span_two.timestamp
-    assert root_span.start_timestamp <= child_span_one.start_timestamp
-    assert root_span.start_timestamp <= child_span_two.start_timestamp
+    assert transaction_data.timestamp >= child_span_one.timestamp
+    assert transaction_data.timestamp >= child_span_two.timestamp
+    assert transaction_data.start_timestamp <= child_span_one.start_timestamp
+    assert transaction_data.start_timestamp <= child_span_two.start_timestamp
 
     assert_valid_trace_id(transaction.contexts.trace.trace_id)
     assert_valid_trace_id(child_span_one.trace_id)

test/sentry/opentelemetry/span_processor_test.exs

@@ -240,9 +240,16 @@ defmodule SentryTest do
     setup do
       transaction =
         Sentry.Transaction.new(%{
+          span_id: "root-span",
           transaction: "test-transaction",
-          start_timestamp: System.system_time(:second),
-          timestamp: System.system_time(:second)
+          spans: [
+            %Sentry.Span{
+              span_id: "root-span",
+              trace_id: "trace-id",
+              start_timestamp: 1_234_567_891.123_456,
+              timestamp: 1_234_567_891.123_456
+            }
+          ]
         })
 
       {:ok, transaction: transaction}

test/sentry_test.exs

@@ -14,7 +14,7 @@ defmodule PhoenixApp.Application do
     })
 
     # OpentelemetryBandit.setup()
-    OpentelemetryPhoenix.setup()
+    OpentelemetryPhoenix.setup(adapter: :bandit)
     OpentelemetryOban.setup()
     OpentelemetryEcto.setup([:phoenix_app, :repo], db_statement: :enabled)