Skip to content

feat: APM Sentry Frontend #14027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jul 23, 2019
Merged

feat: APM Sentry Frontend #14027

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cd2ae45
feat: Trace outgoing XHR, Create transaction event
HazAT Jul 16, 2019
9f51235
fix: Create inital transaction
HazAT Jul 16, 2019
c8ecbb4
fix: Add sampled true
HazAT Jul 16, 2019
0aba508
feat: Fix inital page load span
HazAT Jul 16, 2019
a9df2ab
fix: Pageload + Navigation transaction
HazAT Jul 16, 2019
6342a1e
Merge remote-tracking branch 'origin/master' into feat/apm-v2
untitaker Jul 17, 2019
55c437c
fix: Add traceparent_v2
untitaker Jul 17, 2019
72c5d12
ref: CodeReview changes + history change fix
HazAT Jul 17, 2019
cc319c9
ref: Cleanup
HazAT Jul 19, 2019
679803a
Merge branch 'master' into feat/apm-v2
HazAT Jul 19, 2019
5781c05
feat: Use beta sdk for apm, Add localhost to enable apm
HazAT Jul 19, 2019
5c203d7
Apply suggestions from code review
HazAT Jul 19, 2019
81a9522
ref: CR
HazAT Jul 19, 2019
3a68e29
cr: Minor code changes
HazAT Jul 22, 2019
8d62f17
Merge branch 'master' into feat/apm-v2
HazAT Jul 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
"@babel/preset-react": "^7.0.0",
"@babel/preset-typescript": "^7.3.3",
"@babel/runtime": "^7.0.0",
"@sentry/browser": "^5.4.2",
"@sentry/integrations": "^5.4.2",
"@sentry/browser": "5.6.0-beta.0",
"@sentry/integrations": "5.6.0-beta.0",
"@sentry/typescript": "^5.3.0",
"@types/lodash": "^4.14.134",
"@types/moment-timezone": "^0.5.12",
Expand Down
28 changes: 21 additions & 7 deletions src/sentry/static/sentry/app/api.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,22 @@ export class Client {
}

request(path, options = {}) {
const method = options.method || (options.data ? 'POST' : 'GET');
let data = options.data;

if (!isUndefined(data) && method !== 'GET') {
data = JSON.stringify(data);
}

const hub = Sentry.getCurrentHub();
const requestSpan = hub.startSpan({
data: {
request_data: data,
},
op: 'http',
description: `${method} ${path}`,
});

let query;
try {
query = $.param(options.query || [], true);
Expand All @@ -174,15 +190,10 @@ export class Client {
});
throw err;
}
const method = options.method || (options.data ? 'POST' : 'GET');
let data = options.data;

const id = uniqueId();
metric.mark(`api-request-start-${id}`);

if (!isUndefined(data) && method !== 'GET') {
data = JSON.stringify(data);
}

let fullUrl;
if (path.indexOf(this.baseUrl) === -1) {
fullUrl = this.baseUrl + path;
Expand Down Expand Up @@ -259,7 +270,10 @@ export class Client {
...args
);
},
complete: this.wrapCallback(id, options.complete, true),
complete: (...args) => {
hub.finishSpan(requestSpan);
return this.wrapCallback(id, options.complete, true)(...args);
},
})
);

Expand Down
25 changes: 22 additions & 3 deletions src/sentry/static/sentry/app/bootstrap.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import ConfigStore from 'app/stores/configStore';
import Main from 'app/main';
import ajaxCsrfSetup from 'app/utils/ajaxCsrfSetup';
import plugins from 'app/plugins';
import {startApm} from 'app/utils/apm';

// SDK INIT --------------------------------------------------------
Sentry.init({
Expand All @@ -32,7 +33,6 @@ Sentry.init({
}),
new Tracing({
tracingOrigins: ['localhost', 'sentry.io', /^\//],
autoStartOnDomReady: false,
}),
],
});
Expand All @@ -44,10 +44,14 @@ Sentry.configureScope(scope => {
if (window.__SENTRY__VERSION) {
scope.setTag('sentry_version', window.__SENTRY__VERSION);
}
scope.setSpan(
Sentry.getCurrentHub().startSpan({
op: 'pageload',
sampled: true,
})
);
});

// -----------------------------------------------------------------

// Used for operational metrics to determine that the application js
// bundle was loaded by browser.
metric.mark('sentry-app-init');
Expand All @@ -63,6 +67,21 @@ if (window.__initialData) {
ConfigStore.loadInitialData(window.__initialData);
}

// APM -------------------------------------------------------------
const config = ConfigStore.getConfig();
// This is just a simple gatekeeper to not enable apm for whole sentry.io at first
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we feature flag this instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but this anyway would just report transaction events to our account.
Enabling this for other orgs would just increase the data which we, in the beginning, don't want to.

if (
config &&
((config.userIdentity &&
config.userIdentity.email &&
config.userIdentity.email.includes('sentry')) ||
(config.urlPrefix &&
(config.urlPrefix.includes('localhost') || config.urlPrefix.includes('127.0.0.1'))))
) {
startApm();
}
// -----------------------------------------------------------------

// these get exported to a global variable, which is important as its the only
// way we can call into scoped objects

Expand Down
45 changes: 45 additions & 0 deletions src/sentry/static/sentry/app/utils/apm.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import * as Router from 'react-router';
import * as Sentry from '@sentry/browser';

let flushTransactionTimeout = undefined;
let firstPageLoad = true;

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.
let transactionSpan;
const hub = Sentry.getCurrentHub();
hub.configureScope(scope => {
if (firstPageLoad) {
transactionSpan = scope.getSpan();
firstPageLoad = false;
} else {
const prevTransactionSpan = scope.getSpan();
// If there is a transaction we set the name to the route
if (prevTransactionSpan && prevTransactionSpan.timestamp === undefined) {
hub.finishSpan(prevTransactionSpan);
}
transactionSpan = hub.startSpan({
op: 'navigation',
sampled: true,
});
}
scope.setSpan(transactionSpan);
});

if (flushTransactionTimeout) {
clearTimeout(flushTransactionTimeout);
}

flushTransactionTimeout = setTimeout(() => {
hub.finishSpan(transactionSpan);
}, 5000);
}

export function startApm() {
startTransaction();
Router.browserHistory.listen(() => {
startTransaction();
});
}
12 changes: 8 additions & 4 deletions src/sentry/static/sentry/app/views/app.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import $ from 'jquery';
import {ThemeProvider} from 'emotion-theming';
import {Tracing} from '@sentry/integrations';
import {browserHistory} from 'react-router';
import {get, isEqual} from 'lodash';
import {getCurrentHub} from '@sentry/browser';
Expand Down Expand Up @@ -158,7 +157,6 @@ class App extends React.Component {
}

componentDidMount() {
this.updateTracing();
fetchGuides();
}

Expand All @@ -167,7 +165,6 @@ class App extends React.Component {
if (!isEqual(config, prevProps.config)) {
this.handleConfigStoreChange(config);
}

this.updateTracing();
}

Expand All @@ -177,7 +174,14 @@ class App extends React.Component {

updateTracing() {
const route = getRouteStringFromRoutes(this.props.routes);
Tracing.startTrace(getCurrentHub(), route);
const scope = getCurrentHub().getScope();
if (scope) {
const transactionSpan = scope.getSpan();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this potentially get a span inside the transaction span?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically yes but, scope.getSpan always returns the parent span which always should be the transaction.

// If there is a transaction we set the name to the route
if (transactionSpan) {
transactionSpan.transaction = route;
}
}
}

handleConfigStoreChange(config) {
Expand Down
86 changes: 43 additions & 43 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1182,58 +1182,58 @@
hey-listen "^1.0.8"
style-value-types "^3.1.4"

"@sentry/browser@^5.4.2":
version "5.4.3"
resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-5.4.3.tgz#c9b56611c6b7de624962d2ea3fc0a9318f44a853"
integrity sha512-5fPlco0/65RpyNy9LMURrZhX1RT1+CAoR06yU65ZB81N3oRuAGDYE85Hs62t0T3EEGTKrMundINHcq8UD1V0xw==
dependencies:
"@sentry/core" "5.4.3"
"@sentry/types" "5.4.2"
"@sentry/utils" "5.4.2"
"@sentry/browser@5.6.0-beta.0":
version "5.6.0-beta.0"
resolved "https://registry.yarnpkg.com/@sentry/browser/-/browser-5.6.0-beta.0.tgz#b0524305d618b4e228567939dbd3fadb69fbdd9f"
integrity sha512-/tRVsZoqvsMFGNyg9OBEn1RoWtoFGMTOx6IFHqbTRwlrfyaF0JKhTbHVzj7RahGeOJBrKYePbgv5JKyT1fqNEw==
dependencies:
"@sentry/core" "5.6.0-beta.0"
"@sentry/types" "5.6.0-beta.0"
"@sentry/utils" "5.6.0-beta.0"
tslib "^1.9.3"

"@sentry/core@5.4.3":
version "5.4.3"
resolved "https://registry.yarnpkg.com/@sentry/core/-/core-5.4.3.tgz#c9e3a6cc9f5e296c5a5e19a7a925d9ee9125a95f"
integrity sha512-VjRv9BXip2BtCSohi/WQra+Ep4B8ajer1nU1VpKy5tUCjpVfXRpitY23EdEl+MVJH7h6YPZ45JsuFiKGgrtaFQ==
"@sentry/core@5.6.0-beta.0":
version "5.6.0-beta.0"
resolved "https://registry.yarnpkg.com/@sentry/core/-/core-5.6.0-beta.0.tgz#0c1d7ac47ff0e2fb1c6b9e06fd233641e2514d50"
integrity sha512-gy6BfCR+ITzgfoUbSfiGFyBC3I+Jmqf/CObh5FRjYWFbKAnZ0+YLWJWmerGNtojFmpvo2R8yhlgd0glHAiSGEQ==
dependencies:
"@sentry/hub" "5.4.3"
"@sentry/minimal" "5.4.3"
"@sentry/types" "5.4.2"
"@sentry/utils" "5.4.2"
"@sentry/hub" "5.6.0-beta.0"
"@sentry/minimal" "5.6.0-beta.0"
"@sentry/types" "5.6.0-beta.0"
"@sentry/utils" "5.6.0-beta.0"
tslib "^1.9.3"

"@sentry/hub@5.4.3":
version "5.4.3"
resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-5.4.3.tgz#d6936f976435dd552e06a6a3e9f8cd643e9b9b3f"
integrity sha512-97bnk2dDuCdFv2xhujogqPiDDpKOsHxBXH1jOJ5ezr3/uZNsMRr450FDxxJJYDLuSx+qZ/+vUFfdVNjprBsuSg==
"@sentry/hub@5.6.0-beta.0":
version "5.6.0-beta.0"
resolved "https://registry.yarnpkg.com/@sentry/hub/-/hub-5.6.0-beta.0.tgz#907c46c726d9eb90b9748d7077dfdefbcc0ef6c2"
integrity sha512-IAIeEpduPrWwU+CVNYgTVBrfu+h3ZO7s86eQaaC1P+WcL2fux2qrFQ+PaJnumh5iPdozE1SG2GnhK2oPfKpKrQ==
dependencies:
"@sentry/types" "5.4.2"
"@sentry/utils" "5.4.2"
"@sentry/types" "5.6.0-beta.0"
"@sentry/utils" "5.6.0-beta.0"
tslib "^1.9.3"

"@sentry/integrations@^5.4.2":
version "5.4.2"
resolved "https://registry.yarnpkg.com/@sentry/integrations/-/integrations-5.4.2.tgz#75312407571392badc40ee48b031fae701fee564"
integrity sha512-+nPaFdj8v2EBkZsQBF/pOIwqcf32//R3Iz37aMAWuutw0YOgJXJEaPe4GNMpk78bH3ZwVltGlW8RsDD6zgcc7A==
"@sentry/integrations@5.6.0-beta.0":
version "5.6.0-beta.0"
resolved "https://registry.yarnpkg.com/@sentry/integrations/-/integrations-5.6.0-beta.0.tgz#63a94ac1281839923b2d7b63c515d1576d8cf368"
integrity sha512-bjO/3jPRF2irS4ykkujiOo96nF4nMPObn8kb87p9d83G035/r5E5BqQ6fVDfyjerVbE6pDUWEAFm8dusM5/+TA==
dependencies:
"@sentry/types" "5.4.2"
"@sentry/utils" "5.4.2"
"@sentry/types" "5.6.0-beta.0"
"@sentry/utils" "5.6.0-beta.0"
tslib "^1.9.3"

"@sentry/minimal@5.4.3":
version "5.4.3"
resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-5.4.3.tgz#afaa8f7a3b5074efa8d70a2edac920b2e0cdbe15"
integrity sha512-xSCcKDtVtlmJIGmnlNH2fL++4l7iORJ+pOOfTz1Yjm/Il7Tz9wLVodbEfplXmAbOvgG/x9oilbW0MBSnrwKPfQ==
"@sentry/minimal@5.6.0-beta.0":
version "5.6.0-beta.0"
resolved "https://registry.yarnpkg.com/@sentry/minimal/-/minimal-5.6.0-beta.0.tgz#9e487adf76c6eb0d2d1f653e67dada3d3ca57866"
integrity sha512-aL4nKcWmuq7FIZlnmDQkBbh+pZZEOZXaoQihKqpoP9nAMO+9hXlHo5RqqxW5WoHFBSG3PElZ/BD1Ia13uLNg0Q==
dependencies:
"@sentry/hub" "5.4.3"
"@sentry/types" "5.4.2"
"@sentry/hub" "5.6.0-beta.0"
"@sentry/types" "5.6.0-beta.0"
tslib "^1.9.3"

"@sentry/types@5.4.2":
version "5.4.2"
resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.4.2.tgz#4ab327bced2cfd961dc7a29f0ff43398f913f6a6"
integrity sha512-yh1fd7x5lLOIZ8W3A1I792B3jowJVCWp4HcTRikjTsjbF8lcURY62m+hiLYUFPTIX99AlFRIPiApDkWiwMGYMA==
"@sentry/types@5.6.0-beta.0":
version "5.6.0-beta.0"
resolved "https://registry.yarnpkg.com/@sentry/types/-/types-5.6.0-beta.0.tgz#387faa0d122088ff9828bd35df28301b2ec89162"
integrity sha512-QWuw7YlS8Q2vIBfH35P/sIk916bJ3wuPcwN71pmAFlCDlcv9UN+pPStW/en2S68dt2hwvSUN9HjtzDlN1Vdgtg==

"@sentry/typescript@^5.3.0":
version "5.3.0"
Expand All @@ -1243,12 +1243,12 @@
tslint-config-prettier "^1.18.0"
tslint-consistent-codestyle "^1.15.1"

"@sentry/utils@5.4.2":
version "5.4.2"
resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.4.2.tgz#c88c6c08d635e1744a443cfefe9e2ed7fa250e4e"
integrity sha512-AW7/TGt2HiPQB8lJ8NgMgaFAIDQpKDF+wV8nENRbC1CP1zzcvb1QBF4zBL2auDT4fhkhVa817064s7vlDiJVLQ==
"@sentry/utils@5.6.0-beta.0":
version "5.6.0-beta.0"
resolved "https://registry.yarnpkg.com/@sentry/utils/-/utils-5.6.0-beta.0.tgz#784fe659b49f2c39c52cf782e03d9e9f4da99fe8"
integrity sha512-NnvpyDswCd4uWMplalGTnAvB4ByU5XrOqlahLa5XSjOq833/UO5eooCrylMH5h7r272D9rmDxCyZuxapnuhisw==
dependencies:
"@sentry/types" "5.4.2"
"@sentry/types" "5.6.0-beta.0"
tslib "^1.9.3"

"@storybook/addon-a11y@^4.1.3":
Expand Down