log 5** exceptions to sentry #58
Conversation
jsotobroad
changed the title
| if (sentryConfiguration.dsn().isEmpty()) { | ||
| logger.info("No Sentry DSN found. Starting up without it."); | ||
| } else { | ||
| logger.info("Sentry DSN found. 5xx errors will be sent to Sentry."); |
There was a problem hiding this comment.
nice, i really like these logs.
| @@ -128,9 +129,14 @@ public ResponseEntity<ApiErrorReport> catchallHandler(Exception ex) { | |||
|
|
|||
| private ResponseEntity<ApiErrorReport> buildApiErrorReport( | |||
| @NotNull Throwable ex, HttpStatus statusCode, List<String> causes) { | |||
| // only logging 5** errors to sentry, should we log more/every error? Other services seem to do | |||
There was a problem hiding this comment.
nit: we can probably remove the question here
There was a problem hiding this comment.
yup yup this was for you to see it :). will remove
| @@ -11,7 +11,9 @@ | |||
| @SpringBootTest( | |||
| properties = { | |||
| "spring.main.lazy-initialization=true", | |||
| "datasource.testWithEmbeddedDatabase=false" | |||
| "datasource.testWithEmbeddedDatabase=false", | |||
| "pipelines.sentry.dsn=" // piggyback on this test which already supplies properties to test | |||
There was a problem hiding this comment.
maybe we can chat about this - how did you decide on this test class to add this to?
There was a problem hiding this comment.
i just chose a test that overwrote some properties in the test assplication.conf. I didnt feel like it was worth creating a whole new class to test this (i would rather remove it and not test than do that).
| @@ -48,3 +48,6 @@ cbas: | |||
| pipelines: | |||
| ingress: | |||
| domainName: "some-tsps-domain.com" | |||
| sentry: | |||
| dsn: https://public@sentry.example.com/1 | |||
There was a problem hiding this comment.
do these get used anywhere?
There was a problem hiding this comment.
they get used when sentry is instantiated i.e. it has to look like this pattern otherwise it'll throw an exception
|
Description
To satisy the requirement for Production Readiness section M8, we need to send our exceptions to sentry for monitoring puproses. The work was already done to set up our dashboard and make sentry available to us. This PR introduces sentry to the service and logs all 5** exceptions we generate to sentry for monitoring
This change has to go along with https://github.com/broadinstitute/terra-helmfile/pull/5103
Jira Ticket
https://broadworkbench.atlassian.net/browse/TSPS-172