getsentry / sentry-elixir

The official Elixir SDK for Sentry (sentry.io)
https://sentry.io
MIT License
627 stars 185 forks source link

Refactor span storage #817

Open savhappy opened 4 weeks ago

savhappy commented 4 weeks ago

[ WIP] Needs test adjustment and specs Considerations: Should we store root spans and child spans in separate tables? Storing key/values like this -> parent: {{:root_span, span_id}, span} child: {parent_span_id, span} Should we implement some sort of sweep in case spans don't get removed on_end Should we order the child spans in the table? Or have an inserted_at value?

savhappy commented 1 week ago

@whatyouhide refactored with the genserver added. Left a comment wondering if we should sweep based on ttl or something else to avoid build up.

@solnic I have two failing test that I'm confused about..

  1) test sends captured spans as transactions with child spans (Sentry.Opentelemetry.SpanProcessorTest)
     test/sentry/opentelemetry/span_processor_test.exs:59
     ** (FunctionClauseError) no function clause matching in Calendar.ISO.parse_utc_datetime/2

     The following arguments were given to Calendar.ISO.parse_utc_datetime/2:

         # 1
         nil

         # 2
         :extended

     Attempted function clauses (showing 1 out of 1):

         def parse_utc_datetime(string, format) when is_binary(string) and (format === :basic or format === :extended)

     code: assert_valid_iso8601(transaction.timestamp)
     stacktrace:
       (elixir 1.17.2) lib/calendar/iso.ex:591: Calendar.ISO.parse_utc_datetime/2
       (elixir 1.17.2) lib/calendar/datetime.ex:1233: DateTime.from_iso8601/3
       test/sentry/opentelemetry/span_processor_test.exs:112: Sentry.Opentelemetry.SpanProcessorTest.assert_valid_iso8601/1
       test/sentry/opentelemetry/span_processor_test.exs:67: (test)

Thoughts?