publiclab / mapknitter

Upload your own aerial images, position (rubbersheet) them in a web interface over existing map data, and share via web or composite and export for print.
http://mapknitter.org
GNU General Public License v3.0
267 stars 207 forks source link

migrate sentry-raven to sentry-ruby, upgrade to Ruby 2.7.6, remove GDAL 😲 #1712

Closed jywarren closed 2 years ago

jywarren commented 2 years ago

Re https://github.com/publiclab/plots2/pull/10364/ for MapKnitter, following https://docs.sentry.io/platforms/ruby/guides/rails/migration/#api-changes

Additional updates and removal of GDAL was necessary to get this all running together hence the combined PR, sorry!

gitpod-io[bot] commented 2 years ago

codeclimate[bot] commented 2 years ago

Code Climate has analyzed commit 0bd1f2cf and detected 0 issues on this pull request.

View more on Code Climate.

jywarren commented 2 years ago

https://github.com/getsentry/sentry-ruby/issues/1427

Esp. interference with skylight! https://github.com/getsentry/sentry-ruby/issues/1427#issuecomment-904242825

jywarren commented 2 years ago

Yes, Skylight v5 seems likely a solution! https://blog.skylight.io/skylight-5-source-locations/ - "Skylight 5 also includes a shift to Module#prepend for adding instrumentation to Rails"

codecov[bot] commented 2 years ago

Codecov Report

Merging #1712 (0a9e38e) into main (d501aa4) will decrease coverage by 8.42%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1712      +/-   ##
==========================================
- Coverage   71.89%   63.47%   -8.43%     
==========================================
  Files          40       39       -1     
  Lines        1409     1180     -229     
==========================================
- Hits         1013      749     -264     
- Misses        396      431      +35     
Impacted Files Coverage Δ
lib/not_at_origin_validator.rb 85.71% <100.00%> (ø)
app/models/export.rb 30.55% <0.00%> (-69.45%) :arrow_down:
app/controllers/export_controller.rb 47.27% <0.00%> (-43.64%) :arrow_down:
app/controllers/application_controller.rb 88.23% <0.00%> (-0.66%) :arrow_down:
app/models/map.rb 91.61% <0.00%> (-0.65%) :arrow_down:
app/models/warpable.rb 88.50% <0.00%> (-0.14%) :arrow_down:
app/helpers/application_helper.rb 50.00% <0.00%> (ø)
app/controllers/maps_controller.rb 86.95% <0.00%> (ø)
app/controllers/sessions_controller.rb 33.33% <0.00%> (+0.42%) :arrow_up:
jywarren commented 2 years ago

OK, we need 2.7.6 to get dockerfiles building, 2.7.3 wasn't tagged: https://hub.docker.com/_/ruby/?tab=tags&page=3

jywarren commented 2 years ago

OK, so we have a backported version of GDAL per https://github.com/publiclab/mapknitter/pull/489, so what about just removing GDAL completely?

jywarren commented 2 years ago

OK, hold onto your pants, removed GDAL

jywarren commented 2 years ago

got E: Package 'libappindicator3-1' has no installation candidate for apt install step in /lib/exporter-deps.sh

Trying this: https://github.com/UnchartedBull/OctoDash/issues/2589

jywarren commented 2 years ago

Realized change in deps for Debian 10: https://github.com/jgraph/drawio-desktop/issues/504#issuecomment-816807808

libappindicator3-1 switches to libayatana-appindicator -- and confirmed this worked!

jywarren commented 2 years ago

I see this in log:

I, [2022-03-22T08:00:11.480614 #191] INFO -- : [d99752cf-ae4c-4c8b-8b06-b7e62e858bf5] Sending event 05cf4684a84e40c3ad769cb9ecefc7e3 to Sentry

But no corresponding entry in Sentry?

jywarren commented 2 years ago

Ah, the above is an old sentry-raven event. We need to look for a recent one...

jywarren commented 2 years ago

Here it says it's OK to set the SENTRY_DSN in environment variables, which we're doing on stable/unstable: https://docs.sentry.io/platforms/ruby/configuration/options/#environment-variables

jywarren commented 2 years ago

This message on starting rails c:

W, [2022-05-25T15:27:59.366444 #1] WARN -- sentry: ** [Sentry] sentry-rails can't detect Rails.logger. it may be caused by misplacement of the SDK initialization code please make sure you place the Sentry.init block under theconfig/initializersfolder, e.g.config/initializers/sentry.rb``

jywarren commented 2 years ago

we had put it in config/application.rb, so i'm moving it!

jywarren commented 2 years ago

ok, so that didn't work -- now it says: sentry: [Sessions] Sessions won't be captured without a valid release

I think i need to add this workflow so it generates releases, and also use a PR so it gets triggered?

https://github.com/publiclab/plots2/blob/a0b090258beee49f043ae8e36af6c91146007fb8/.github/workflows/merge-pr-sentry-release.yml#L1-L20

jywarren commented 2 years ago

I believe now we added that workflow, we are connected, after also adding sentry secrets to https://github.com/publiclab/mapknitter/settings/secrets/actions

The workflow: https://github.com/publiclab/mapknitter/commit/4b00848e81e17380782bfdf475b3fd8a75134992

https://github.com/publiclab/mapknitter/runs/6596606213?check_suite_focus=true ran without error,

AND I don't see any issues in Sentry (haven't been able to trigger one) but I do see a staging environment listed in the dropdown!!!

jywarren commented 2 years ago

We can trigger a 500 at https://stable.mapknitter.org/maps/test-bcccfc71-64d1-4299-a8a1-9806a1c95ca7/edit (i deleted that map)

but i don't see it appearing on sentry...

jywarren commented 2 years ago

in the stable log:

I, [2022-05-25T17:04:29.459460 #795]  INFO -- : [SKYLIGHT] [5.3.2] Skylight agent enabled
D, [2022-05-25T17:04:31.220061 #795] DEBUG -- sentry: Sentry HTTP Transport will connect to https://sentry.io
D, [2022-05-25T17:04:31.220207 #795] DEBUG -- sentry: Initializing the background worker with 8 threads
D, [2022-05-25T17:04:31.220345 #795] DEBUG -- sentry: [Sessions] Sessions won't be captured without a valid release

do we need to redeploy now with the new code that somehow matches or comes later than the release we made?

jywarren commented 2 years ago

will try rebuilding stable

jywarren commented 2 years ago

Still getting D, [2022-05-25T17:19:27.142127 #1] DEBUG -- sentry: [Sessions] Sessions won't be captured without a valid release

jywarren commented 2 years ago

Corrected an environment name mismatch in https://github.com/publiclab/mapknitter/pull/1726 - mapknitter_stable is now also appearing in Sentry dropdown!

jywarren commented 2 years ago

OK, continuing to debug this here. We seem connected - the env mapknitter_stable shows up in sentry.io. But no events are shown. Going to try manually triggering an event on stable from commandline.

jywarren commented 2 years ago

Hmm. The DSN is correctly recorded but we aren't seeing the reports appear. Strange.

irb(main):007:0> ENV['SENTRY_DSN']
=> "https://d51bdd76485d488c8104efdbcb109ba2@o239675.ingest.sentry.io/1726938"  
irb(main):006:0> Sentry.capture_message("tests", extra: { debug: true })
=> nil

Yet setup is very very similar to plots2:

https://github.com/publiclab/mapknitter/blob/5204c4021cb61162aedcb7a96c899f1f63e9e8a5/config/initializers/sentry.rb#L1-L16

(the above is after https://github.com/publiclab/mapknitter/pull/1723 moved the file, which wasn't necessary in plots2 but just in case)

https://github.com/publiclab/plots2/blob/b4cd0a35a026f3c1c13825093069e48c29f30804/config/application.rb#L99-L126

and pretty close to the example:

https://github.com/getsentry/sentry-ruby/blob/c237974b185e51296542237cbca7f9c1ba1a7eb6/sentry-rails/examples/rails-6.0/config/initializers/sentry.rb#L1-L14

jywarren commented 2 years ago

I posted this to https://github.com/getsentry/sentry/discussions/35614 to ask for assistance.

jywarren commented 2 years ago

For what it's worth, i got the same lack of issue when i tried creating a message from the console in plots2, where sentry is working fine:

[2] pry(main)> ENV['SENTRY_DSN']
=> "https://0490297edae647b3bd935bdb4658da54@o239675.ingest.sentry.io/1410626"
[3] pry(main)> Sentry.capture_message("test", extra: { debug: true })
=> nil

So I must be doing something wrong with that means of testing, and it's not necessarily a confirmation of what's wrong with the rest of the config ☹️