WIP - rework SpanStorage to use ETS

Author: solnic

lib/sentry/application.ex

@@ -10,6 +10,9 @@ defmodule Sentry.Application do
     config = Config.validate!()
     :ok = Config.persist(config)
 
+    # Setup ETS tables for span storage
+    Sentry.Opentelemetry.SpanStorage.setup()
+
     http_client = Keyword.fetch!(config, :client)
 
     maybe_http_client_spec =
@@ -27,7 +30,6 @@ defmodule Sentry.Application do
         Sentry.Sources,
         Sentry.Dedupe,
         Sentry.ClientReport.Sender,
-        Sentry.Opentelemetry.SpanStorage,
         {Sentry.Integrations.CheckInIDMappings,
          [
            max_expected_check_in_time:

lib/sentry/opentelemetry/span_processor.ex

@@ -62,7 +62,7 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
       contexts: %{
         trace: build_trace_context(root_span)
       },
-      spans: Enum.map([root_span | child_spans], &build_span(&1))
+      spans: [build_span(root_span) | Enum.map(child_spans, &build_span(&1))]
     })
   end
 
@@ -304,7 +304,9 @@ defmodule Sentry.Opentelemetry.SpanProcessor do
       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
+      parent_span_id: span_record.parent_span_id,
+      # Add origin to match other span types
+      origin: span_record.origin
     }
   end
 

lib/sentry/opentelemetry/span_storage.ex

@@ -1,88 +1,78 @@
 defmodule Sentry.Opentelemetry.SpanStorage do
-  use GenServer
+  @moduledoc false
 
-  def start_link(_opts) do
-    GenServer.start_link(__MODULE__, nil, name: __MODULE__)
-  end
-
-  def init(_) do
-    {:ok, %{root_spans: %{}, child_spans: %{}}}
-  end
+  @root_spans_table :sentry_root_spans
+  @child_spans_table :sentry_child_spans
 
-  def store_span(span_data) do
-    GenServer.call(__MODULE__, {:store_span, span_data})
-  end
+  def setup do
+    case :ets.whereis(@root_spans_table) do
+      :undefined ->
+        :ets.new(@root_spans_table, [:set, :public, :named_table])
 
-  def get_root_span(span_id) do
-    GenServer.call(__MODULE__, {:get_root_span, span_id})
-  end
+      _ ->
+        :ok
+    end
 
-  def get_child_spans(parent_span_id) do
-    GenServer.call(__MODULE__, {:get_child_spans, parent_span_id})
-  end
+    case :ets.whereis(@child_spans_table) do
+      :undefined ->
+        :ets.new(@child_spans_table, [:bag, :public, :named_table])
 
-  def update_span(span_data) do
-    GenServer.call(__MODULE__, {:update_span, span_data})
-  end
+      _ ->
+        :ok
+    end
 
-  def remove_span(span_id) do
-    GenServer.call(__MODULE__, {:remove_span, span_id})
+    :ok
   end
 
-  def remove_child_spans(parent_span_id) do
-    GenServer.call(__MODULE__, {:remove_child_spans, parent_span_id})
-  end
-
-  def handle_call({:store_span, span_data}, _from, state) do
+  def store_span(span_data) do
     if span_data.parent_span_id == nil do
-      new_state = put_in(state, [:root_spans, span_data.span_id], span_data)
-      {:reply, :ok, new_state}
+      :ets.insert(@root_spans_table, {span_data.span_id, span_data})
     else
-      new_state =
-        update_in(state, [:child_spans, span_data.parent_span_id], fn spans ->
-          (spans || []) ++ [span_data]
-        end)
-
-      {:reply, :ok, new_state}
+      :ets.insert(@child_spans_table, {span_data.parent_span_id, span_data})
     end
+
+    :ok
   end
 
-  def handle_call({:get_root_span, span_id}, _from, state) do
-    {:reply, state.root_spans[span_id], state}
+  def get_root_span(span_id) do
+    case :ets.lookup(@root_spans_table, span_id) do
+      [{^span_id, span}] -> span
+      [] -> nil
+    end
   end
 
-  def handle_call({:get_child_spans, parent_span_id}, _from, state) do
-    {:reply, state.child_spans[parent_span_id] || [], state}
+  def get_child_spans(parent_span_id) do
+    :ets.lookup(@child_spans_table, parent_span_id)
+    |> Enum.map(fn {_parent_id, span} -> span end)
   end
 
-  def handle_call({:update_span, span_data}, _from, state) do
+  def update_span(span_data) do
     if span_data.parent_span_id == nil do
-      new_state = put_in(state, [:root_spans, span_data.span_id], span_data)
-      {:reply, :ok, new_state}
+      :ets.insert(@root_spans_table, {span_data.span_id, span_data})
     else
-      new_state =
-        update_in(state, [:child_spans, span_data.parent_span_id], fn spans ->
-          Enum.map(spans || [], fn span ->
-            if span.span_id == span_data.span_id, do: span_data, else: span
-          end)
-        end)
-
-      {:reply, :ok, new_state}
+      existing_spans = :ets.lookup(@child_spans_table, span_data.parent_span_id)
+
+      :ets.delete(@child_spans_table, span_data.parent_span_id)
+
+      Enum.each(existing_spans, fn {parent_id, span} ->
+        if span.span_id != span_data.span_id do
+          :ets.insert(@child_spans_table, {parent_id, span})
+        end
+      end)
+
+      :ets.insert(@child_spans_table, {span_data.parent_span_id, span_data})
     end
-  end
 
-  def handle_call({:remove_span, span_id}, _from, state) do
-    new_state = %{
-      state
-      | root_spans: Map.delete(state.root_spans, span_id),
-        child_spans: Map.delete(state.child_spans, span_id)
-    }
+    :ok
+  end
 
-    {:reply, :ok, new_state}
+  def remove_span(span_id) do
+    :ets.delete(@root_spans_table, span_id)
+    :ok
   end
 
-  def handle_call({:remove_child_spans, parent_span_id}, _from, state) do
-    new_state = %{state | child_spans: Map.delete(state.child_spans, parent_span_id)}
-    {:reply, :ok, new_state}
+  def remove_child_spans(parent_span_id) do
+    :ets.delete(@child_spans_table, parent_span_id)
+    :ok
   end
 end

test/sentry/opentelemetry/span_processor_test.exs

@@ -3,6 +3,26 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
 
   import Sentry.TestHelpers
 
+  alias Sentry.Opentelemetry.SpanStorage
+
+  setup do
+    # Create tables
+    SpanStorage.setup()
+
+    on_exit(fn ->
+      # Only try to clean up tables if they exist
+      if :ets.whereis(:sentry_root_spans) != :undefined do
+        :ets.delete_all_objects(:sentry_root_spans)
+      end
+
+      if :ets.whereis(:sentry_child_spans) != :undefined do
+        :ets.delete_all_objects(:sentry_child_spans)
+      end
+    end)
+
+    :ok
+  end
+
   defmodule TestEndpoint do
     require OpenTelemetry.Tracer, as: Tracer