getodk / aggregate

ODK Aggregate is a Java server that stores, analyzes, and presents survey data collected using ODK Collect. Contribute and make the world a better place! ✨🗄✨
https://docs.opendatakit.org/aggregate-intro/
Other
74 stars 228 forks source link

Sentry integration #206

Open ggalmazor opened 6 years ago

ggalmazor commented 6 years ago

We can add support for Sentry by following the instructions at: https://docs.sentry.io/clients/java/modules/jul/

Basically, it consists on adding this conf to logging.properties:

# Enable the Console and Sentry handlers
handlers=java.util.logging.ConsoleHandler,io.sentry.jul.SentryHandler

# Override the Sentry handler log level to WARNING
io.sentry.jul.SentryHandler.level=WARNING

To configure Sentry (DSN and other settings), probably the most convenient way to do it would be to add a sentry.properties file to the classpath.

To make the integration with Sentry useful, we should centralize crash reports of all online Aggregate instances. We need to identify each crash report at least with the Aggregate build version that has produced it, but the following information would also be very useful:

This information would not only be used to categorize/filter crash reports but also to contact the owners of the instance to ask for collaboration or suggest actions to solve a particular situation.

Some considerations:

Pinging @yanokwa and @lognaturel to the conversation

yanokwa commented 6 years ago

Couple of questions come to mind.

Can you confirm that the only change to Aggregate would be to the logging.properties? There'd be no other code change and the other logging (Tomcat, GAE) would work like normal?

What do you mean by the Aggregate dashboard? Is it the GAE dashboard or something else? Why isn't the FQDN enough?

As to the email of the administrator, that's the kind of thing that we would need to ask for in the installer.

ggalmazor commented 6 years ago

Can you confirm that the only change to Aggregate would be to the logging.properties

I'll do a small test to confirm that, but that's what's promised on the Sentry docs...

What do you mean by the Aggregate dashboard?

You're right, there's no such thing :P I was referring to the submission list (it's the first page you see when accessing Aggregate)

I think we would need this because we don't know where it's really deployed in the IP/FQDN (if it's a Tomcat it can be any path) but I recon it doesn't feel like something critical right now.

email of the administrator

I'm in doubt with this one. I don't know how important will be to be able to eventually contact ppl after we notice some error on Sentry. Also, changing the installer to ask this email is no fun at all :P

yanokwa commented 6 years ago

I think IP/FQDN is enough to group the logs. Doesn't feel important to get any more of the URL for now.

I say we wait to see what kinda crashes we get and I bet we can fix most of those without requiring a lot of feedback from users. Once we see that, we can then add a "Contact ODK" message in errors that show up in the UI. If that gets no traction, then we can ask for emails. Basically, I want to minimize the stuff we collect until we really need it.

ggalmazor commented 6 years ago

OK, I'll work on the IP/FQDN.