Browse Source

feat(apm): Use new TransactionActivity integration (#15517)

* feat: Use new TransactionActivity integration

With activity idle debounce

* meta: Bump SDK

* feat: Use latest version of integration

* ref: 5.10.0-beta.0

* fix: Mocks
Daniel Griesser 5 years ago
parent
commit
8f52c8fa86

+ 2 - 2
package.json

@@ -17,8 +17,8 @@
     "@babel/preset-react": "^7.7.0",
     "@babel/preset-typescript": "^7.7.2",
     "@babel/runtime": "~7.7.2",
-    "@sentry/browser": "5.7.0-beta.1",
-    "@sentry/integrations": "5.6.0-beta.4",
+    "@sentry/browser": "5.10.0-beta.0",
+    "@sentry/integrations": "5.10.0-beta.0",
     "@types/classnames": "^2.2.0",
     "@types/clipboard": "^2.0.1",
     "@types/color": "^3.0.0",

+ 4 - 10
src/sentry/static/sentry/app/api.tsx

@@ -3,6 +3,7 @@ import isNil from 'lodash/isNil';
 import get from 'lodash/get';
 import $ from 'jquery';
 import * as Sentry from '@sentry/browser';
+import {TransactionActivity} from '@sentry/integrations';
 
 import {
   PROJECT_MOVED,
@@ -15,8 +16,6 @@ import {uniqueId} from 'app/utils/guid';
 import GroupActions from 'app/actions/groupActions';
 import createRequestError from 'app/utils/requestError/createRequestError';
 
-import {startRequest, finishRequest} from 'app/utils/apm';
-
 export class Request {
   alive: boolean;
   xhr: JQueryXHR;
@@ -269,17 +268,13 @@ export class Client {
       }
     }
 
-    // TODO(kamil): We forgot to add this to Spans interface
-    const requestSpan = Sentry.startSpan({
+    const xhrActivity = TransactionActivity.pushActivity('xhr', {
       data: {
         request_data: data,
       },
       op: 'http',
       description: `${method} ${fullUrl}`,
-    }) as Sentry.Span;
-
-    // notify apm utils that a request has started
-    startRequest(id);
+    });
 
     const errorObject = new Error();
 
@@ -348,8 +343,7 @@ export class Client {
           );
         },
         complete: (jqXHR: JQueryXHR, textStatus: string) => {
-          requestSpan.finish();
-          finishRequest(id);
+          TransactionActivity.popActivity(xhrActivity);
 
           return this.wrapCallback<[JQueryXHR, string]>(id, options.complete, true)(
             jqXHR,

+ 8 - 9
src/sentry/static/sentry/app/bootstrap.jsx

@@ -12,7 +12,7 @@ import ReactDOM from 'react-dom';
 import Reflux from 'reflux';
 import * as Router from 'react-router';
 import * as Sentry from '@sentry/browser';
-import {ExtraErrorData, Tracing} from '@sentry/integrations';
+import {ExtraErrorData, Tracing, TransactionActivity} from '@sentry/integrations';
 import createReactClass from 'create-react-class';
 import jQuery from 'jquery';
 import moment from 'moment';
@@ -35,17 +35,16 @@ Sentry.init({
     new Tracing({
       tracingOrigins: ['localhost', 'sentry.io', /^\//],
     }),
+    new TransactionActivity(),
   ],
 });
 
-Sentry.configureScope(scope => {
-  if (window.__SENTRY__USER) {
-    scope.setUser(window.__SENTRY__USER);
-  }
-  if (window.__SENTRY__VERSION) {
-    scope.setTag('sentry_version', window.__SENTRY__VERSION);
-  }
-});
+if (window.__SENTRY__USER) {
+  Sentry.setUser(window.__SENTRY__USER);
+}
+if (window.__SENTRY__VERSION) {
+  Sentry.setTag('sentry_version', window.__SENTRY__VERSION);
+}
 
 // Used for operational metrics to determine that the application js
 // bundle was loaded by browser.

+ 11 - 128
src/sentry/static/sentry/app/utils/apm.jsx

@@ -1,141 +1,24 @@
-import * as Router from 'react-router';
 import * as Sentry from '@sentry/browser';
-
-let firstPageLoad = true;
-let flushTransactionTimeout = undefined;
-let wasInterrupted = false;
-let currentTransactionSpan = null;
-
-const TRANSACTION_TIMEOUT = 5000;
-const requests = new Set([]);
-const renders = new Set([]);
-const hasActiveRequests = () => requests.size > 0;
-const hasActiveRenders = () => renders.size > 0;
-
-function startTransaction() {
-  // We do set the transaction name in the router but we want to start it here
-  // since in the App component where we set the transaction name, it's called multiple
-  // times. This would result in losing the start of the transaction.
-  Sentry.configureScope(scope => {
-    if (firstPageLoad) {
-      return;
-    }
-
-    // If there's a previous transaction span open, finish it
-    if (currentTransactionSpan) {
-      currentTransactionSpan.finish();
-    }
-
-    currentTransactionSpan = Sentry.startSpan({
-      op: 'navigation',
-      sampled: true,
-    });
-    scope.setSpan(currentTransactionSpan);
-    scope.setTag('ui.nav', 'navigation');
-  });
-
-  // Timeout a transaction if no other spans get started
-  finishTransaction(TRANSACTION_TIMEOUT);
-}
-
-/**
- * Postpone finishing the root span until all renders and requests are finished
- *
- * TODO(apm): We probably want a hard limit for root span, e.g. it's possible we have long
- * API requests combined with renders that could create a very long root span.
- *
- * TODO(apm): Handle polling requests?
- */
-function interruptFlush() {
-  if (!flushTransactionTimeout) {
-    return;
-  }
-
-  clearTimeout(flushTransactionTimeout);
-  wasInterrupted = true;
-}
-
-export function finishTransaction(delay = TRANSACTION_TIMEOUT) {
-  if (flushTransactionTimeout || (hasActiveRenders() || hasActiveRequests())) {
-    interruptFlush();
-  }
-
-  flushTransactionTimeout = setTimeout(() => {
-    if (currentTransactionSpan) {
-      currentTransactionSpan.finish();
-      currentTransactionSpan = null;
-      firstPageLoad = false;
-    }
-  }, delay);
-}
-
-/**
- * These `start-` functions attempt to track the state of "actions".
- *
- * They interrupt the transaction flush (which times out), and
- * requires the related `finish-` function to be called.
- */
-export function startRequest(id) {
-  requests.add(id);
-  interruptFlush();
-}
-export function startRender(id) {
-  renders.add(id);
-  interruptFlush();
-}
-
-/**
- * These `finish-` functions clean up the "active" state of an ongoing "action".
- * If there are no other "actions" and we have interrupted a flush, we should
- * finish the transaction
- */
-export function finishRequest(id) {
-  requests.delete(id);
-  // TODO(apm): Is this necessary? flush should be interrupted already from start()
-  interruptFlush();
-
-  if (wasInterrupted && !hasActiveRenders() && !hasActiveRequests()) {
-    finishTransaction(1);
-  }
-}
-export function finishRender(id) {
-  renders.delete(id);
-  interruptFlush();
-
-  if (wasInterrupted && !hasActiveRenders() && !hasActiveRequests()) {
-    finishTransaction(1);
-  }
-}
+import {TransactionActivity} from '@sentry/integrations';
 
 /**
  * Sets the transaction name
  */
 export function setTransactionName(name) {
-  Sentry.configureScope(scope => {
-    const span = scope.getSpan();
-
-    if (!span) {
-      return;
-    }
-
-    span.transaction = firstPageLoad ? `PageLoad: ${name}` : name;
-    scope.setTag('ui.route', name);
-  });
+  TransactionActivity.updateTransactionName(name);
+  Sentry.setTag('ui.route', name);
 }
 
 /**
- * This is called only when our application is initialized. Creates a root span
- * and creates a router listener to create a new root span as user navigates.
+ * This is called only when our application is initialized. Creates a transaction
+ * and creates a router listener to create a new transaction as user navigates.
  */
 export function startApm() {
-  Sentry.configureScope(scope => {
-    currentTransactionSpan = Sentry.startSpan({
-      op: 'pageload',
-      sampled: true,
-    });
-    scope.setSpan(currentTransactionSpan);
-    scope.setTag('ui.nav', 'pageload');
+  // `${window.location.href}` will be used a temp transaction name
+  // Internally once our <App> is mounted and the Router is initalized
+  // we call `setTransactionName` to update the full URL to the route name
+  TransactionActivity.startIdleTransaction(`${window.location.href}`, {
+    op: 'pageload',
+    sampled: true,
   });
-  startTransaction();
-  Router.browserHistory.listen(() => startTransaction());
 }

+ 9 - 24
src/sentry/static/sentry/app/utils/profiler.jsx

@@ -1,8 +1,7 @@
 import PropTypes from 'prop-types';
 import React from 'react';
-import * as Sentry from '@sentry/browser';
+import {TransactionActivity} from '@sentry/integrations';
 
-import {startRender, finishRender} from 'app/utils/apm';
 import getDisplayName from 'app/utils/getDisplayName';
 
 export default function profiler() {
@@ -20,33 +19,19 @@ export default function profiler() {
         this.finishProfile();
       }
 
-      span = this.initializeSpan();
-
-      initializeSpan() {
-        const span = Sentry.startSpan({
-          data: {},
-          op: 'react',
-          description: `<${displayName}>`,
-        });
-        startRender(displayName);
-
-        // TODO(apm): We could try to associate this component with API client
-        // e.g:
-        // if (this.props.api) {
-        // this.props.api.setParentSpan(span);
-        // }
-
-        return span;
-      }
+      activity = TransactionActivity.pushActivity(displayName, {
+        data: {},
+        op: 'react',
+        description: `<${displayName}>`,
+      });
 
       finishProfile = () => {
-        if (!this.span) {
+        if (!this.activity) {
           return;
         }
 
-        this.span.finish();
-        finishRender(displayName);
-        this.span = null;
+        TransactionActivity.popActivity(this.activity);
+        this.activity = null;
       };
 
       render() {

+ 4 - 0
tests/js/setup.js

@@ -103,6 +103,10 @@ jest.mock('@sentry/browser', () => {
   return {
     init: jest.fn(),
     configureScope: jest.fn(),
+    setTag: jest.fn(),
+    setTags: jest.fn(),
+    setExtra: jest.fn(),
+    setExtras: jest.fn(),
     captureBreadcrumb: jest.fn(),
     addBreadcrumb: jest.fn(),
     captureMessage: jest.fn(),

+ 43 - 56
yarn.lock

@@ -1830,78 +1830,65 @@
     hey-listen "^1.0.8"
     style-value-types "^3.1.4"
 
-"@sentry/browser@5.7.0-beta.1":
-  version "5.7.0-beta.1"
-  resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-5.7.0-beta.1.tgz#937ff3d35d1b0d66e52382b4349947fa5b9fedff"
-  integrity sha512-OsuqdkiwcbGtKIo8/Z75N+jQell3YWbt+BwePm2HDgojOsoZiSORytVM5yyCuQGNOF/n7/YkQMICoMPvHyh1qA==
-  dependencies:
-    "@sentry/core" "5.7.0-beta.1"
-    "@sentry/types" "5.7.0-beta.1"
-    "@sentry/utils" "5.7.0-beta.1"
+"@sentry/browser@5.10.0-beta.0":
+  version "5.10.0-beta.0"
+  resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-5.10.0-beta.0.tgz#5f65a032fede5288814e0a2ab045159b78fa8482"
+  integrity sha512-IYhUAoYTrO9mlYwYNK1kaxfyUGrdMwauxKrY4l92z8bSExFEax/KqPsfPnfZdx5v9w4b9l+hKNe/7qYOH2Fryg==
+  dependencies:
+    "@sentry/core" "5.10.0-beta.0"
+    "@sentry/types" "5.10.0-beta.0"
+    "@sentry/utils" "5.10.0-beta.0"
     tslib "^1.9.3"
 
-"@sentry/core@5.7.0-beta.1":
-  version "5.7.0-beta.1"
-  resolved "https://registry.yarnpkg.com/@sentry/core/-/core-5.7.0-beta.1.tgz#fa6e206574e72f4cb2520ea7881b5dbc7f947fd9"
-  integrity sha512-pbiO2MsV7VikKuERvYJwaQspt6egvTkoYyExdYXXb2Ja8uXD+bm94wdIvZbTpQuSrhqDWbxyaIsGWNJyzQgadQ==
+"@sentry/core@5.10.0-beta.0":
+  version "5.10.0-beta.0"
+  resolved "https://registry.yarnpkg.com/@sentry/core/-/core-5.10.0-beta.0.tgz#f96d07a74ddeb13d0bc1e774409556a50b042358"
+  integrity sha512-vc84lh1/AbPTvHTpPK1SN9EXaLqUzm4RQcURqPxC2fUo38zGRBU36GLlYBzYp4aZNdXAj3pgtO1JV3yHNGWaug==
   dependencies:
-    "@sentry/hub" "5.7.0-beta.1"
-    "@sentry/minimal" "5.7.0-beta.1"
-    "@sentry/types" "5.7.0-beta.1"
-    "@sentry/utils" "5.7.0-beta.1"
+    "@sentry/hub" "5.10.0-beta.0"
+    "@sentry/minimal" "5.10.0-beta.0"
+    "@sentry/types" "5.10.0-beta.0"
+    "@sentry/utils" "5.10.0-beta.0"
     tslib "^1.9.3"
 
-"@sentry/hub@5.7.0-beta.1":
-  version "5.7.0-beta.1"
-  resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-5.7.0-beta.1.tgz#a07813b28a7c7ad04097da9af65d95f5e81a0265"
-  integrity sha512-SAx8+arIfP59c4L7EIXtxJ1G0HbdRObUGoxNaNOCOnIEfTLBDYcrEU1RqKUcdpDjYc+hclBB5KsbdhDEEXvj0g==
+"@sentry/hub@5.10.0-beta.0":
+  version "5.10.0-beta.0"
+  resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-5.10.0-beta.0.tgz#b58c8049782866207322153f1b100cd54347c2f5"
+  integrity sha512-40PN15dc1FECXaaCOs6k6I97YjtjVR28x5U0JeQs18bZyarpUixj3bTMi8xcMpHREg5NJEwmGoyrBARqYG6p2Q==
   dependencies:
-    "@sentry/types" "5.7.0-beta.1"
-    "@sentry/utils" "5.7.0-beta.1"
+    "@sentry/types" "5.10.0-beta.0"
+    "@sentry/utils" "5.10.0-beta.0"
     tslib "^1.9.3"
 
-"@sentry/integrations@5.6.0-beta.4":
-  version "5.6.0-beta.4"
-  resolved "https://registry.yarnpkg.com/@sentry/integrations/-/integrations-5.6.0-beta.4.tgz#0d4cc40d1191c4f3b9f68f1fa20a0caa5611186a"
-  integrity sha512-D0f7sVP3DQ/4OXmoCg4t7BSOUPeSZrageShfYH4/i8RGP5qMnucyZ1yJQnga59xF1hJ9mpdDiKmtXxN5gmaNcQ==
+"@sentry/integrations@5.10.0-beta.0":
+  version "5.10.0-beta.0"
+  resolved "https://registry.yarnpkg.com/@sentry/integrations/-/integrations-5.10.0-beta.0.tgz#81179bf295598669951a13d2ff2448f82b37750a"
+  integrity sha512-+/xxSkby0lLbzA8VOrHL/LF4AM8Zqa76EVP3hIcxGuh22LiGwJzARrrxcz64Bx3G/JqpjvzA/XfS3M4EnLnqIg==
   dependencies:
-    "@sentry/types" "5.6.0-beta.4"
-    "@sentry/utils" "5.6.0-beta.4"
+    "@sentry/types" "5.10.0-beta.0"
+    "@sentry/utils" "5.10.0-beta.0"
     tslib "^1.9.3"
 
-"@sentry/minimal@5.7.0-beta.1":
-  version "5.7.0-beta.1"
-  resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-5.7.0-beta.1.tgz#886e9b7498613186c74d7dadcf137126de49a127"
-  integrity sha512-WiMmy6u6uuzpulynJRAlSKz07dAMSC5LeD+n2ZF17f8FHUSsgvcPnizxSjtKzxycf43xD3B26wrZQyFaSHfXoQ==
+"@sentry/minimal@5.10.0-beta.0":
+  version "5.10.0-beta.0"
+  resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-5.10.0-beta.0.tgz#0bf7834b0346454b9cc73cfbbc4535cad273ba1b"
+  integrity sha512-5qqO3Bq3d5B0Qh4hrF+vi0GcsjihEZ/tGwjqDwsKIzrkvByL1WTr5E4VHCg8pAzltgIQtvvQQDGdKwMz28s3rw==
   dependencies:
-    "@sentry/hub" "5.7.0-beta.1"
-    "@sentry/types" "5.7.0-beta.1"
+    "@sentry/hub" "5.10.0-beta.0"
+    "@sentry/types" "5.10.0-beta.0"
     tslib "^1.9.3"
 
-"@sentry/types@5.6.0-beta.4":
-  version "5.6.0-beta.4"
-  resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.6.0-beta.4.tgz#7ad453fd6969cc38c97c85c4d7e5e1f7588276b2"
-  integrity sha512-YVloNOyyek0Qackamn1kDyFpVTRysj8CBUJwZx774VMtYHMy48Lre2rxz3s1j/XztsaZEqn0nO86aP8DzlNNzg==
+"@sentry/types@5.10.0-beta.0":
+  version "5.10.0-beta.0"
+  resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.10.0-beta.0.tgz#c065ca0f61ef289c7fe3ec744907f030439f94ee"
+  integrity sha512-KaOOvzEAYpKRJ7arMpjzCGzazU+YnQcPWfbBVuwWN+Ofaz6oDaJ4DU1iEKopWg35gW6hwZyUb2/MrQm8N118Ew==
 
-"@sentry/types@5.7.0-beta.1":
-  version "5.7.0-beta.1"
-  resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.7.0-beta.1.tgz#91e79fb3862a285275b094330c2d44d93c2af927"
-  integrity sha512-0qbS4TzD1/JJy7YEHONA5ItXiTdDPOk9svjj1/+VuBv95yjmlRJ8fIrpv4woN9Lijas+uUB1tDxOpPCpq8InwQ==
-
-"@sentry/utils@5.6.0-beta.4":
-  version "5.6.0-beta.4"
-  resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.6.0-beta.4.tgz#d3e2ffc7487590af7555da836d17f3a19ff07a8d"
-  integrity sha512-ysrXMsbdiB3rogh4sridexazJKZRIJdSYarj8fGMKryPsF7FwvVu8i3VGCz4nmGqxtFhyDDUu0udhxgx9wQhZA==
-  dependencies:
-    "@sentry/types" "5.6.0-beta.4"
-    tslib "^1.9.3"
-
-"@sentry/utils@5.7.0-beta.1":
-  version "5.7.0-beta.1"
-  resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.7.0-beta.1.tgz#fd7a0235617f177a5e36028c19a0a15a4ff9d51f"
-  integrity sha512-Mf4Hm+dykF817ybspVuevsEfFAWgogtnwOUDkue1iUFZBJiYUJoJS76Auaw8VA/Us2tj1bIcZkCEQsG/93EqvA==
+"@sentry/utils@5.10.0-beta.0":
+  version "5.10.0-beta.0"
+  resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.10.0-beta.0.tgz#2070e2fe3b209bcc18bd931f46060540c2b159c2"
+  integrity sha512-6r5wdgC8iEIe37sSe4t2dg4li5KLb5RysrwhSD8FBvinJSSQAIbKsVMtcj6Ydh02KBk12KGDLiZXRrutriXOIw==
   dependencies:
-    "@sentry/types" "5.7.0-beta.1"
+    "@sentry/types" "5.10.0-beta.0"
     tslib "^1.9.3"
 
 "@storybook/addon-a11y@^4.1.3":