Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DTA-1034] Integrate BugSnag error monitoring in the Payment Service #6

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

AdolfoLopezSB
Copy link

@AdolfoLopezSB AdolfoLopezSB commented Dec 16, 2024

Changes

Installed Bugnsag Error Monitoring into the payment service.
DTA-1034


module.exports.charge = async request => {
const span = tracer.startSpan('charge');

await OpenFeature.setProviderAndWait(flagProvider);
if (await OpenFeature.getClient().getBooleanValue("paymentServiceFailure", false)) {
Bugsnag.notify(new Error("PaymentService Fail Feature Flag Enabled"));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bugsnag by default should report unhandled errors. If you remove this line, does it still send an error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't manage to make it work without it. But having a look into the documentation, I understood that only unhandled exceptions and unhandled promises would be reported, so I assumed that it's the expected behaviour.
image

Copy link

@LucjanMichalowski LucjanMichalowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

A note, that before merging and deploying it, we need to add the PAYMENT_SERVICE_BUGSNAG_API_KEY to GitHub secrets.

@@ -439,6 +439,7 @@ services:
- "${PAYMENT_SERVICE_PORT}"
environment:
- PAYMENT_SERVICE_PORT
- PAYMENT_SERVICE_BUGSNAG_API_KEY

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on how the Terraform configuration looks, it seems that we need to keep the BUGSNAG_API_KEY in the app and add the mapping in here, so what should work is:

- BUGSNAG_API_KEY=${PAYMENT_SERVICE_BUGSNAG_API_KEY}

So in the .env file we will have PAYMENT_SERVICE_BUGSNAG_API_KEY and in the code in the app BUGSNAG_API_KEY.

Please let me know if it works.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the change and it works, I could still see the error in Bugnsag

@LucjanMichalowski LucjanMichalowski merged commit 850f027 into main Dec 18, 2024
25 of 29 checks passed
@LucjanMichalowski LucjanMichalowski deleted the adolfo/DTA-1034-Bugsnag-error-monitoring branch December 18, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants