feat(core): Make custom tracing methods return spans & set default op (…

…#10633) This should make using these much easier: * add a default `op` to the spans, so users don't need to specify them. * Return the created span (or undefined), ensuring users don't need to do all the checking for op etc. themselves. Also make some small adjustments to ember & angular instrumentation to leverage some of these changes.
getsentry · Feb 13, 2024 · 88cc758 · 88cc758
1 parent 8ba1d77
commit 88cc758
Show file tree

Hide file tree

Showing 5 changed files with 422 additions and 17 deletions.
diff --git a/packages/angular/src/tracing.ts b/packages/angular/src/tracing.ts
@@ -129,7 +129,6 @@ export class TraceService implements OnDestroy {
         if (!getActiveSpan()) {
           startBrowserTracingNavigationSpan(client, {
             name: strippedUrl,
-            op: 'navigation',
             origin: 'auto.navigation.angular',
             attributes: {
               [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',

diff --git a/packages/ember/addon/instance-initializers/sentry-performance.ts b/packages/ember/addon/instance-initializers/sentry-performance.ts
@@ -11,7 +11,7 @@ import type { ExtendedBackburner } from '@sentry/ember/runloop';
 import type { Span } from '@sentry/types';
 import { GLOBAL_OBJ, browserPerformanceTimeOrigin, timestampInSeconds } from '@sentry/utils';
 
-import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '@sentry/core';
+import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from '@sentry/core';
 import type { BrowserClient } from '..';
 import { getActiveSpan, startInactiveSpan } from '..';
 import type { EmberRouterMain, EmberSentryConfig, GlobalConfig, OwnConfig } from '../types';
@@ -115,17 +115,18 @@ export function _instrumentEmberRouter(
     browserTracingOptions.instrumentPageLoad !== false
   ) {
     const routeInfo = routerService.recognize(url);
-    Sentry.startBrowserTracingPageLoadSpan(client, {
+    activeRootSpan = Sentry.startBrowserTracingPageLoadSpan(client, {
       name: `route:${routeInfo.name}`,
-      op: 'pageload',
       origin: 'auto.pageload.ember',
+      attributes: {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
+      },
       tags: {
         url,
         toRoute: routeInfo.name,
         'routing.instrumentation': '@sentry/ember',
       },
     });
-    activeRootSpan = getActiveSpan();
   }
 
   const finishActiveTransaction = (_: unknown, nextInstance: unknown): void => {
@@ -147,19 +148,19 @@ export function _instrumentEmberRouter(
     const { fromRoute, toRoute } = getTransitionInformation(transition, routerService);
     activeRootSpan?.end();
 
-    Sentry.startBrowserTracingNavigationSpan(client, {
+    activeRootSpan = Sentry.startBrowserTracingNavigationSpan(client, {
       name: `route:${toRoute}`,
-      op: 'navigation',
       origin: 'auto.navigation.ember',
+      attributes: {
+        [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route',
+      },
       tags: {
         fromRoute,
         toRoute,
         'routing.instrumentation': '@sentry/ember',
       },
     });
 
-    activeRootSpan = getActiveSpan();
-
     transitionSpan = startInactiveSpan({
       attributes: {
         [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.ember',

diff --git a/packages/tracing-internal/src/browser/browserTracingIntegration.ts b/packages/tracing-internal/src/browser/browserTracingIntegration.ts
@@ -1,5 +1,6 @@
-/* eslint-disable max-lines, complexity */
+/* eslint-disable max-lines */
 import type { IdleTransaction } from '@sentry/core';
+import { getActiveSpan } from '@sentry/core';
 import { getCurrentHub } from '@sentry/core';
 import {
   SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
@@ -237,7 +238,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
     latestRouteName = finalContext.name;
     latestRouteSource = getSource(finalContext);
 
-    // eslint-disable-next-line deprecation/deprecation
     if (finalContext.sampled === false) {
       DEBUG_BUILD && logger.log(`[Tracing] Will not send ${finalContext.op} transaction because of beforeNavigate.`);
     }
@@ -316,7 +316,10 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
             // If there's an open transaction on the scope, we need to finish it before creating an new one.
             activeSpan.end();
           }
-          activeSpan = _createRouteTransaction(context);
+          activeSpan = _createRouteTransaction({
+            op: 'navigation',
+            ...context,
+          });
         });
 
         client.on('startPageLoadSpan', (context: StartSpanOptions) => {
@@ -325,7 +328,10 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
             // If there's an open transaction on the scope, we need to finish it before creating an new one.
             activeSpan.end();
           }
-          activeSpan = _createRouteTransaction(context);
+          activeSpan = _createRouteTransaction({
+            op: 'pageload',
+            ...context,
+          });
         });
       }
 
@@ -334,7 +340,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
           name: WINDOW.location.pathname,
           // pageload should always start at timeOrigin (and needs to be in s, not ms)
           startTimestamp: browserPerformanceTimeOrigin ? browserPerformanceTimeOrigin / 1000 : undefined,
-          op: 'pageload',
           origin: 'auto.pageload.browser',
           attributes: {
             [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
@@ -363,7 +368,6 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
             startingUrl = undefined;
             const context: StartSpanOptions = {
               name: WINDOW.location.pathname,
-              op: 'navigation',
               origin: 'auto.navigation.browser',
               attributes: {
                 [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
@@ -401,24 +405,32 @@ export const browserTracingIntegration = ((_options: Partial<BrowserTracingOptio
  * Manually start a page load span.
  * This will only do something if the BrowserTracing integration has been setup.
  */
-export function startBrowserTracingPageLoadSpan(client: Client, spanOptions: StartSpanOptions): void {
+export function startBrowserTracingPageLoadSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
   if (!client.emit) {
     return;
   }
 
   client.emit('startPageLoadSpan', spanOptions);
+
+  const span = getActiveSpan();
+  const op = span && spanToJSON(span).op;
+  return op === 'pageload' ? span : undefined;
 }
 
 /**
  * Manually start a navigation span.
  * This will only do something if the BrowserTracing integration has been setup.
  */
-export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): void {
+export function startBrowserTracingNavigationSpan(client: Client, spanOptions: StartSpanOptions): Span | undefined {
   if (!client.emit) {
     return;
   }
 
   client.emit('startNavigationSpan', spanOptions);
+
+  const span = getActiveSpan();
+  const op = span && spanToJSON(span).op;
+  return op === 'navigation' ? span : undefined;
 }
 
 /** Returns the value of a meta tag */