feat: APM Sentry Frontend #14027
Merged
feat: APM Sentry Frontend #14027
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
markstory
reviewed
Jul 16, 2019
| Tracing.startTrace(getCurrentHub(), route); | ||
| const scope = getCurrentHub().getScope(); | ||
| if (scope) { | ||
| const transactionSpan = scope.getSpan(); |
There was a problem hiding this comment.
Could this potentially get a span inside the transaction span?
There was a problem hiding this comment.
Theoretically yes but, scope.getSpan always returns the parent span which always should be the transaction.
|
|
||
| flushTransactionTimeout = setTimeout(() => { | ||
| Sentry.getCurrentHub().finishSpan(transactionSpan); | ||
| }, 5000); |
There was a problem hiding this comment.
Should this be longer? Is it possible to use the unload event to push up incomplete transactions when the tab is closed or moved away via URL bar?
bruno-garcia
requested changes
Jul 16, 2019
Codecov Report
@@ Coverage Diff @@
## master #14027 +/- ##
==========================================
- Coverage 86.8% 80.48% -6.32%
==========================================
Files 3219 3214 -5
Lines 140347 139935 -412
Branches 4952 4963 +11
==========================================
- Hits 121831 112633 -9198
- Misses 17127 25920 +8793
+ Partials 1389 1382 -7
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #14027 +/- ##
==========================================
- Coverage 86.8% 80.48% -6.32%
==========================================
Files 3219 3214 -5
Lines 140347 139935 -412
Branches 4952 4963 +11
==========================================
- Hits 121831 112633 -9198
- Misses 17127 25920 +8793
+ Partials 1389 1382 -7
Continue to review full report at Codecov.
|
* master: ref(admin): Convert user edit page to react (#14074) ref: Remove unused Group.get_oldest_event and legacy events behavior (#14038) ref(api): Update DELETE users/ to support hard deleting (#14068) test(OrganizationDiscoverSavedQueryDetailTest): Stabilize put test (#14077) meta(readme): Sentry logo should link to sentry.io (#14076) ref: Remove duplicate column (#14073) App platform/update permissions token auth (#14046) feat: Support issue IDs as canonical parameters ref: Change to new traceparent header for Python SDK (#14070) feat: Use option to force-disable transaction events (#14056) feat(apm): Register option to force-disable transaction events (#14055) Feat/mark sentry app installed put route (#14060) ref: Remove unused Group.event_set property (#14036) fix: Filter out groups that are pending deletion/merge from `by_qualified_short_id` (SEN-849) fix(ui): Fix resolve/ignore actions for accounts without multi… (#14058) Fix: Remove extra $.param introduced in GH-14051 (#14061) feat: Use Snuba for Group.from_event_id (#14034) fix(ui) Display implicit default sort and default to descending (#14042) fix(github) Fix 404s not being handled in repository search (#14030) fix: Pass an empty array to $.param instead of an empty string when options.query is falsey (#14051) # Conflicts: # src/sentry/utils/sdk.py
untitaker
approved these changes
Jul 19, 2019
dashed
requested changes
Jul 19, 2019
dashed
requested changes
Jul 19, 2019
bruno-garcia
approved these changes
Jul 19, 2019
Co-Authored-By: Alberto Leal <[email protected]>
|
The description of the spans now look like this: |
|
I plan to merge this tomorrow 23. July. |
billyvg
reviewed
Jul 22, 2019
| @@ -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 | |||
There was a problem hiding this comment.
Can we feature flag this instead?
There was a problem hiding this comment.
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.
* master: (25 commits) ref(onboarding): Fix install promprt URL (#14106) fix(app-platform): Allow GET requests for published apps (#14109) feat: Update Group.get_latest_event to use Snuba event (#14039) ref(onboarding): Rename wizardNew -> onboarding (#14104) feat(apm): Update props to address proptype warnings for new transaction attributes (SEN-800) (#14040) ref(ui): Move and codesplit `ProjectPlugins` (#13952) feat(typescript): Add TypeScript compatibility (#13786) ref(templates): Remove unused content block default (#14090) ref(less): Remove unused admin.less (#14097) ref(onobarding): Remove old onboarding experience (#14066) fix(ui) Fix missing conditions in tag bars (#14063) ref(suspect-commits): Add hook (#14057) ref(frontend): Segment frontend web urls (#14096) feat(suspect-commits): Add analytics events (#14080) feat(servicehooks): Update servicehook URLs (#14093) license: Remove license headers (#14095) ref(templates): Remove unused account_nav (#14091) fix: Disable transaction events in store (#14088) fix(InstallWizard): Fix exception when InstallWizard completed (#14092) ref(admin): Fix thrashing on stat charts (#14094) ...
jan-auer
added a commit
that referenced
this pull request
Jul 24, 2019
* master: (115 commits) feat: Update to JS SDK 5.6.0-beta.1 + 0.10.2 sentry-python (#14116) fix(apm): Whitelist dev.getsentry.net for local development (#14117) test(datasets): Make Sentry use generic test functions in Snuba (#14111) ref(suspect-commits): Add text changes to empty state (#14121) build: Switch to psycopg2-binary feat(api): Add option to fetch Organization details without Pr… (#13925) ref: Remove EventDetails endpoint (#14107) test(ui): Mock the onboarding learn more video (#14108) tests(acceptance): Add tests for resolving issues in Issues Li… (#14069) feat(ui): Add basic templates for Incident Rules in settings (#14112) feat(eventsv2) Add basic transaction list (#14103) ref(environments) Optimize environment queries (#14102) fix(events-v2) Add additional user attributes to the user column (#14101) fix: Don't start pageload transaction (#14115) feat: APM Sentry Frontend (#14027) ref(onboarding): Fix install promprt URL (#14106) fix(app-platform): Allow GET requests for published apps (#14109) feat: Update Group.get_latest_event to use Snuba event (#14039) ref(onboarding): Rename wizardNew -> onboarding (#14104) feat(apm): Update props to address proptype warnings for new transaction attributes (SEN-800) (#14040) ...
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds frontend APM instrumentation to sentry.io.
It needs the
apmbranch insentry-javascriptthis PR getsentry/sentry-javascript#2161The way it works right now is that we decided that we only create transaction events that were trigger from the frontend throughout our entire system.
Meaning the frontend takes the sampling decision, python / our backend doesn't create transaction events on its own.
We create a
op: pageloadtransaction event on the initial page load, after 5 sec or on a navigation change, we send the transaction to Sentry.Currently we only instrument requests as individual spans.
After the initial page load, new transaction events with
op: navigationwill be emitted.There is a gatekeeper in there so only logged in users with emails that contain the word
sentryorlocalhostinstallation will enable emitting transaction events for now.