DataBiosphere / azul

Metadata indexer and query service used for AnVIL, HCA, LungMAP, and CGP
Apache License 2.0
7 stars 2 forks source link

Detect duplicate subscriptions #823

Closed jessebrennan closed 3 years ago

jessebrennan commented 5 years ago

It could be useful for debugging if there is an alarm or warning in the case where there are duplicate subscriptions to DSS.

┆Issue is synchronized with this Jira Story ┆Project Name: azul ┆Issue Number: AZUL-510

jessebrennan commented 5 years ago

According to the Airbrake docs if an issue is marked resolved, that won't prevent the issue from showing up again (it just removes the cruft from your immediate view.

Also from the docs if the error is of severity warning it will be logged, but it won't create an email notification. One possible solution would be to log at lever error when duplicate notifications are detected.

hannes-ucsc commented 5 years ago

Seems like Airbrake (or at least the free plan) is currently insufficient for what we are trying to achieve. Lets go ahead with your proposition to log that particular at ERROR severity and call it a day.

jessebrennan commented 5 years ago

I agree (although interestingly, we have a paid plan).

On Wed, Jul 31, 2019 at 4:02 PM Hannes Schmidt notifications@github.com wrote:

Seems like Airbrake (or at least the free plan) is currently insufficient for what we are trying to achieve. Lets go ahead with your proposition to log that particular at ERROR severity and call it a day.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/DataBiosphere/azul/issues/823?email_source=notifications&email_token=ACR4ZFGN77PLQCVXVVVB2ADQCIKYJA5CNFSM4HAKP5N2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3IZ3LI#issuecomment-517053869, or mute the thread https://github.com/notifications/unsubscribe-auth/ACR4ZFEBWOWTDZQXTVMPGLDQCIKYJANCNFSM4HAKP5NQ .

jessebrennan commented 5 years ago

Since duplicate notifications happen during integration tests (intentionally) This error would be raised every time integration tests are run which is impractical. Instead we need a measure to check for duplicate notifications.

jessebrennan commented 5 years ago

A decision was made during standup to ignore this issues unless it's possible to receive warning level messages from Airbrake. I'm contacting Matt to see about this possibility.

jessebrennan commented 5 years ago

It's possible to set the error severity with Airbrake, but it all has to be done manually by examining the error. Originally, @hannes-ucsc and I were under the impression that Airbrake figures out which exceptions should be logged. This was incorrect. Instead, any log message containing

'stacktrace',
'traceback',
'error',
'exception',
'critical'

(this comes from https://github.com/HumanCellAtlas/logs/blob/master/scripts/secret_templates.py#L46)

will be automatically sent to Airbrake. So while it IS possible to add specify the severity, it would be complicated and involve parsing the logged messages.

If we wanted to make full use of Airbrake's features, we would be better off integrating Airbrake into Azul directly which seems pretty straight forward, and would isolate us from the noise of DSS errors, etc. While that's a possibility worth considering, @hannes-ucsc I think this is probably ok to close.

hannes-ucsc commented 5 years ago

I may have an idea of how we could make this change relatively easily. Let's talk. Hint: turn airbrake_whitelisted_log_message_terms into a dict mapping an Airbrake severity into list of terms.

jessebrennan commented 5 years ago

The current proposed solution is to add Airbrake integration directly into Azul via https://github.com/airbrake/pybrake. This allows easier and more informative errors to be logged. Also changes would need to be made to the https://github.com/HumanCellAtlas/logs/blob/master/apps/firehose_to_es_processor/lib/airbrake_notifier.py to prevent Azul notifications from being logged twice.

theathorn commented 3 years ago

Obsolete DCP/1 issue.