-
Notifications
You must be signed in to change notification settings - Fork 479
[gp-code] measure all sessions vs errored sessions #428
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.
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
Conversation
b0a9999
to
cf7bec5
Compare
cf7bec5
to
48bb437
Compare
1518919
to
e8e8a93
Compare
b309d99
to
30d27c7
Compare
e2c3ab1
to
58baf65
Compare
30d27c7
to
1f46172
Compare
84afe45
to
f0949e3
Compare
if (resourceSource.match(new RegExp(/\/build\/ide\/code:.+\/__files__\//g))) { | ||
// TODO(ak) reconsider how to hide knowledge of VS Code from supervisor frontend, i.e instrument amd loader instead | ||
labels['resource'] = 'vscode-web-workbench'; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this check is relevant here, because we are checking for errors in the whole window and there may be resources like in <iframe>
s which may trigger this, so the check is perhaps justified here.
b133f64
to
1493922
Compare
f0949e3
to
6fc5d45
Compare
ee48889
to
8a04b4e
Compare
6fc5d45
to
815cfe0
Compare
357e46d
to
c2e7e9c
Compare
const headers: http.OutgoingHttpHeaders = { | ||
'Content-Type': 'text/html', | ||
'Content-Security-Policy': cspDirectives | ||
'Content-Security-Policy': this._environmentService.isBuilt ? cspDirectives : allowAllCSP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this to make workbench.html onerror
works in dev mode. allowAllCSP
value comes from stackoverflow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested in this PR gitpod-io/gitpod#12702 (comment)
c2e7e9c
to
e2c44a2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test it in gitpod-io/gitpod#12702 (review)
} | ||
url.pathname = '/metrics-api'; | ||
return url.toString(); | ||
let workspaceId = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mustard-mh we should be careful with creating global variables, we had gitpodMetricsAddCounter
on purpose to avoid polluting global scope
|
||
// TODO collect errors, we can capture resourceSource, error if possible with stack, message, name, and window URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mustard-mh it is done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll clean some comments up by push commit to gp-code/main
, and improve this #428 (comment)
<script type="text/javascript" src="/_supervisor/frontend/main.js" onerror="onVsCodeWorkbenchError(event)" charset="utf-8"></script> | ||
<script src="./static/out/vs/loader.js" onerror="onVsCodeWorkbenchError(event)" ></script> | ||
<script src="./static/out/vs/webPackagePaths.js" onerror="onVsCodeWorkbenchError(event)" ></script> | ||
<script type="text/javascript" src="/_supervisor/frontend/main.js" onerror="onVsCodeWorkbenchError(event)" charset="utf-8" crossorigin="anonymous"></script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mustard-mh crossorigin="anonymous"
it does not seem you are making use of it, i.e. idea was to add it if we want to collect information about script errors, onVsCodeWorkbenchError should report them too then
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we add this, supervisor frontend
can use it too
See gitpod-io/gitpod#12702 for details