sentry-elixir
sentry-elixir copied to clipboard
Add installer (based on Igniter)
with this change, the installation instructions can be as simple as
mix igniter.install sentry --dsn "your_dsn"
For the release task change, a notice is emitted instructing the user to add the relevant task to their release process.
All good if you'd rather not add this change, but the installation steps for sentry were a great candidate for an installer.
My next steps would be:
- either to contribute this package the necessary telemetry listeners to trace Ash applications automatically
- create an
ash_sentrypackage that composes this installer and adds the necessary integration.
We can talk about Ash integration in a separate issue, just thought that I'd explain where I'm looking to go.
Oh, and you can test it yourself creating a new app:
mix igniter.new my_app --with phx.new --install sentry@github:zachdaniel/sentry-elixir
I forgot to explain the main endgame which is to put sentry as an option on the https://ash-hq.org installer
Also realized I was missing some pieces here to make the installer idempotent, will have some new code to look at shortly.
Something also to keep in mind is that for users who don't want to use igniter or can't for whatever reason, this shouldn't affect them at all given that it is an optional dependency. What we typically do is add a tabset to our installation guides, one for "with igniter (recommended)" and one for "manual". https://hexdocs.pm/ash/get-started.html#create-a-new-project
Any updates on how we're feeling about this?
Thanks @zachdaniel - I like the idea behind igniter. The only concern here is that having it officially in the SDK adds a little bit of extra maintenance burden for us. I can see that igniter has 0.5M downloads already, so I guess it's becoming popular quickly though (congrats!).
Is the idea here that each lib / framework is meant to provide their igniter integration? I'm asking because it's also common for tools like this to ship some integrations built-in instead.
In general yes the idea is that the installer lives directly inside the thing being installed. This ensures that it's versioned alongside the thing that is being installed, allowing hex version resolution to be the driver of which installer you get.
We've talked about adding ways to set up one shot installers that remove themselves after installation and don't live in the package being installed, but they lose a lot of the properties that make an installer good (like the above stuff).
I wouldn't personally maintain the sentry installer as something built in to igniter(we don't have any built in installers), although I'm happy support any igniter related issues and to be in the loop for the work done in this PR, i.e I'll support it but the buck stops w/ the project maintainers not me.
No pressure from me to accept the PR at the end of the day 😄 you could also just leave it open and wait to see if it becomes a truly ubiquitous standard?
@zachdaniel if we could count on you to help us with maintenance then I think it's reasonable, but it's not my call at the end of the day.
@sl0thentr0py please chime in whenever you find a minute 😄
i'm not familiar enough with the elixir ecosystem to tell how popular this is and so on, but if it's just an optional thing I don't mind. Final call is yours @solnic :)
how popular this is and so on
Not very popular yet, but this is not a lot to maintain (and it might become popular). I would just really strongly advise to add this as a dependency to Sentry itself ever in the future, but I don't think we would need that.
Not very popular yet, but this is not a lot to maintain (and it might become popular).
I'm not being defensive I promise, but it depends a bit on how you define popular. Oban, live debugger, error tracker, all of Ash, inertia, tidewave, mishka chelekom, beacon, oidcc etc. all have igniter installers. It's not ubiquitous because many packages have no need of an installer. Again, its fine to say its not popular, just pointing out that a fair few current important packages are using igniter for their onboarding 😄
Something we can also do is add your CI to our test suite: https://github.com/ash-project/igniter/actions/runs/15287244884
So if we can run your tests with --only igniter, we can ensure that your installer doesn't break w/ new versions of igniter.
@solnic happy to help if any issues arise.
I would just really strongly advise to add this as a dependency to Sentry itself ever in the future, but I don't think we would need that.
I don't understand what you mean here.
Not very popular yet, but this is not a lot to maintain (and it might become popular).
I'm not being defensive I promise, but it depends a bit on how you define popular. Oban, live debugger, error tracker, all of Ash, inertia, tidewave, mishka chelekom, beacon, oidcc etc. all have igniter installers. It's not ubiquitous because many packages have no need of an installer. Again, its fine to say its not popular, just pointing out that a fair few current important packages are using igniter for their onboarding 😄
Something we can also do is add your CI to our test suite: https://github.com/ash-project/igniter/actions/runs/15287244884
So if we can run your tests with
--only igniter, we can ensure that your installer doesn't break w/ new versions of igniter.@solnic happy to help if any issues arise.
@zachdaniel alright so let's get this rebased/updated + fix CI and we could merge it in 🙂
I've got a test failing that seems unrelated to my change, but otherwise I've rebased.
Looks like some issues come from building for pre 1.15 versions of Elixir, due to spitfire. This wouldn't cause problems for existing users on older versions of sentry, as igniter is an optional dependency. So we'd need to do something like only run CI for 1.15+ versions of Elixir 🥶
I'll update the tests not to require phx_new as an optional dependency. That is a utility some projects choose to use, but its not necessary 😄
Alright, fixed. I could also potentially go find a way to make spitfire still compile on Elixir 1.13. It doesn't matter if it doesn't work, its a "bonus" tool that makes our code patching smarter in various cases.
Ready for another run 😄
PR feedback/failures should be resolved, last major issue is the 1.13 issue. I'm going to try and make a PR to spitfire that lets it at least compile on 1.13. Our usage of it in igniter doesn't care if it blows up when we try to use it, as what we use it for only works on recent Elixir versions anyway.
🤔 it doesn't look like the plug dependency compiles on 1.13 either?
== Compilation error in file lib/plug/conn/utils.ex ==
** (ArgumentError) invalid right argument for operator "in", it expects a compile-time proper list or compile-time range on the right side when used in guard expressions, got: %{first: 65, last: 90, __struct__: Range, step: 1}
(elixir 1.13.4) lib/kernel.ex:4245: Kernel.raise_on_invalid_args_in_2/1
(elixir 1.13.4) expanding macro: Kernel.in/2
lib/plug/conn/utils.ex:67: Plug.Conn.Utils.mt_first/2
could not compile dependency :plug, "mix compile" failed. Errors may have been logged above. You can recompile this dependency with "mix deps.compile plug", update it with "mix deps.update plug" or clean it with "mix deps.clean plug"
I have a PR I can make to spitfire, not sure if it will be accepted. https://github.com/elixir-tools/spitfire/compare/main...zachdaniel:spitfire:main?expand=1
I'm assuming yanking 1.13 from CI is off the table 😅
I'm assuming yanking 1.13 from CI is off the table 😅
Yes we need to support it. Let's just move igniter tests to test_integration - we only run them on Elixir > 1.16.
I'm not sure that will work, because it's the dependency failing to compile. We'd have to make the dependency only appear conditionally using some kind of flag. I'll see about getting spitfire compiling on 1.13 😄
https://github.com/elixir-tools/spitfire/pull/62
Opened a PR to make spitfire compile on 1.13
Updated spitfire