Handle propagator names with colons, better error handling

cadence-workflow · Sep 15, 2020 · 500b131 · 500b131
1 parent ac2bff4
commit 500b131
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 22 deletions.
diff --git a/src/main/java/com/uber/cadence/context/OpenTracingContextPropagator.java b/src/main/java/com/uber/cadence/context/OpenTracingContextPropagator.java
@@ -82,14 +82,11 @@ public Object deserializeContext(Map<String, byte[]> context) {
 
   @Override
   public Object getCurrentContext() {
-    log.debug("Getting current context");
     Tracer currentTracer = GlobalTracer.get();
     Span currentSpan = currentTracer.scopeManager().activeSpan();
     if (currentSpan != null) {
       HashMapTextMap contextTextMap = new HashMapTextMap();
       currentTracer.inject(currentSpan.context(), Format.Builtin.TEXT_MAP, contextTextMap);
-      log.debug(
-          "Retrieving current span data as current context: " + contextTextMap.getBackingMap());
       return contextTextMap.getBackingMap();
     } else {
       return null;
@@ -98,11 +95,9 @@ public Object getCurrentContext() {
 
   @Override
   public void setCurrentContext(Object context) {
-    log.debug("Setting current context");
     Tracer currentTracer = GlobalTracer.get();
     Map<String, String> contextAsMap = (Map<String, String>) context;
     if (contextAsMap != null) {
-      log.debug("setting current context to " + contextAsMap);
       HashMapTextMap contextTextMap = new HashMapTextMap(contextAsMap);
       setCurrentOpenTracingSpanContext(
           currentTracer.extract(Format.Builtin.TEXT_MAP, contextTextMap));
@@ -111,7 +106,6 @@ public void setCurrentContext(Object context) {
 
   @Override
   public void setUp() {
-    log.debug("Starting a new opentracing span");
     Tracer openTracingTracer = GlobalTracer.get();
     Tracer.SpanBuilder builder =
         openTracingTracer
@@ -123,7 +117,6 @@ public void setUp() {
     }
 
     Span span = builder.start();
-    log.debug("New span: " + span);
     openTracingTracer.activateSpan(span);
     currentOpenTracingSpan.set(span);
     Scope scope = openTracingTracer.activateSpan(span);
@@ -133,24 +126,31 @@ public void setUp() {
   @Override
   public void onError(Throwable t) {
     Span span = currentOpenTracingSpan.get();
-    Tags.ERROR.set(span, true);
-    Map<String, Object> errorData = new HashMap<>();
-    errorData.put(Fields.EVENT, "error");
-    if (t != null) {
-      errorData.put(Fields.ERROR_OBJECT, t);
-      errorData.put(Fields.MESSAGE, t.getMessage());
-    }
-    span.log(errorData);
+    if (span != null) {
+      Tags.ERROR.set(span, true);
+      Map<String, Object> errorData = new HashMap<>();
+      errorData.put(Fields.EVENT, "error");
+      if (t != null) {
+        errorData.put(Fields.ERROR_OBJECT, t);
+        errorData.put(Fields.MESSAGE, t.getMessage());
+      }
+      span.log(errorData);
+    }
   }
 
   @Override
   public void finish(boolean successful) {
     Scope currentScope = currentOpenTracingScope.get();
     Span currentSpan = currentOpenTracingSpan.get();
 
-    log.debug("Closing currently open span " + currentSpan.context().toSpanId());
-    currentScope.close();
-    currentSpan.finish();
+    if (currentScope != null) {
+      currentScope.close();
+    }
+
+    if (currentSpan != null) {
+      currentSpan.finish();
+    }
+
     currentOpenTracingScope.remove();
     currentOpenTracingSpan.remove();
     currentOpenTracingSpanContext.remove();

diff --git a/src/main/java/com/uber/cadence/internal/replay/WorkflowContext.java b/src/main/java/com/uber/cadence/internal/replay/WorkflowContext.java
@@ -176,7 +176,8 @@ Map<String, Object> getPropagatedContexts() {
               .filter(e -> e.getKey().startsWith(propagator.getName()))
               .collect(
                   Collectors.toMap(
-                      e -> e.getKey().substring(e.getKey().indexOf(":") + 1), Map.Entry::getValue));
+                      e -> e.getKey().substring(propagator.getName().length() + 1),
+                      Map.Entry::getValue));
       contextData.put(propagator.getName(), propagator.deserializeContext(filteredData));
     }
 

diff --git a/src/main/java/com/uber/cadence/internal/worker/ActivityWorker.java b/src/main/java/com/uber/cadence/internal/worker/ActivityWorker.java
@@ -262,7 +262,7 @@ void propagateContext(PollForActivityTaskResponse response) {
                 .filter(e -> e.getKey().startsWith(propagator.getName()))
                 .collect(
                     Collectors.toMap(
-                        e -> e.getKey().substring(e.getKey().indexOf(":") + 1),
+                        e -> e.getKey().substring(propagator.getName().length() + 1),
                         Map.Entry::getValue));
         propagator.setCurrentContext(propagator.deserializeContext(filteredData));
       }

diff --git a/src/main/java/com/uber/cadence/internal/worker/LocalActivityWorker.java b/src/main/java/com/uber/cadence/internal/worker/LocalActivityWorker.java
@@ -283,7 +283,8 @@ private void restoreContext(Map<String, byte[]> context) {
               .filter(e -> e.getKey().startsWith(propagator.getName()))
               .collect(
                   Collectors.toMap(
-                      e -> e.getKey().substring(e.getKey().indexOf(":") + 1), Map.Entry::getValue));
+                      e -> e.getKey().substring(propagator.getName().length() + 1),
+                      Map.Entry::getValue));
       propagator.setCurrentContext(propagator.deserializeContext(filteredData));
     }
   }

diff --git a/src/test/java/com/uber/cadence/context/ContextTests.java b/src/test/java/com/uber/cadence/context/ContextTests.java
@@ -123,7 +123,7 @@ public static class TestContextPropagator implements ContextPropagator {
 
     @Override
     public String getName() {
-      return this.getClass().getName();
+      return "TestContextPropagator::withSomeColons";
     }
 
     @Override