Skip to content

Log more data on failed clean#2180

Merged
ChrisLovering merged 3 commits into
mainfrom
Log-more-data-on-failed-clean
May 29, 2022
Merged

Log more data on failed clean#2180
ChrisLovering merged 3 commits into
mainfrom
Log-more-data-on-failed-clean

Conversation

@ChrisLovering
Copy link
Copy Markdown
Member

This adds request data as a sentry breadcrumb when clean cog fails, so we can understand the data that is causing the site api to error.

This also updates the log string to use % based format strings, rather than a mix of f-string and % string which caused the %r to not work.

This also updates the log string to use % based format strings, rather than a mix of f-string and % string which caused the %r to not work.
This is to aid in the debugging on an issue we have encountered
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

Hard to test properly but looks fine and doesn't seem to break anything

@Xithrius Xithrius added a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority t: enhancement Changes or improvements to existing features s: needs review Author is waiting for someone to review and approve labels May 27, 2022
Comment on lines +80 to +85
add_breadcrumb(
category="api_error",
message=str(e),
level="error",
data=deletedmessage_set,
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's a discrepancy in the documentation of this function: https://getsentry.github.io/sentry-python/api.html?highlight=add_breadcrumb#sentry_sdk.add_breadcrumb vs https://docs.sentry.io/platforms/python/enriching-events/breadcrumbs/#manual-breadcrumbs

I don't know how to test this so let's just try and fix it if it doesn't work.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yea, I was following the latter. I guess we can just find out.

@ChrisLovering ChrisLovering enabled auto-merge May 29, 2022 08:35
@ChrisLovering ChrisLovering merged commit c29434b into main May 29, 2022
@ChrisLovering ChrisLovering deleted the Log-more-data-on-failed-clean branch May 29, 2022 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority s: needs review Author is waiting for someone to review and approve t: enhancement Changes or improvements to existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants