-
Notifications
You must be signed in to change notification settings - Fork 109
Improve tracing and logging for write errors. #740
Conversation
This adds error flags and exception messages to the spans for metadata, suggest, and bigtable writes when the chain of futures fails. The same exceptions are also logged. Previously some exceptions, such as grpc errors, could be masked by sl4j settings. The trace for a metric write was cleaned up to remove several intermediary spans. These spans did not have useful information and had no branching paths, so they were just clutter in the overall trace. Closes #724.
Codecov Report
@@ Coverage Diff @@
## master #740 +/- ##
============================================
+ Coverage 54.82% 55.02% +0.20%
- Complexity 3105 3130 +25
============================================
Files 742 743 +1
Lines 20191 20248 +57
Branches 1313 1316 +3
============================================
+ Hits 11070 11142 +72
+ Misses 8653 8633 -20
- Partials 468 473 +5
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -82,7 +83,7 @@ | |||
* See: https://github.com/spotify/heroic/pull/208 */ | |||
protected boolean brokenSegmentsPr208 = false; | |||
protected boolean eventSupport = false; | |||
protected Optional<Integer> maxBatchSize = Optional.empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the replacement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just doing some cleanup as I went through the code. While I generally like Optional, in this case its only purpose is a single assume statement in the test, so removing it makes things a tiny bit easier to read/less verbose.
This adds error flags and exception messages to the spans for
metadata, suggest, and bigtable writes when the chain of futures
fails. The same exceptions are also logged. Previously some
exceptions, such as grpc errors, could be masked by sl4j settings.
The trace for a metric write was cleaned up to remove several
intermediary spans. These spans did not have useful information and
had no branching paths, so they were just clutter in the overall
trace.
Closes #724.