From a367ae999a4e1cf73395e65ec383fd11f83888d5 Mon Sep 17 00:00:00 2001
From: Reasno <guxi99@gmail.com>
Date: Thu, 5 Nov 2020 12:15:17 +0800
Subject: [PATCH] fix: safely finish spans in case of exception

also make tracing aspects usable in CLI mode.
---
 src/tracer/src/Aspect/HttpClientAspect.php      | 12 ++++++++----
 src/tracer/src/Aspect/JsonRpcAspect.php         | 15 +++++++++------
 src/tracer/src/Aspect/MethodAspect.php          |  7 +++++--
 src/tracer/src/Aspect/RedisAspect.php           |  9 ++++++---
 src/tracer/src/Aspect/TraceAnnotationAspect.php |  7 +++++--
 src/tracer/src/SpanStarter.php                  |  7 ++++++-
 6 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/src/tracer/src/Aspect/HttpClientAspect.php b/src/tracer/src/Aspect/HttpClientAspect.php
index a3c67b5f5..805f1deac 100644
--- a/src/tracer/src/Aspect/HttpClientAspect.php
+++ b/src/tracer/src/Aspect/HttpClientAspect.php
@@ -91,11 +91,15 @@ class HttpClientAspect implements AroundInterface
         );
         $options['headers'] = array_replace($options['headers'] ?? [], $appendHeaders);
         $proceedingJoinPoint->arguments['keys']['options'] = $options;
-        $result = $proceedingJoinPoint->process();
-        if ($result instanceof ResponseInterface) {
-            $span->setTag($this->spanTagManager->get('http_client', 'http.status_code'), $result->getStatusCode());
+
+        try {
+            $result = $proceedingJoinPoint->process();
+            if ($result instanceof ResponseInterface) {
+                $span->setTag($this->spanTagManager->get('http_client', 'http.status_code'), $result->getStatusCode());
+            }
+        } finally {
+            $span->finish();
         }
-        $span->finish();
         return $result;
     }
 }
diff --git a/src/tracer/src/Aspect/JsonRpcAspect.php b/src/tracer/src/Aspect/JsonRpcAspect.php
index 9a28ed4e0..218e5da23 100644
--- a/src/tracer/src/Aspect/JsonRpcAspect.php
+++ b/src/tracer/src/Aspect/JsonRpcAspect.php
@@ -90,13 +90,16 @@ class JsonRpcAspect implements AroundInterface
         }
 
         if ($proceedingJoinPoint->methodName === 'send') {
-            $result = $proceedingJoinPoint->process();
-            /** @var Span $span */
-            if ($span = CT::get('tracer.span.' . static::class)) {
-                if ($this->spanTagManager->has('rpc', 'status')) {
-                    $span->setTag($this->spanTagManager->get('rpc', 'status'), isset($result['result']) ? 'OK' : 'Failed');
+            try {
+                $result = $proceedingJoinPoint->process();
+            } finally {
+                /** @var Span $span */
+                if ($span = CT::get('tracer.span.' . static::class)) {
+                    if ($this->spanTagManager->has('rpc', 'status')) {
+                        $span->setTag($this->spanTagManager->get('rpc', 'status'), isset($result['result']) ? 'OK' : 'Failed');
+                    }
+                    $span->finish();
                 }
-                $span->finish();
             }
 
             return $result;
diff --git a/src/tracer/src/Aspect/MethodAspect.php b/src/tracer/src/Aspect/MethodAspect.php
index d51fa83de..18df9f8d2 100644
--- a/src/tracer/src/Aspect/MethodAspect.php
+++ b/src/tracer/src/Aspect/MethodAspect.php
@@ -58,8 +58,11 @@ class MethodAspect extends AbstractAspect
 
         $key = $proceedingJoinPoint->className . '::' . $proceedingJoinPoint->methodName;
         $span = $this->startSpan($key);
-        $result = $proceedingJoinPoint->process();
-        $span->finish();
+        try {
+            $result = $proceedingJoinPoint->process();
+        } finally {
+            $span->finish();
+        }
         return $result;
     }
 }
diff --git a/src/tracer/src/Aspect/RedisAspect.php b/src/tracer/src/Aspect/RedisAspect.php
index 514422114..3c22714dd 100644
--- a/src/tracer/src/Aspect/RedisAspect.php
+++ b/src/tracer/src/Aspect/RedisAspect.php
@@ -74,9 +74,12 @@ class RedisAspect implements AroundInterface
         $arguments = $proceedingJoinPoint->arguments['keys'];
         $span = $this->startSpan('Redis' . '::' . $arguments['name']);
         $span->setTag($this->spanTagManager->get('redis', 'arguments'), json_encode($arguments['arguments']));
-        $result = $proceedingJoinPoint->process();
-        $span->setTag($this->spanTagManager->get('redis', 'result'), json_encode($result));
-        $span->finish();
+        try {
+            $result = $proceedingJoinPoint->process();
+            $span->setTag($this->spanTagManager->get('redis', 'result'), json_encode($result));
+        } finally {
+            $span->finish();
+        }
         return $result;
     }
 }
diff --git a/src/tracer/src/Aspect/TraceAnnotationAspect.php b/src/tracer/src/Aspect/TraceAnnotationAspect.php
index fd9eee666..46ea1829d 100644
--- a/src/tracer/src/Aspect/TraceAnnotationAspect.php
+++ b/src/tracer/src/Aspect/TraceAnnotationAspect.php
@@ -58,8 +58,11 @@ class TraceAnnotationAspect implements AroundInterface
         }
         $span = $this->startSpan($name);
         $span->setTag($tag, $source);
-        $result = $proceedingJoinPoint->process();
-        $span->finish();
+        try {
+            $result = $proceedingJoinPoint->process();
+        } finally {
+            $span->finish();
+        }
         return $result;
     }
 }
diff --git a/src/tracer/src/SpanStarter.php b/src/tracer/src/SpanStarter.php
index f22cf6c12..68ea1313d 100644
--- a/src/tracer/src/SpanStarter.php
+++ b/src/tracer/src/SpanStarter.php
@@ -36,7 +36,12 @@ trait SpanStarter
             /** @var ServerRequestInterface $request */
             $request = Context::get(ServerRequestInterface::class);
             if (! $request instanceof ServerRequestInterface) {
-                throw new \RuntimeException('ServerRequest object missing.');
+                // If the request object is absent, we are probably in a commandline context.
+                // Throwing an exception is unnecessary.
+                $root = $this->tracer->startSpan($name, $option);
+                $root->setTag(SPAN_KIND, $kind);
+                Context::set('tracer.root', $root);
+                return $root;
             }
             $carrier = array_map(function ($header) {
                 return $header[0];