search

silverstripe / silverstripe-raygun

Raygun.com integration for PHP
BSD 3-Clause "New" or "Revised" License
6 stars 23 forks source link

Update for Raygun PHP SDK v2 when it is released #59

Closed robbieaverill closed 2 years ago

robbieaverill commented 4 years ago

The PHP SDK is preparing for its v2 release, which includes some breaking changes (notably https://github.com/MindscapeHQ/raygun4php/pull/117 re: client instantiation).

Just a placeholder for now, but when it's tagged it'd be a good idea to update this module to either allow both versions to be installed, or release a new minor version with an updated upstream dependency.

GuySartorelli commented 3 years ago

v2.0.0 was released on Jan 28, 2020 and there have been two minor releases since then. Has there been any work towards updating this module to support the v2 sdk?

brynwhyman commented 3 years ago

Hey @GuySartorelli no work that I'm aware of. I wouldn't say that this module is active development by any stretch.

I'd say there's an opportunity to become a maintainer if you're interested?

GuySartorelli commented 3 years ago

Good to know @brynwhyman We're still looking into our options as to which error monitoring service we want to go with - I'll keep this in mind and if we do choose raygun I'll probably take you up on that.

GuySartorelli commented 2 years ago

I've taken a few minutes to look at what would be involved to resolve this issue. graze/monolog-extensions has a dependency on mindscape/raygun4php ^1.0 so it's a bit more involved than I was hoping.

The three options I can see are (in no particular order):

  1. Replace the monolog-extensions package's raygun functionality inside this module. Hopefully that's just RaygunFormatter and RaygunHandler but I haven't looked particularly hard at this option yet so I don't know how dependent those classes are on anything else within the monolog-extensions package.
  2. Fork monolog-extensions and update the mindscape/raygun4php dependency. I haven't gone as far as to look at what would be involved for that yet.
  3. Wait for graze/monolog-extensions#31 to be completed, merged in, and released. I commented on that PR quite recently to see where this is at and my fingers are crossed... but we'll see what (if anything) happens.

@brynwhyman Let me know if you want me to go ahead with either of the first two options. We're doing a trial run with raygun on one of the sites we support at the moment and it is looking likely we'll be rolling it out on more of them over time, so I'm quite keen to get this module to support raygun SDK v2.

Note that only the first two options would (potentially) allow supporting v1 and v2 of the sdk simultaneously - and option 3 updates a few more upstream dependencies as well.

brynwhyman commented 2 years ago

Hey @GuySartorelli, I'm personally open to following your lead on this, especially if you're also going to be the one to do the work! To your point, the third option is clearly the ideal, fingers crossed there might be some traction on that other pull request.

I've also just shared this issue around internally in the hopes that someone else might be able to help with maintainership/ supporting this contribution to the module.

GuySartorelli commented 2 years ago

Awesome, thank you!

I think I'll wait a few weeks in hope the other PR has some movement - but failing that I think pulling the relevant code into this module (if there isn't too much of it) might be my preference. Much better than maintaining a whole extra forked package I think.

Update: graze/monolog-extensions is no longer being maintained (see graze/monolog-extensions#32) so when I get a chance I'll start work on option 1 above - though if someone beats me to it that would also work out :p

GuySartorelli commented 2 years ago

Finally got around to submitting a PR for this 🎉

madmatt commented 2 years ago

Fantastic work, thank you!