npct / pct-shiny

The Shiny map for Local Authorites
GNU Affero General Public License v3.0
24 stars 14 forks source link

Add rollbar #871

Open nikolai-b opened 3 years ago

nikolai-b commented 3 years ago

Adds error logging in Rollbar. After adding this to another project I realised it wasn't much work in R. The other logging I tried to add never seemed to work as the R server logs weren't in quite the correct format. I've added the Renviron and rollbar package to all the servers in preparation.

Robinlovelace commented 3 years ago

The idea being that any error messages emanating from R inside the app will get reported, right? I've glanced over the minimal documentation in the rollbar R package. One thing to note: the package was last updated 5 years ago so doesn't seem to be actively maintained. If it works to allow named people to see the logs it's a :+1: from me although I cannot at this stage see the advantage compared with the output of /var/log/shiny-server/*.log files. Good article on debugging shiny apps: https://shiny.rstudio.com/articles/debugging.html

On a different note while searching for that I came across this, may also log errors but not sure: https://github.com/dreamRs/shinylogs

Looks fine but a little more info would be useful. Is it the ease with which we can see logs on the rollbar website, which allows authentication via GitHub? I recall you set up a similar system already but cannot recall - did it have 'dog' in the name?

nikolai-b commented 3 years ago

I agree. I tried to get /var/log/shiny-server/*.log files to be pushed somewhere a year or two ago but I failed. Didn't want to put much effort into it. The issue with the logs being on the server is you have to actively log in to see problems. A service like rollbar will send notifications (which can be customised etc.) so proactively alerting us. If we had the full logs sent somewhere we'd still want to set up custom alerts as most log lines shouldn't trigger a notification. I'll look at the other package but it is a full error handeling Saas that I wanted. Thanks for checking the package age btw

Robinlovelace commented 3 years ago

To clarify, what is the next step on this that you're suggesting @nikolai-b ? Package age alone doesn't need to be a blocker but wanted to explore options before reviewing this.

nikolai-b commented 3 years ago

If you want to explore options we can leave it sitting for a week or two. No rush. Otherwise it's good to go. Even if we do deploy it we can always do something better, I'm not set on rollbar. What I do want is to be alerted if the code errors.

Robinlovelace commented 3 years ago

I think there is no chance of issues with this and generally think moving things forward is good. Given low risk and minimal change to code base will review with :+1: now.

Robinlovelace commented 3 years ago

Does rollbar need to be added in the install.packages() bit also?

nikolai-b commented 3 years ago

Does rollbar need to be added in the install.packages() bit also?

I did it before as I wanted the installed packages to raise an error that will be caught (assuming rollbar works) but we could probably rejig it if you prefer.

Robinlovelace commented 3 years ago

I did it before as I wanted the installed packages to raise an error that will be caught (assuming rollbar works) but we could probably rejig it if you prefer.

Was wondering how to ensure an error message is generated. But not sure that

library(rollbar)
#> Error in library(rollbar): there is no package called 'rollbar'

Created on 2021-03-29 by the reprex package (v1.0.0)

Is a particularly useful error message if we're looking to test the package!

nikolai-b commented 3 years ago

You'll need to install the package and use the correct Renviron with our ROLLBAR_ACCESS_TOKEN and R_ENV if you want to test the error capturing. I'll add you to the rollbar project. I purposely didn't add the package to either available_locally_pkgs or must_be_installed_pkgs in case code in there has errors. For error handling it made sense to have that code run first. This is likely to need some tweaking as when I run shiny from Rstudio it is in interactive mode so rollbar doesn't handle errors but we can tweak how rollbar attaches if your happy for it to be included.

Robinlovelace commented 3 years ago

This is good to go I think @nikolai-b.