Sentry Integration in Release Monitor #45
Conversation
Codecov Report
@@ Coverage Diff @@
## master #45 +/- ##
==========================================
+ Coverage 83.33% 84.21% +0.87%
==========================================
Files 2 2
Lines 126 133 +7
==========================================
+ Hits 105 112 +7
Misses 21 21
Continue to review full report at Codecov.
|
|
@animuk Your image is available in the registry: |
| # if the "title" does not exist, catch the error and return nothing | ||
| logger.error('Titles does not exist : {e}'.format(e=str(e))) |
There was a problem hiding this comment.
What is the point of logging these errors here?
There was a problem hiding this comment.
And by here I mean in the whole file :)
There was a problem hiding this comment.
@msrb As I mentioned below, in case upstream format change, those logger.errors are beneficial to the developers to correct their code displayed in Sentry. If you think otherwise, we can make those into logger.info so that Sentry will not create event for that.
|
@animuk Handled exceptions are something that we use to control the flow of a program. They are expected to happen a therefore not worth logging into Sentry. Log only those failures which are fatal or unexpected. |
|
@msehnout The purpose of Sentry integration is to send error events as a notification due to some internal bug or formatting error, not necessarily only fatal or unexpected error needs to be there in portal. I also understand that in release monitor few areas, handled exception is used to control the flow, may not be suitable for sentry logging. |
|
@animuk internal bug will result in unhandled exception, thus fatal error. I am not sure what you mean by formatting error, but if you mean formatting of the code, then code with wrong indentation won't even start. |
|
@msehnout Lets assume, the possibility, the input JSON any particular key format from the Upstream feed has been changed in future or some key has been removed. Now, in that scenario, the service will return null every time. now, this is a part of handled exception. but, developers also should be aware of that change that ingestion is not happening due to that. |
|
@animuk fair point, it would be useful to know if the upstream format changed. |
No description provided.