feat: Use timestamp of last span

HazAT · HazAT · commit 41b552c261fc · 2019-11-14T14:36:27.000+01:00
diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts
@@ -405,12 +405,12 @@ export class Hub implements HubInterface {
   /**
    * @inheritDoc
    */
-  public startSpan(spanOrSpanContext?: Span | SpanContext): Span {
+  public startSpan(spanOrSpanContext?: Span | SpanContext, forceNoChild: boolean = false): Span {
     const scope = this.getScope();
     const client = this.getClient();
     let span;
 
-    if (!isSpanInstance(spanOrSpanContext)) {
+    if (!isSpanInstance(spanOrSpanContext) && !forceNoChild) {
       if (scope) {
         const parentSpan = scope.getSpan();
         if (parentSpan) {
diff --git a/packages/hub/src/span.ts b/packages/hub/src/span.ts
@@ -261,13 +261,13 @@ export class Span implements SpanInterface, SpanContext {
   /**
    * Sets the finish timestamp on the current span
    */
-  public finish(endTimestamp?: number): string | undefined {
+  public finish(useLastSpanTimestamp: boolean = false): string | undefined {
     // This transaction is already finished, so we should not flush it again.
     if (this.timestamp !== undefined) {
       return undefined;
     }
 
-    this.timestamp = endTimestamp || timestampWithMs();
+    this.timestamp = timestampWithMs();
 
     if (this.spanRecorder === undefined) {
       return undefined;
@@ -291,6 +291,10 @@ export class Span implements SpanInterface, SpanContext {
     }
     const finishedSpans = this.spanRecorder ? this.spanRecorder.finishedSpans.filter(s => s !== this) : [];
 
+    if (useLastSpanTimestamp && finishedSpans.length > 0) {
+      this.timestamp = finishedSpans[finishedSpans.length - 1].timestamp;
+    }
+
     return this._hub.captureEvent({
       // TODO: Is this necessary? We already do store contextx in in applyToEvent,
       // so maybe we can move `getTraceContext` call there as well?
diff --git a/packages/integrations/src/transactionactivity.ts b/packages/integrations/src/transactionactivity.ts
@@ -1,5 +1,4 @@
 import { EventProcessor, Hub, Integration, Scope, Span, SpanContext } from '@sentry/types';
-import { timestampWithMs } from '@sentry/utils';
 
 /** JSDoc */
 interface TransactionActivityOptions {
@@ -11,7 +10,7 @@ interface TransactionActivityOptions {
   // onActivity?: (info) => {
   //   return info.type !== 'xhr' || !info.url.match(/zendesk/);
   // },
-  idleTimeout?: number;
+  idleTimeout: number;
 }
 
 /** JSDoc */
@@ -51,11 +50,12 @@ export class TransactionActivity implements Integration {
   /**
    * @inheritDoc
    */
-  public constructor(options?: TransactionActivityOptions) {
-    TransactionActivity._options = {
+  public constructor(
+    public readonly _options: TransactionActivityOptions = {
       idleTimeout: 500,
-      ...options,
-    };
+    },
+  ) {
+    TransactionActivity._options = _options;
   }
 
   /**
@@ -65,35 +65,16 @@ export class TransactionActivity implements Integration {
     TransactionActivity._getCurrentHub = getCurrentHub;
   }
 
-  /**
-   * Internal run loop that checks if activy is running
-   */
-  private static _watchActivity(): void {
-    const count = Object.keys(TransactionActivity._activities).length;
-    clearTimeout(TransactionActivity._debounce);
-    if (count > 0) {
-      setTimeout(() => {
-        TransactionActivity._watchActivity();
-      }, 10);
-    } else {
-      const timeout = TransactionActivity._options && TransactionActivity._options.idleTimeout;
-      TransactionActivity._debounce = (setTimeout(() => {
-        const active = TransactionActivity._activeTransaction;
-        if (active) {
-          active.finish(timestampWithMs() - (timeout || 0));
-        }
-      }, timeout) as any) as number; // TODO 500
-    }
-  }
-
   /**
    * Starts a Transaction waiting for activity idle to finish
    */
   public static startIdleTransaction(name: string, spanContext?: SpanContext): Span | undefined {
     const activeTransaction = TransactionActivity._activeTransaction;
 
     if (activeTransaction) {
-      // We need to finish any active transaction before starting a new
+      // If we already have an active transaction it means one of two things
+      // a) The user did rapid navigation changes and didn't wait until the transaction was finished
+      // b) A activity wasn't popped correctly and therefore the transaction is stalling
       activeTransaction.finish();
     }
 
@@ -107,17 +88,27 @@ export class TransactionActivity implements Integration {
       return undefined;
     }
 
-    const span = hub.startSpan({
-      ...spanContext,
-      transaction: name,
-    });
+    const span = hub.startSpan(
+      {
+        ...spanContext,
+        transaction: name,
+      },
+      true,
+    );
 
     TransactionActivity._activeTransaction = span;
 
     hub.configureScope((scope: Scope) => {
       scope.setSpan(span);
     });
 
+    // The reason we do this here is because of cached responses
+    // If we start and transaction without an activity it would never finish since there is no activity
+    const id = TransactionActivity.pushActivity('idleTransactionStarted');
+    setTimeout(() => {
+      TransactionActivity.popActivity(id);
+    }, (TransactionActivity._options && TransactionActivity._options.idleTimeout) || 100);
+
     return span;
   }
 
@@ -152,7 +143,6 @@ export class TransactionActivity implements Integration {
       };
     }
 
-    TransactionActivity._watchActivity();
     return TransactionActivity._currentIndex++;
   }
 
@@ -168,5 +158,18 @@ export class TransactionActivity implements Integration {
       // tslint:disable-next-line: no-dynamic-delete
       delete TransactionActivity._activities[id];
     }
+
+    const count = Object.keys(TransactionActivity._activities).length;
+    clearTimeout(TransactionActivity._debounce);
+
+    if (count === 0) {
+      const timeout = TransactionActivity._options && TransactionActivity._options.idleTimeout;
+      TransactionActivity._debounce = (setTimeout(() => {
+        const active = TransactionActivity._activeTransaction;
+        if (active) {
+          active.finish(true);
+        }
+      }, timeout) as any) as number; // TODO 500
+    }
   }
 }
diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts
@@ -179,5 +179,5 @@ export interface Hub {
    *
    * @param span Already constructed span which should be started or properties with which the span should be created
    */
-  startSpan(span?: Span | SpanContext): Span;
+  startSpan(span?: Span | SpanContext, forceNoChild?: boolean): Span;
 }
diff --git a/packages/types/src/span.ts b/packages/types/src/span.ts
@@ -1,7 +1,7 @@
 /** Span holding trace_id, span_id */
 export interface Span {
   /** Sets the finish timestamp on the current span and sends it if it was a transaction */
-  finish(endTimestamp?: number): string | undefined;
+  finish(useLastSpanTimestamp?: boolean): string | undefined;
   /** Return a traceparent compatible header string */
   toTraceparent(): string;
   /** Convert the object to JSON for w. spans array info only */

Original file line number	Diff line number	Diff line change
`@@ -179,5 +179,5 @@ export interface Hub {`
`179`	`179`	`*`
`180`	`180`	`* @param span Already constructed span which should be started or properties with which the span should be created`
`181`	`181`	`*/`
`182`		`- startSpan(span?: Span \| SpanContext): Span;`
	`182`	`+ startSpan(span?: Span \| SpanContext, forceNoChild?: boolean): Span;`
`183`	`183`	`}`