getsentry / sentry-elixir

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

Default `:max_breadcrumbs` config not picked up after updating to v10.1 #680

Closed kwardynski closed 9 months ago

kwardynski commented 9 months ago

Hi sentry team! I'm running into a configuration issue after bumping from v9.x to v10.x -> the :max_breadcrumbs default config does not seem be picked up, which might be related to how the config is now stored as a persistent term.

Environment

macOs Ventura 13.0 (M2) erlang 25.3.2.7 elixir 1.15.4-otp-25 sentry 10.1.0 -> :max_breadcrumbs was not previously configured

Steps to Reproduce

  1. Update versions from 9.x -> 10.x
  2. Attempt any mix task (mix test, mix compile, etc.)
  3. Receive compilation error message

Expected Result

Successful compilation

Actual Result

All mix tasks now fail with the following compilation error:

== Compilation error in file <file_name>.ex ==
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: no persistent term stored with this key

    :persistent_term.get({:sentry_config, :max_breadcrumbs})
    (sentry 10.1.0) lib/sentry/config.ex:504: Sentry.Config.max_breadcrumbs/0
    (sentry 10.1.0) lib/sentry/context.ex:421: anonymous fn/2 in Sentry.Context.add_breadcrumb/1
    (elixir 1.15.4) lib/map.ex:676: Map.update/4
    (sentry 10.1.0) lib/sentry/context.ex:419: Sentry.Context.add_breadcrumb/1
    (stdlib 4.3.1.3) erl_eval.erl:748: :erl_eval.do_apply/7
    (stdlib 4.3.1.3) erl_eval.erl:136: :erl_eval.exprs/6
    <file_name>.ex:1: (file)

I have tried the following:

whatyouhide commented 9 months ago

@kwardynski hiya, thanks for the bug report! ๐Ÿ™

My best guess from the info you gave us is that the Sentry application is not starting. So, the first question I have is: do you by any chance have runtime: false or something like that alongside the :sentry dependency?

If that's not the case, we'll need a bit more info to try and debug this:

  1. The Sentry config you're using
  2. How and where you're calling Sentry.Config.max_breadcrumbs/0
kwardynski commented 9 months ago

Hey @whatyouhide, thanks for helping dig into this!

Here are the :sentry configs:

# config/prod.exs
config :sentry, 
  release: version_name,
  enable_source_code_context: true,
  root_source_code_paths: [File.cwd!()]

# config/runtime.exs
if config_env() == :prod do
  config :sentry,
    dsn: env!("SENTRY_DSN", :string?, nil),
    environment_name: infra_env,
    before_send: {MyApp.Sentry, :before_send}
end

# config/test.exs
config :sentry,
  environment_name: "test"
whatyouhide commented 9 months ago

@kwardynski awesome, thanks for the additional detail. Are you calling Sentry.Context.add_breacrumb/1 at compile time?

kwardynski commented 9 months ago

Yes we are... That might be the cause of this I'm guessing?

Could you confirm at which version the :max_breadcrumbs config moved to :persistent_term? We just did the bump from 9.0 to 10.1 and this "problem" is new

whatyouhide commented 9 months ago

Yeah the bump is in 10.0. The cause is that you're calling this at compile time, that is correct. If I may ask, what's the use case?

kwardynski commented 9 months ago

We have a macro which defines an add_breadcrumbs/x function, which we then invoke with a decorator (@decorate_all). Is :sentry still able to support breadcrumbs at compile time or do you have a workaround/better solution we could use? Right now we're calling Sentry.put_config(:max_breadcrumbs, 100) above the call to Sentry.Context.add_breadcrumb in the macro as a workaround, but the docs discourage that.

I would guess compile time support would make sense for breadcrumbs so that functionality could be used in decorators/macros but I could be wrong.

whatyouhide commented 9 months ago

I don't think that's how it works. Even if you write a macro that injects Sentry.Context.add_breadcrumb/1 calls at compile time into decorated functions, those calls should still happen at runtime for things to be working correctly. add_breadcrumb/1 uses the process dictionary to store the breadcrumbs, which means that you're not adding those breadcrumbs to the actual caller, but to the process compiling your code.

If you are able to, can you share the implementation of the relevant pieces (the macro, the decorator)? If yes, I could try and help to figure out if there's a way to do what you want to do. ๐Ÿ™ƒ

kwardynski commented 9 months ago

Hey @whatyouhide Sorry for the delayed response.

Just to clarify, it's not the storing of the breadcrumbs that's causing the compile time issue, it's accessing the :max_breadcrumbs parameter which is stored in :persistent_term (at least that's what I gather from the call stack)

Here is some anonymized and condensed code for the implementation. The Sentry.Context.add._breadcrumb function is accessed in two ways

  1. Inherited directly from the MyApp.SentryDecorator macro

    
    defmodule MyApp.SentryDecorator do
    def add_breadcrumbs(body, _context) do
    quote do
      {function, arity} = __ENV__.function
    
      # This was not needed before v10
      # -----
      Sentry.put_config(:max_breadcrumbs, 100)
      # -----
    
      Sentry.Context.add_breadcrumb(
        type: :function,
        message: "#{__MODULE__}.#{function}/#{arity}"
      )
    
      unquote(body)
    end
    end
    end

defmodule MyApp.SomeContext do use MyApp.SentryDecorator

@decorate_all add_breadcrumbs()

def foo, do: bar() end


2. Inherited indirectly through another macro...
```elixir  
defmodule MyApp.SentryDecorator do
  def add_breadcrumbs(body, _context) do
    quote do
      {function, arity} = __ENV__.function

      # This was not needed before v10
      # -----
      Sentry.put_config(:max_breadcrumbs, 100)
      # -----

      Sentry.Context.add_breadcrumb(
        type: :function,
        message: "#{__MODULE__}.#{function}/#{arity}"
      )

      unquote(body)
    end
  end
end

defmodule MyApp.Service do
  defmacro __using__(_) do
    use MyApp.SentryDecorator
    @decorate_all add_breadcrumbs()
  end
end

defmodule MyApp.SomeOtherContext do
  use MyApp.Service
  def bar, do: baz()
end
whatyouhide commented 9 months ago

@kwardynski no need to apologize ๐Ÿ˜Š

  1. Where does the @decorate_all attribute come from, another library?
  2. Where are you calling MyApp.SomeOtherContext.bar/0?
kwardynski commented 9 months ago

@whatyouhide

  1. yes - :decorator v1.4
  2. sort of a non-answer, but lots of places.

To elaborate on 2 - no modules which use the MyApp.Service module will compile without the call to Sentry.put_config in MyApp.SentryDecorator. When running any mix task (i.e. compile, test, etc.) there will be an error trace:

== Compilation error in file lib/my_app/some_other_context.ex ==
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: no persistent term stored with this key

    :persistent_term.get({:sentry_config, :max_breadcrumbs})
    (sentry 10.1.0) lib/sentry/config.ex:504: Sentry.Config.max_breadcrumbs/0
    (sentry 10.1.0) lib/sentry/context.ex:421: anonymous fn/2 in Sentry.Context.add_breadcrumb/1
    (elixir 1.15.4) lib/map.ex:676: Map.update/4
    (sentry 10.1.0) lib/sentry/context.ex:419: Sentry.Context.add_breadcrumb/1
    (stdlib 4.3.1.3) erl_eval.erl:748: :erl_eval.do_apply/7
    (stdlib 4.3.1.3) erl_eval.erl:136: :erl_eval.exprs/6
    /Users/kacperwardynski/git/server/lib/my_app/some_other_context.ex:1: (file)
whatyouhide commented 9 months ago

@kwardynski yeah, I understand that no modules compile without put_config/2. Iโ€™m pretty sure that the reason that happens is just that the :sentry application hasn't started when your add_breadcrumbs/2 function executes. You can confirm this by inserting this line before the call to put_config/2:

dbg(List.keyfind(Application.started_applications(), :sentry, 0))

If that prints out nil, then that's the issue: the :sentry application was not started yet.

If that's the case, then the issue here is that the add_breadcrumbs/2 function is called before the :sentry app has been started. You said that this is called in lots of places, so Iโ€™m not sure where that would happen. ๐Ÿ˜•

kwardynski commented 9 months ago

@whatyouhide

Confirmed that :sentry is not started when we're compiling the add_breadcrumbs function in the SentryDecorator module. Confident to say this is an "us" issue, not a "you" issue so I'll close this. Thank you for your help!

whatyouhide commented 9 months ago

Awesome! Glad we figured it out. I also tried to improve the error message here in 05c4c22. The message you got was pretty horrible, my bad ๐Ÿ™ƒ Thoughts?

kwardynski commented 9 months ago

@whatyouhide That's way more helpful ๐Ÿ‘