Add SpanProcessor for OpenTelemetry #875

solnic · 2025-03-14T10:15:23Z

To make this work you need to add opentelemetry deps:

      {:opentelemetry, "~> 1.3"},
      {:opentelemetry_api, "~> 1.2"},
      {:opentelemetry_exporter, "~> 1.6"},
      {:opentelemetry_phoenix, "~> 2.0"},
      {:opentelemetry_ecto, "~> 1.2"},
      {:opentelemetry_bandit, "~> 0.2"}

Then configure our span processor:

config :opentelemetry, span_processor: {Sentry.OpenTelemetry.SpanProcessor, []}

Things should start looking more or less like this:

solnic · 2025-03-14T14:09:34Z

1) test loads in_app_module_allow_list (Public.Sentry.ConfigTest)
Error:      apps/public/test/public_test.exs:4
     ** (RuntimeError) the Sentry configuration seems to be not available (while trying to fetch :in_app_module_allow_list). This is likely because the :sentry application has not been started yet. Make sure that you start the :sentry application before using any of its functions.

     code: assert Sentry.Config.in_app_module_allow_list() |> Enum.sort() ==
     stacktrace:
       (sentry 10.8.1) lib/sentry/config.ex:698: Sentry.Config.in_app_module_allow_list/0
       test/public_test.exs:5: (test)


Finished in 0.00 seconds (0.00s async, 0.00s sync)
1 test, 1 failure
==> admin
Running ExUnit with seed: 25[95](https://github.com/getsentry/sentry-elixir/actions/runs/13856201920/job/38773483717?pr=875#step:10:96)1, max_cases: 8



  1) test loads in_app_module_allow_list (Admin.Sentry.ConfigTest)
Error:      apps/admin/test/admin_test.exs:4
     ** (RuntimeError) the Sentry configuration seems to be not available (while trying to fetch :in_app_module_allow_list). This is likely because the :sentry application has not been started yet. Make sure that you start the :sentry application before using any of its functions.

     code: assert Sentry.Config.in_app_module_allow_list() |> Enum.sort() ==
     stacktrace:
       (sentry 10.8.1) lib/sentry/config.ex:6[98](https://github.com/getsentry/sentry-elixir/actions/runs/13856201920/job/38773483717?pr=875#step:10:99): Sentry.Config.in_app_module_allow_list/0
       test/admin_test.exs:5: (test)

@whatyouhide ☝🏻 I'm having trouble figuring out these failures. Could you tell me how I could debug what exactly causes the :sentry app startup failure? We start it manually in the umbrella test helpers and for whatever reason it started failing when opentelemetry is included.

whatyouhide · 2025-03-17T08:30:38Z

Adding just the OTel deps without any other changes leads to the same issue?

sl0thentr0py · 2025-03-17T11:47:36Z

please write some minimal description in the PR about

what functionality this adds
instructions to add this to a Phoenix app with opentelemetry instrumentation and test it out

solnic · 2025-03-18T09:55:26Z

Adding just the OTel deps without any other changes leads to the same issue?

@whatyouhide @sl0thentr0py I narrowed it down to opentelemetry dep. When it's included, the app doesn't start in umbrella integration test. We need to make otel deps optional anyway, so it's time to address this. How do we want to approach it though? Should I create sentry-opentelemetry package in this repo?

solnic · 2025-03-18T10:32:11Z

@whatyouhide for the time being I addressed it by using optional deps for otel packages via 6fdf121 but then one of the tests in event_test.exs started to fail so I fixed it via d04c5d3 even though I don't understand what's going on there 🙃

whatyouhide · 2025-03-18T11:46:33Z

@solnic is this ready for review? It's still a draft

solnic · 2025-03-18T14:31:25Z

@solnic is this ready for review? It's still a draft

Not yet. I got it working but phoenix + bandit spans are not processed in a way that would make sense for Sentry for some reason. I've been investigating how to fix. It seems like phoenix spans are not coming in as children of bandit spans so there's a disconnect here. I'll figure it out 🙂

sl0thentr0py · 2025-03-18T14:37:33Z

We need to make otel deps optional anyway, so it's time to address this.

If we're just shipping a SpanProcessor first, having them as peer dependencies is more than fine. We just document well how to get otel up and running in parallel to sentry and hook them up properly.

We will revisit packaging later once we have a proper working prototype.

solnic · 2025-03-19T13:00:31Z

@sl0thentr0py @whatyouhide this is now open for reviews. I got it deployed to production already and it's working well (see screenshots in the description).

sl0thentr0py · 2025-03-25T18:16:25Z

okay I'm taking a preliminary look now

one thing we definitely need is that instead of the boolean Config.tracing we need

a traces_sample_rate
also ideally, a traces_sampler callback

without these, it is very hard for people to control quota spend so this is a hard requirement

sl0thentr0py

some suggestions, looks very good otherwise!

sl0thentr0py · 2025-03-25T18:46:25Z

lib/sentry/opentelemetry/span_processor.ex

+    alias Sentry.Interfaces.Span
+
+    @impl true
+    def on_start(_ctx, otel_span, _config) do


The way you've implemented it, we can do nothing in on_start since the on_end logic is sufficient to build the span tree and transaction.

This will also simplify your SpanStorage.update_span where you can remove the unnecessary deletes.
(This is also how we've done it in the new python and JS span processors.)

@sl0thentr0py I'll dig into this more but when I make on_start a no-op the tests are no longer passing 🙃

@solnic did you dig into it more?

@whatyouhide yes but not until now I managed to figure out how to do it to make this work, I just pushed 9a5c550 that simplified things

lib/sentry/opentelemetry/span_storage.ex

whatyouhide

Looking like a good start but still a bunch of work left to do. Let me know if any of the comments are not clear!

lib/sentry/config.ex

mix.exs

lib/sentry/opentelemetry/sampler.ex

lib/sentry/opentelemetry/span_record.ex

lib/sentry/opentelemetry/span_storage.ex

test/sentry/opentelemetry/sampler_test.exs

test/sentry/opentelemetry/span_processor_test.exs

lib/sentry/opentelemetry/span_processor.ex

solnic · 2025-04-03T11:03:49Z

also ideally, a traces_sampler callback

without these, it is very hard for people to control quota spend so this is a hard requirement

@sl0thentr0py I'll add traces_sampler config in a separate PR

solnic · 2025-04-03T11:09:11Z

@whatyouhide @sl0thentr0py ready for another round of reviews 🤞🏻

whatyouhide · 2025-04-03T11:13:59Z

I'll review tomorrow.

lib/sentry/config.ex

whatyouhide · 2025-04-04T11:38:38Z

lib/sentry/opentelemetry/sampler.ex

@@ -0,0 +1,31 @@
+if Code.ensure_loaded?(:otel_sampler) do
+  defmodule Sentry.OpenTelemetry.Sampler do


Can you elaborate on why we need a Sentry-specific sampler?

@whatyouhide in order to provide a standard interface for traces_sampler option we need a custom abstraction for this

Can you elaborate on this with links to docs?

@whatyouhide Sentry has a standard way of handling trace sampling via traces_sampler configuration option which is meant to be a function that receives a sampling context. In order to provide support for this setting we just need our own Sampler that would call configured tracer sampler with a context that it would be built from information that it receives in the should_sample callback.

Here are docs about this feature in the Ruby SDK as a reference https://docs.sentry.io/platforms/ruby/configuration/sampling/#sampling-transaction-events

I would expect the Sampler to be my application-specific concern; this module seems unnecessary to me

lib/sentry/opentelemetry/span_processor.ex

whatyouhide · 2025-04-04T11:40:45Z

lib/sentry/opentelemetry/span_record.ex

+    require OpenTelemetry
+
+    @fields Record.extract(:span, from_lib: "opentelemetry/include/otel_span.hrl")
+    Record.defrecordp(:span, @fields)


I don't think this conversation is resolved?

jwaldrip · 2025-04-11T15:22:39Z

This feels so close! How far away do we think it is from release?

solnic · 2025-04-14T13:08:39Z

This feels so close! How far away do we think it is from release?

@jwaldrip yes just waiting for @whatyouhide for the final review :) then I'll follow up with another PR with support for sampler (initial work is already done) and we should be ready to release it 🙂

jwaldrip · 2025-04-14T16:54:02Z

@solnic I am pulling the branch directly and testing with our self hosted instance of sentry. Is there anything we need to do in our API configuration to have these show up in sentry under performance?

solnic · 2025-04-16T13:31:10Z

@solnic I am pulling the branch directly and testing with our self hosted instance of sentry. Is there anything we need to do in our API configuration to have these show up in sentry under performance?

@jwaldrip I'm sorry but I'm not familiar with self hosted instances. I need to confirm if anything extra is needed. For now please try to replicate setup that I added to our test_integrations/phoenix_app test app 🙂 Thanks for helping with testing!

whatyouhide

This is not really ready for review, there are quite a few comments that have been resolved without being addressed or comments that have not been addressed in general.

This is a big, complex PR, so I would in general suggest splitting it up in "less controversial" pieces. For example, adding a :traces_sample configuration is very straightforward, and if it were its own PR, then it could be a piece we merge and not think about. Same goes for adding tests to the Phoenix app, for example.

In general, for this kind of workflow I suggest something like stacked PRs if you're familiar with the concept.

lib/sentry/config.ex

whatyouhide · 2025-04-18T06:57:29Z

lib/sentry/opentelemetry/span_processor.ex

+    alias Sentry.Interfaces.Span
+
+    @impl true
+    def on_start(_ctx, otel_span, _config) do


@solnic did you dig into it more?

whatyouhide · 2025-04-18T06:59:55Z

lib/sentry/opentelemetry/span_processor.ex

+    defp get_op_description(%{
+           attributes:
+             %{unquote(to_string(MessagingAttributes.messaging_system())) => :oban} = attributes
+         }) do
+      {"queue.process", attributes["oban.job.worker"]}
+    end


Wouldn't the Oban/OTel integration be the one responsible for getting this right here?

@whatyouhide queue.process is a Sentry concept, that's why we have to handle it ourselves here. From Sentry docs:

queue.process

Based on the knowledge sources, queue.process is a span operation used in Sentry's tracing instrumentation for queue consumers. This operation is used to track when messages are delivered to or processed by a consumer in a messaging system.

Consumer Instrumentation

To implement queue consumer instrumentation with Sentry, you need to:
Create a span with the operation queue.process
Include specific attributes to enrich your consumer spans with queue metrics

lib/sentry/opentelemetry/span_storage.ex

whatyouhide · 2025-04-18T07:03:56Z

lib/sentry/opentelemetry/sampler.ex

@@ -0,0 +1,31 @@
+if Code.ensure_loaded?(:otel_sampler) do
+  defmodule Sentry.OpenTelemetry.Sampler do


Can you elaborate on this with links to docs?

test/sentry/opentelemetry/span_processor_test.exs

whatyouhide · 2025-04-18T07:05:48Z

test/support/case.ex

+  end
+
+  defp setup_span_storage(opts) do
+    uid = :rand.uniform(1000000)


Is this supposed to be unique?

@whatyouhide yes

Then :rand.uniform/1 won't guarantee that in any way. Use System.unique_integer([:positive]) instead.

test_integrations/phoenix_app/mix.exs

solnic · 2025-04-18T07:48:33Z

This is not really ready for review, there are quite a few comments that have been resolved without being addressed or comments that have not been addressed in general.

Oops, seems like I've got a commit from April 4th that I forgot to push. I used GH PR ext in VSCode to mark things as done but then my branch wasn't synced. Dooh! Sorry about that. The docs were updated according to your requests via f883094

In general, for this kind of workflow I suggest something like stacked PRs if you're familiar with the concept.

Yes I'm familiar with this approach. Should I start doing it here?

Previous versions were no longer supported by actions that we rely on.

jwaldrip · 2025-04-25T15:40:17Z

@solnic are these the final parts that need to be reviewed by @whatyouhide? Hoping we can see this released this month. 🥺

solnic linked an issue Mar 14, 2025 that may be closed by this pull request

Add SpanProcessor for OTel #874

Open

solnic force-pushed the 874-add-spanprocessor-for-otel branch from 0ebd132 to dee9540 Compare March 14, 2025 11:17

This was referenced Mar 14, 2025

Support for Traces/Transactions via Opentelemetry #784

Closed

OpenTelemetry Support #853

Closed

Performance powered by OTel #538

Open

solnic mentioned this pull request Mar 18, 2025

OpenTelemetry integration #843

Closed

solnic force-pushed the 874-add-spanprocessor-for-otel branch 2 times, most recently from b607ac1 to bdcc5de Compare March 19, 2025 12:45

solnic marked this pull request as ready for review March 19, 2025 12:59

solnic requested review from whatyouhide, savhappy and sl0thentr0py March 19, 2025 13:00

sl0thentr0py requested changes Mar 25, 2025

View reviewed changes

whatyouhide requested changes Mar 30, 2025

View reviewed changes

solnic added 6 commits April 2, 2025 11:19

Add SpanProcessor for OpenTelemetry

0710007

Move opentelemetry config to test env

178825d

Add tracing config option

5eb88c2

Minor formatting fix

4c96544

Mark SpanRecord as @moduledoc false

71937b0

Match otel span when building SpanRecord

894a30b

solnic force-pushed the 874-add-spanprocessor-for-otel branch from c9152c6 to 1654724 Compare April 3, 2025 10:44

solnic added 3 commits April 3, 2025 11:07

Make SpanStorage more robust

edd724c

Minor formatting tweak

149d0fe

tracing => traces_sample_rate

5e8fc99

solnic force-pushed the 874-add-spanprocessor-for-otel branch from 01585a8 to 5e8fc99 Compare April 3, 2025 11:07

solnic requested review from whatyouhide and sl0thentr0py April 3, 2025 11:08

whatyouhide reviewed Apr 4, 2025

View reviewed changes

Update docs for :traces_sample_rate option

f883094

solnic requested a review from whatyouhide April 8, 2025 11:07

whatyouhide requested changes Apr 18, 2025

View reviewed changes

solnic added 8 commits April 18, 2025 14:50

Update doc

293d4e1

Add specs for pub functions in SpanStorage

e8f2bb7

SpanProcessor test can't be async due to config change

7460c58

Make otel deps mandatory for phx test app

0485c10

Simplify span processor

9a5c550

Add missing type for span record

5959016

Fix spec for store_span

ad340c0

Update 1.13 / OTP / os in CI

0559b9e

Previous versions were no longer supported by actions that we rely on.

solnic force-pushed the 874-add-spanprocessor-for-otel branch from 7b00dd9 to 0559b9e Compare April 23, 2025 07:29

solnic requested a review from whatyouhide April 23, 2025 07:54

		@@ -0,0 +1,31 @@
		if Code.ensure_loaded?(:otel_sampler) do
		defmodule Sentry.OpenTelemetry.Sampler do

Add SpanProcessor for OpenTelemetry #875

Are you sure you want to change the base?

Add SpanProcessor for OpenTelemetry #875

Conversation

solnic commented Mar 14, 2025 • edited Loading

solnic commented Mar 14, 2025

whatyouhide commented Mar 17, 2025

sl0thentr0py commented Mar 17, 2025 • edited Loading

solnic commented Mar 18, 2025

solnic commented Mar 18, 2025

whatyouhide commented Mar 18, 2025

solnic commented Mar 18, 2025

sl0thentr0py commented Mar 18, 2025

solnic commented Mar 19, 2025

sl0thentr0py commented Mar 25, 2025

sl0thentr0py left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whatyouhide left a comment

Choose a reason for hiding this comment

solnic commented Apr 3, 2025

solnic commented Apr 3, 2025

whatyouhide commented Apr 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwaldrip commented Apr 11, 2025

solnic commented Apr 14, 2025

jwaldrip commented Apr 14, 2025

solnic commented Apr 16, 2025

whatyouhide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solnic commented Apr 18, 2025 • edited Loading

jwaldrip commented Apr 25, 2025

solnic commented Mar 14, 2025 •

edited

Loading

sl0thentr0py commented Mar 17, 2025 •

edited

Loading

solnic commented Apr 18, 2025 •

edited

Loading