add genserver back

Author: savhappy

lib/sentry/application.ex

@@ -10,9 +10,6 @@ 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 =
@@ -30,6 +27,7 @@ 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_storage.ex

@@ -1,5 +1,6 @@
 defmodule Sentry.Opentelemetry.SpanStorage do
   @moduledoc false
+  use GenServer
 
   @table :span_storage
 
@@ -9,7 +10,6 @@ defmodule Sentry.Opentelemetry.SpanStorage do
     GenServer.start_link(__MODULE__, nil, name: name)
   end
 
-  # start ets table on initialization
   @impl true
   def init(nil) do
     _table =
@@ -21,7 +21,10 @@ defmodule Sentry.Opentelemetry.SpanStorage do
   end
 
   def store_span(span_data) when span_data.parent_span_id == nil do
-    _ = :ets.insert(@table, {{:root_span, span_data.span_id}, span_data})
+    case :ets.lookup(@table, {:root_span, span_data.span_id}) do
+      [] -> :ets.insert(@table, {{:root_span, span_data.span_id}, span_data})
+      _ -> :ok
+    end
   end
 
   def store_span(span_data) do
@@ -31,43 +34,37 @@ defmodule Sentry.Opentelemetry.SpanStorage do
   def get_root_span(span_id) do
     case :ets.lookup(@table, {:root_span, span_id}) do
       [{{:root_span, ^span_id}, span}] -> span
-      # how do we handle if span cannot be found?
       [] -> nil
     end
   end
 
   def get_child_spans(parent_span_id) do
-    child_spans = :ets.lookup(@table, parent_span_id)
-
-    if child_spans == [] do
-      nil
-    else
-      Enum.map(child_spans, fn {parent_span_id, span} -> span end)
-    end
-  end
-
-  def update_span(span_data) when span_data.parent_span_id == nil do
-    case :ets.lookup(@table, {:root_span, span_data.span_id}) do
-      [] ->
-        :ets.insert(@table, {{:root_span, span_data.span_id}, span_data})
-
-      [{{:root_span, span_data.span_id}, span}] ->
-        :ets.delete(@table, {:root_span, span_data.span_id})
-        :ets.insert(@table, {{:root_span, span_data.span_id}, span_data})
-    end
+    :ets.lookup(@table, parent_span_id)
+    |> Enum.map(fn {_parent_id, span} -> span end)
   end
 
   def update_span(span_data) do
-    existing_spans = :ets.lookup(@table, span_data.parent_span_id)
-
-    :ets.delete(@table, span_data.parent_span_id)
-
-    Enum.each(existing_spans, fn {parent_span_id, span} ->
-      if span.span_id == span_data.span_id do
-        :ets.delete_object(@table, span)
-        :ets.insert(@table, {parent_span_id, span_data})
+    if span_data.parent_span_id == nil do
+      case :ets.lookup(@table, {:root_span, span_data.parent_span_id}) do
+        [] ->
+          :ets.insert(@table, {{:root_span, span_data.parent_span_id}, span_data})
+
+        _ ->
+          :ets.delete(@table, {:root_span, span_data.parent_span_id})
+          :ets.insert(@table, {{:root_span, span_data.parent_span_id}, span_data})
       end
-    end)
+    else
+      existing_spans = :ets.lookup(@table, span_data.parent_span_id)
+
+      Enum.each(existing_spans, fn {parent_id, span} ->
+        if span.span_id == span_data.span_id do
+          :ets.delete_object(@table, {parent_id, span})
+          :ets.insert(@table, {span_data.parent_span_id, span_data})
+        end
+      end)
+    end
+
+    :ok
   end
 
   def remove_span(span_id) do
@@ -79,4 +76,6 @@ defmodule Sentry.Opentelemetry.SpanStorage do
     :ets.delete(@table, parent_span_id)
     :ok
   end
+
+  # need to ad in sweep for any leftover spans. Should we initiate a sweep on_end or add a ttl or both?
 end

test/sentry/opentelemetry/span_processor_test.exs

@@ -6,17 +6,10 @@ defmodule Sentry.Opentelemetry.SpanProcessorTest do
   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)
+      if :ets.whereis(:span_storage) != :undefined do
+        :ets.delete_all_objects(:span_storage)
       end
     end)
 
@@ -71,7 +64,6 @@ 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