Skip to content
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

plugin metrics breaks application start up in Firefox #6582

Closed
akosyakov opened this issue Nov 19, 2019 · 11 comments
Closed

plugin metrics breaks application start up in Firefox #6582

akosyakov opened this issue Nov 19, 2019 · 11 comments
Labels
browser/firefox issues related to the firefox browser browser/safari issues related to the safari browser critical critical bugs / problems metrics issues related to metrics and logging plug-in system issues related to the plug-in system

Comments

@akosyakov
Copy link
Member

akosyakov commented Nov 19, 2019

Screen Shot 2019-11-19 at 14 42 50

cc @tsmaeder @JPinkney

It seems FireFox cannot handle parsing some regex patterns. Maybe https://github.com/eclipse-theia/theia/blob/master/packages/plugin-metrics/src/browser/plugin-metrics-creator.ts#L29

@akosyakov akosyakov added plug-in system issues related to the plug-in system browser/firefox issues related to the firefox browser critical critical bugs / problems metrics issues related to metrics and logging labels Nov 19, 2019
@vince-fugnitto
Copy link
Member

Lookbehind regexp are not supported in browsers besides Chrome.
The regexp will need to be updated.

@vince-fugnitto vince-fugnitto added the browser/safari issues related to the safari browser label Nov 19, 2019
@vince-fugnitto
Copy link
Member

All non-chrome browsers likely do not start now (verified with Firefox and Safari).

@tsmaeder
Copy link
Contributor

The regex in question is: (?<=Request)(.*?)(?=failed). I don't think it contains anything that could not be rewritten without the negative lookahead.

@vince-fugnitto
Copy link
Member

The regex in question is: (?<=Request)(.*?)(?=failed). I don't think it contains anything that could not be rewritten without the negative lookahead.

@tsmaeder would you like to provide a fix for it? I'm really not the best at regexp.
Also without tests, I'm not sure my implementation would be working correctly.

@tsmaeder
Copy link
Contributor

I can have a look tomorrow, not time today.

@tsmaeder
Copy link
Contributor

I'm trying to test a fix, but I never get any failures if I test like described on #6303

@tsmaeder
Copy link
Contributor

@akosyakov @vince-fugnitto any ideas what I'm doing wrong?

@tsmaeder
Copy link
Contributor

Hmh...all of a sudden, it works. Ignore my pleas!

@vince-fugnitto
Copy link
Member

@tsmaeder I did see this when testing the original PR:

 language_server_time_count{id="redhat.vscode-yaml" method="textDocument/hover" result="fail"} 0 # HELP language_server_time_sum Sum of time in milliseconds that language server requests take # TYPE language_server_time_sum gauge language_server_time_sum{id="redhat.vscode-yaml" method="textDocument/hover" result="success"} 33.35499999229796 language_server_time_sum{id="redhat.vscode-yaml" method="textDocument/hover" result="failure"} 0

I'm not sure this enough or what to look for.

@tsmaeder
Copy link
Contributor

Simply using /Request(.*)failed/ will not work, because of eager matching. I think I'll just cut off the prefix/postfix.

@vince-fugnitto
Copy link
Member

Closed by #6591

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser/firefox issues related to the firefox browser browser/safari issues related to the safari browser critical critical bugs / problems metrics issues related to metrics and logging plug-in system issues related to the plug-in system
Projects
None yet
Development

No branches or pull requests

3 participants