[GitHub] [skywalking-nodejs] tom-pytel commented on a change in pull request #9: [WIP] express plugin, make spans actually work

[GitBox <gi...@apache.org>]

tom-pytel commented on a change in pull request #9:
URL: https://github.com/apache/skywalking-nodejs/pull/9#discussion_r536822567



##########
File path: src/trace/context/SpanContext.ts
##########
@@ -57,39 +58,62 @@ export default class SpanContext implements Context {
       });
     }
 
-    ContextManager.spansDup();
+    const spans = ContextManager.spansDup();
+    const parent = spans[spans.length - 1];
 
-    const span = new EntrySpan({
-      id: this.spanId++,
-      parentId: this.parentId,
-      context: this,
-      operation,
-    });
+    let span;
 
-    if (carrier && carrier.isValid()) {
-      span.inject(carrier);
+    if (parent && parent.type === SpanType.ENTRY) {
+      span = parent;
+      parent.operation = operation;
+
+    } else {
+      span = new EntrySpan({
+        id: this.spanId++,
+        parentId: this.parentId,
+        context: this,
+        operation,
+      });
+
+      if (carrier && carrier.isValid()) {
+        span.extract(carrier);
+      }
     }
 
     return span;
   }
 
-  newExitSpan(operation: string, peer: string): Span {
+  newExitSpan(operation: string, peer: string, carrier?: ContextCarrier): Span {
     if (logger.isDebugEnabled()) {
       logger.debug('Creating exit span', {
         parentId: this.parentId,
         executionAsyncId: executionAsyncId(),
       });
     }
 
-    ContextManager.spansDup();
+    const spans = ContextManager.spansDup();
+    const parent = spans[spans.length - 1];
 
-    return new ExitSpan({
-      id: this.spanId++,
-      parentId: this.parentId,
-      context: this,
-      peer,
-      operation,
-    });
+    let span;
+
+    if (parent && parent.type === SpanType.EXIT) {
+      span = parent;
+
+    } else {
+      span = new ExitSpan({
+        id: this.spanId++,
+        parentId: this.parentId,
+        context: this,
+        peer,
+        operation,
+      });
+
+      // if (carrier && carrier.isValid()) {  // is this right?
+      //   Object.assign(carrier, span.inject());

Review comment:
       Forgot this detail, what is the proper way to do the injection in `SpanContext.newExitSpan()`?




----------------------------------------------------------------
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