zipkin-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-zipkin] adriancole commented on a change in pull request #2552: Fixes logging and metrics for collectors
Date Wed, 01 May 2019 04:42:44 GMT
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_r280004552
 
 

 ##########
 File path: zipkin-collector/core/src/main/java/zipkin2/collector/Collector.java
 ##########
 @@ -178,60 +198,56 @@ void warn(String message, Throwable e) {
     return sampled;
   }
 
-  Callback<Void> acceptSpansCallback(final List<Span> spans) {
+  Callback<Void> storeSpansCallback(final List<Span> spans) {
     return new Callback<Void>() {
-      @Override
-      public void onSuccess(Void value) {}
+      @Override public void onSuccess(Void value) {
+      }
 
-      @Override
-      public void onError(Throwable t) {
-        errorStoringSpans(spans, t);
+      @Override public void onError(Throwable t) {
+        handleStorageError(spans, t, NOOP_CALLBACK);
       }
 
-      @Override
-      public String toString() {
-        return appendSpanIds(spans, new StringBuilder("AcceptSpans(")).append(")").toString();
+      @Override public String toString() {
+        return appendSpanIds(spans, new StringBuilder("StoreSpans(")) + ")";
       }
     };
   }
 
-  RuntimeException errorReading(Throwable e) {
-    return errorReading("Cannot decode spans", e);
-  }
-
-  RuntimeException errorReading(String message, Throwable e) {
+  void handleDecodeError(Throwable e, Callback<Void> callback) {
     metrics.incrementMessagesDropped();
-    return doError(message, e);
+    handleError(e, "Cannot decode spans"::toString, callback);
   }
 
   /**
    * When storing spans, an exception can be raised before or after the fact. This adds context
of
    * span ids to give logs more relevance.
    */
-  RuntimeException errorStoringSpans(List<Span> spans, Throwable e) {
+  void handleStorageError(List<Span> spans, Throwable e, Callback<Void> callback)
{
     metrics.incrementSpansDropped(spans.size());
     // The exception could be related to a span being huge. Instead of filling logs,
     // print trace id, span id pairs
-    StringBuilder msg = appendSpanIds(spans, new StringBuilder("Cannot store spans "));
-    return doError(msg.toString(), e);
+    handleError(e, () -> appendSpanIds(spans, new StringBuilder("Cannot store spans ")),
callback);
   }
 
-  RuntimeException doError(String message, Throwable e) {
+  void handleError(Throwable e, Supplier<String> defaultLogMessage, Callback<Void>
callback) {
+    propagateIfFatal(e);
+    callback.onError(e);
+    if (!logger.isLoggable(FINE)) return;
+
     String error = e.getMessage() != null ? e.getMessage() : "";
-    if (e instanceof RuntimeException
-        && (error.startsWith("Malformed") || error.startsWith("Truncated"))) {
-      if (shouldWarn()) warn(error, e);
-      return (RuntimeException) e;
-    } else {
-      if (shouldWarn()) {
-        message = format("%s due to %s(%s)", message, e.getClass().getSimpleName(), error);
-        warn(message, e);
-      }
-      return new RuntimeException(message, e);
+    // We have specific code that customizes log messages. Use this when the case.
 
 Review comment:
   PS I really don't think this special casing is helpful anymore

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message