adriancole commented on a change in pull request #2552: Fixes logging and
metrics for collectors
URL: https://github.com/apache/incubator-zipkin/pull/2552#discussion_r280004163
##########
File path: zipkin-collector/core/src/main/java/zipkin2/collector/Collector.java
##########
@@ -110,34 +119,46 @@ public void accept(List<Span> spans, Callback<Void>
callback) {
}
try {
+ // adding a separate callback intentionally decouples collection from
storage
record(sampled, acceptSpansCallback(sampled));
callback.onSuccess(null);
- } catch (RuntimeException e) {
- callback.onError(errorStoringSpans(sampled, e));
- return;
+ } catch (RuntimeException | Error e) {
+ handleStorageError(spans, e, callback);
Review comment:
sorry wires crossed. So, In any case I think we'll still need to invoke
either callback.onSuccess or onError.
So I guess you are asking if the code added to separate out which of the
failures into separate log prefixes would be worth it..
ex instead of just "storage error" a separate if statement and log message
for `storage.spanConsumer()` vs `spanConsumer.accept()` vs `call.enqueue()`.
I can understand that in the logs you might not have the stack trace (though
the caller may), so this could be a way to know the difference, if the message
wasn't already clear what happened..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
