Skip to content

Add rollbar#871

Open
nikolai-b wants to merge 1 commit intomasterfrom
error-logging
Open

Add rollbar#871
nikolai-b wants to merge 1 commit intomasterfrom
error-logging

Conversation

@nikolai-b
Copy link
Copy Markdown
Contributor

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.

Adds error logging in Rollbar
@Robinlovelace
Copy link
Copy Markdown
Member

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 👍 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
Copy link
Copy Markdown
Contributor Author

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
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Contributor Author

nikolai-b commented Mar 29, 2021 via email

@Robinlovelace
Copy link
Copy Markdown
Member

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 👍 now.

@Robinlovelace
Copy link
Copy Markdown
Member

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

@nikolai-b
Copy link
Copy Markdown
Contributor Author

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
Copy link
Copy Markdown
Member

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
Copy link
Copy Markdown
Contributor Author

nikolai-b commented Mar 29, 2021

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
Copy link
Copy Markdown
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants