fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: Try to use a better performance API - #3356

Merged
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin
Mar 31, 2021
Merged

fix: Try to use a better performance API#3356
rhcarvalho merged 12 commits into
masterfrom
wmak/fix/performance-time-origin

Conversation

@wmak

@wmakwmak commented Mar 30, 2021

Copy link
Copy Markdown
Member
  • Trying to address the inconsistencies between browsers
  • Always try to use the timeOrigin first, as long as its consistent with the current time
  • Otherwise, use the navigationStart if its consistent with the current time
  • And still fallback to date if all of the above fails
  • Remember what source is used for the time origin, so we can tag events and analyze later
  • An attempt to fixTiming issues using Performance API #2590

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests. -- Manually tested with Safari, Chrome + Firefox
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

- Trying to address the inconsistencies between browsers
- Always try to use the timeOrigin first, as long as its consistent with
either the navigationStart or the current time
- Otherwise, use the navigationStart if its consistent with the current
time
- And still fallback to date if all else fails
@wmak
wmak requested a review from kamilogorek as a code ownerMarch 30, 2021 15:11
@wmak
wmak requested a review from rhcarvalhoMarch 30, 2021 15:22
@github-actions

github-actionsBot commented Mar 30, 2021

Copy link
Copy Markdown
Contributor

size-limit report

PathSize
@sentry/browser - CDN Bundle (gzipped)20.51 KB (+0.19% 🔺)
@sentry/browser - Webpack21.37 KB (+0.26% 🔺)
@sentry/react - Webpack21.41 KB (+0.26% 🔺)
@sentry/browser + @sentry/tracing - CDN Bundle (gzipped)27.6 KB (+0.14% 🔺)

@dashed

Copy link
Copy Markdown
Member

@wmak were you able to test this change in Safari, Chrome, and Firefox?

@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

@dashed
Yup!
In firefox when timeOrigin was incorrect, this changes to timing.navigationStart, and if timeOrigin was correct it would continue using it.
In chrome where timeOrigin is usually correct this continues to use it
in Safari which only has timing.navigationStart this continues to use that.

Comment threadpackages/utils/src/time.ts Outdated

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like this, it can likely mitigate the problems we're seeing on FF.

Thanks @wmak <3

Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts
Comment threadpackages/utils/src/time.ts Outdated
wmakand others added 2 commits March 30, 2021 13:01
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Rodolfo Carvalho <rodolfo.carvalho@sentry.io>
Comment threadpackages/utils/src/time.ts Outdated
Comment threadpackages/utils/src/time.ts Outdated
@wmak

wmak commented Mar 30, 2021

Copy link
Copy Markdown
MemberAuthor

Also, starting with Threshold == 1 hour, but this will likely mean we start seeing inaccurate durations up to an hour, if this solution works out we could drop Threshold down to a few seconds instead, just want to start with something safer for now

Comment threadpackages/utils/src/time.ts Outdated
Co-authored-by: Alberto Leal <mail4alberto@gmail.com>

@rhcarvalhorhcarvalho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd love to see this in prod.

@wmak I had one idea since yesterday -- as a way to debug and quantify how this is working, we could set some internal variable with a string depending on what time origin is being used (timeOrigin, navigationStart or dateNow) and then in Sentry we can use this + event processor to set a tag on every transaction, so we can filter and see what fraction of users use which method, see if there is a clear browser correlation etc.
We could also mark/tag whenever we decided that timeOrigin or navigationStart is reliable or not.

@kamilogorek would you have some suggestion on how to implement the "internal global value"? E.g. how exactly to bind to the global sentry object?

@rhcarvalho

Copy link
Copy Markdown
Contributor

Getting this in so we can test across more browsers in the wild.

@rhcarvalho
rhcarvalho merged commit dece89a into masterMar 31, 2021
@rhcarvalho
rhcarvalho deleted the wmak/fix/performance-time-origin branch March 31, 2021 17:21
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Timing issues using Performance API

3 participants

@wmak@dashed@rhcarvalho