feat!: Add Phx 1.5 Telemetry Handlers

[novaugust]

This is work to close #23

Design

Check out the readme update - does that seem like a good approach for getting people to attach event handlers from this lib?

Note that we're losing the ability to instrument views in 1.5

Phx 1.5 & Spandex Phoenix..

Moving the test dep on phx to 1.5 breaks a lot of the existing tests, unsurprisingly. Would you be interested in releasing a backwards incompatible version of Spandex Phoenix for Phoenix >= 1.5?

If we did go to a hard requirement on Phx 1.5 +, we might look at using telemetry events only and removing the reliance on the use Phoenix.Spandex macro.

All Phx Telemetry events are here, and i'll excerpt relevant ones below:

• endpoint start
• endpoint stop
• router dispatch exception (error at the router pipeline/dispatch leve, sent instead of stopl)
• error rendered "dispatched at the end of an error view being rendered" (sent in adddition to exception, if an error view was used)

The first two replace the def call in the endpoint, and the third one replaces most of the error handling but maybe not all? I'd have to do more digging into error rendered to see.

Testing

Can't write tests for it until we have Phx 1.5, so blocked on the above =)

[novaugust]

@GregMefford lemme know what you think. In the meantime, implementing this shows me I can use the current Spandex Phoenix w/ phx 1.5 in my own app by just setting up these telemetry listeners and not using the instrumenter, so woohoo.

[zachdaniel]

This looks good to me so far, had a commend and a question.

[egze]

Is there any way I can help? Would really love to have something for Phoenix 1.5.

[novaugust]

Eh, it's blocked on my questions re: phx 1.5 (testing),
but also on the fact that I only have a day or two every other month to work on OSS ;)
if someone else has something that works, i'm pull my draft down and point at theirs.
i'd point out that anyone waiting for phx 1.5 integration can just manually integrate w/ telemetry like i'm doing here, test it in their own app, and report back with findings. i spiked it out at my own workplace but haven't been able to push the project forward

[GregMefford]

Yep, sorry this has sat for so long. I’ve also just been limited by time and energy lately outside of work. I’d like for this upgrade to just work smoothly for folks, so we need to be careful to do what we can there. For example, we don’t want to double-trace things, but we also don’t want to break things for people by requiring that they do something that they didn’t need to do before.

[novaugust]

@GregMefford dude, i feel you there. noooo worries.

So, I got traces working via telemetry. Installation documentation is easy: something in the readme for "Phx >= 1.5" and one part for older phx. The crux comes down to what you want for testing. Getting phx 1.5 into this repo breaks all the tests, and I'm only so excited about rewriting the entire test story for this lib xD If no tests works for you, it'd work for me! Orrr I could add telemetry as a test dep and manually release those events just to prove that traces get created for them. LMK what approach works for ya'll and I can get it done in the next month

[novaugust]

I’d like for this upgrade to just work smoothly for folks, so we need to be careful to do what we can there.

i should be clear on what the upgrade path would be. it would be:

1. undo any installation from previous spandex_phx
2. call Spandex.Phoenix.Telemetry.install/1 in your application startup

and i should also point out, there's no dependency on phx for that and this library is really almost not even needed. could just add a new module in spandex itself ;) (this is what i've done at work)

[egze]

I will start upgrading our phx app to 1.5 tomorrow and will surely test this PR as well. But I can already say that I love how it looks. ❤️

[zachdaniel]

This looks good to me. @novaugust if you wouldn't mind, can you add a bit more text to the section on using this w/ phoenix 1.5 to explain that you should remove any setup you currently have as well? I'll merge and release when that is done.

[novaugust]

@zachdaniel sure, i'll do a bit of cleanup and documentation and ping you

[novaugust]

@zachdaniel i think that's the money

[zachdaniel]

This has been released as version 1.0.0.

🚀Thank you for your contribution! 🚀