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

[frontend] Always log CheckEventBlobSizeLimit violations #6183

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

3vilhamster
Copy link
Contributor

@3vilhamster 3vilhamster commented Jul 22, 2024

What changed?
Always log details on CheckEventBlobSizeLimit violations and failWorkflowIfBlobSizeExceedsLimit.

Why?
It is the only source that can point domain owners to specific workflow failures. Metrics do not have a workflow-id dimension, so we rely on logs to find exact workflows.
These operations lead to failure of either a workflow or request (in case of query) or data truncation - in case of activity failure result. Having too many of these violations is unexpected, and it should be easy to point customers to specific workflows.

How did you test it?
unit tests

Potential risks
If there are customers that have a lot of workflows with failures, we can bloat logs.
However, even in Uber's case, these violations are rare - since they are failing workflows.

Release notes

Documentation Changes

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.77%. Comparing base (95ba44c) to head (1f1b271).

Additional details and impacted files
Files Coverage Δ
service/frontend/api/handler.go 65.27% <100.00%> (ø)
service/history/decision/handler.go 94.93% <100.00%> (ø)

... and 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 95ba44c...1f1b271. Read the comment docs.

@3vilhamster 3vilhamster force-pushed the always-log-blob-size-violation branch from 949f948 to 1f1b271 Compare July 24, 2024 08:16
Copy link
Contributor

@jakobht jakobht left a comment

Choose a reason for hiding this comment

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

Looks good. Maybe we could do something in the client? But this at least improves the situation.

@3vilhamster 3vilhamster merged commit b85c66e into uber:master Jul 24, 2024
20 checks passed
@3vilhamster 3vilhamster deleted the always-log-blob-size-violation branch July 24, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants