plugin-ext-metrics extension for creating metrics from language plugins #6303

JPinkney · 2019-10-01T14:24:18Z

What it does

This PR introduces a new extension that creates metrics from language plugins. It creates metrics for two specific instances:

The time it takes for a language server request to be made for a specific request for a specific plugin
E.g. vscode-yaml -> document symbols -> 13 ms
The percentage of success made for a specific request for a specific plugin
E.g. vscode-yaml -> document symbols -> 99% successful

Spectrum discussion: https://spectrum.chat/theia/dev/current-state-of-metrics-in-theia~8efd05d2-a25b-4629-a1b1-1dd8a0ccef96
Related issues:
eclipse-che/che#14245

How to test

For testing I used a modified version of vscode-yaml that errors randomly in document symbols (just so you don't have to go out and find a plugin that errors on a language server request): you can download from https://github.com/JPinkney/Sample-vsix-repo/blob/master/0.5.2-beta7.vsix

You can actually use anything in this list though: https://github.com/eclipse-theia/theia/compare/master...JPinkney:plugin-ext-metrics?expand=1#diff-fb56011c00b860bfb7d05eedd825006eR33 it's just easier to verify that the percentage of successes is working when you know how to generate an error in the plugin.

Play around with the extension (autocompletion, document symbols, etc, etc) just so that we can get some data for metrics.

After you've played around with it go to ${yourUrl}/metrics and you should see the updated analytics for language_server_success_metrics and language_server_time_metrics.

Review checklist

as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

as a reviewer, I agree to behave in accordance with the review guidelines

packages/plugin-ext-metrics/.gitignore

akosyakov · 2019-10-02T05:56:41Z

I see during the build:

The following dependency issues were detected in '@theia/plugin-ext-metrics':
 - error: Dependency 'vscode-jsonrpc' [4.0.0] was not hoisted to the root 'node_modules' folder. The same dependency already exists with version 4.1.0-next.3 at '/workspace/theia/node_modules/vscode-jsonrpc'.
 - error: Dependency 'vscode-languageserver-protocol' [3.14.1] was not hoisted to the root 'node_modules' folder. The same dependency already exists with version 3.15.0-next.8 at '/workspace/theia/node_modules/vscode-languageserver-protocol'.
 - error: Dependency 'vscode-languageserver-types' [3.14.0] was not hoisted to the root 'node_modules' folder. The same dependency already exists with version 3.15.0-next.4 at '/workspace/theia/node_modules/vscode-languageserver-types'.
⚠️  This is a reminder to fix the dependency issues.

Please use vscode-languageserver-types of the same version which is used in core, i.e. ^3.15.0-next. If i remember correctly it is required in order to package electron distr. cc @kittaakos

packages/plugin-ext-metrics/package.json

packages/plugin-ext-metrics/src/browser/plugin-metrics-languages-main.ts

packages/plugin-ext-metrics/src/browser/plugin-metrics-output-registry.ts

packages/plugin-ext-metrics/src/browser/plugin-metrics-languages-main.ts

packages/plugin-ext-metrics/src/browser/plugin-metrics-output-registry.ts

packages/plugin-ext-metrics/src/node/plugin-ext-metrics-backend-module.ts

packages/plugin-ext-metrics/README.md

akosyakov

I've looked through code, someone has to test that metrics are actually collected.

packages/plugin-ext-metrics/compile.tsconfig.json

packages/plugin-ext-metrics/src/common/index.ts

packages/plugin-ext-metrics/src/common/plugin-metrics-interfaces.ts

packages/plugin-ext-metrics/src/node/plugin-ext-metrics-backend-module.ts

packages/plugin-ext-metrics/src/node/plugin-metrics-handler.ts

packages/plugin-ext-metrics/src/node/metrics-contributor.ts

packages/plugin-ext-metrics/src/browser/plugin-metrics-resolver.ts

kittaakos · 2019-10-15T08:22:08Z

If i remember correctly it is required in order to package electron distr.

Correct. Here: https://spectrum.chat/theia/general/our-theia-electron-builder-app-no-longer-starts~f5cf09a0-6d88-448b-8818-24ad0ec2ee7c?m=MTU3MDcxNjg1OTMwNA==

packages/plugin-ext-metrics/src/common/metrics-protocol.ts

skabashnyuk · 2019-10-15T08:35:46Z

@JPinkney can you please add an output example of metrics endpoint?

packages/plugin-ext-metrics/src/common/plugin-metrics-interfaces.ts

JPinkney · 2019-10-21T14:17:00Z

@skabashnyuk example would be something like:

# HELP language_server_success_metrics Percentage of successful language requests # TYPE language_server_success_metrics gauge language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeLens"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/documentSymbol"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/completion"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/hover"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/formatting"} 88.88888888888889 language_server_success_metrics{id="ms-vscode.go" method="textDocument/signatureHelp"} 100

skabashnyuk · 2019-10-21T14:52:45Z

@JPinkney I see The percentage of success . What about The time it takes for a language server request?

JPinkney · 2019-10-21T14:56:17Z

It's also there, it just wasn't pasted in. Heres the full output:

# HELP language_server_success_metrics Percentage of successful language requests # TYPE language_server_success_metrics gauge language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeLens"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/documentSymbol"} 100 language_server_success_metrics{id="ms-vscode.go" method="textDocument/completion"} 100 # HELP language_server_time_metrics Number of milliseconds it takes on average for a language server request # TYPE language_server_time_metrics gauge language_server_time_metrics{id="ms-vscode.go" method="textDocument/codeAction"} 24.225000001024455 language_server_time_metrics{id="ms-vscode.go" method="textDocument/codeLens"} 3.427499992540106 language_server_time_metrics{id="ms-vscode.go" method="textDocument/documentSymbol"} 15.69250000466127 language_server_time_metrics{id="ms-vscode.go" method="textDocument/completion"} 314.4249999895692

packages/plugin-metrics/src/browser/index.ts

skabashnyuk · 2019-10-22T06:29:13Z

@JPinkney ok I see

# HELP language_server_success_metrics Percentage of successful language requests 
# TYPE language_server_success_metrics gauge 
language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction"} 100 
language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeLens"} 100 
language_server_success_metrics{id="ms-vscode.go" method="textDocument/documentSymbol"} 100
language_server_success_metrics{id="ms-vscode.go" method="textDocument/completion"} 100 
# HELP language_server_time_metrics Number of milliseconds it takes on average for a language server request 
# TYPE language_server_time_metrics gauge 
language_server_time_metrics{id="ms-vscode.go" method="textDocument/codeAction"} 24.225000001024455 
language_server_time_metrics{id="ms-vscode.go" method="textDocument/codeLens"} 3.427499992540106 
language_server_time_metrics{id="ms-vscode.go" method="textDocument/documentSymbol"} 15.69250000466127 
language_server_time_metrics{id="ms-vscode.go" method="textDocument/completion"} 314.4249999895692

I have such a question: instead of Percentage can we get raw counters? It would be much easier to calculate different derivative metrics from raw values on the Prometheus side.

JPinkney · 2019-10-22T12:56:27Z

@skabashnyuk For sure I can make that switch. How should I output the metric? like:
language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction" totalRequests="15"} 4

where 4 is the number of successful requests?

skabashnyuk · 2019-10-22T13:22:29Z

what about

language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction" result="fail" } 4
language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction" result="success" } 47629

skabashnyuk · 2019-10-23T14:52:03Z

where 4523423 is the sum of all time taken for requests?

yes

Would you also want the total requests as well? something like:
language_server_time_metrics{id="ms-vscode.go" method="textDocument/codeAction" totalRequests="4587"} 4523423

yes and no.

I want to see

language_server_time_sum{id="ms-vscode.go" method="textDocument/codeAction" resutl="success"} 3349590
language_server_time_count{id="ms-vscode.go" method="textDocument/codeAction" resutl="success"} 34345
language_server_time_sum{id="ms-vscode.go" method="textDocument/codeAction" resutl="fail"} 533
language_server_time_count{id="ms-vscode.go" method="textDocument/codeAction" resutl="fail"} 23

tsmaeder · 2019-10-24T08:05:15Z

@skabashnyuk so what you want are really not metrics, but raw events, right? If we go down that way, I would suggest we do it in a way that keeps computation of the metrics local to the front end: that would mean that the collection of events would be general, whereas the computation of metrics would be a che-specific Theia extension.

skabashnyuk · 2019-11-05T13:31:00Z

@skabashnyuk so what you want are really not metrics, but raw events, right?

Sort of. It's much easier to get different reports from raw data.

JPinkney · 2019-11-05T14:13:49Z

@skabashnyuk Regarding:

language_server_time_count{id="ms-vscode.go" method="textDocument/codeAction" result="success"} 34345

is 34345 just the number of times that we've measured time for a succesful request?

Would that be any different from this metric?

language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction" result="success" } 47629

skabashnyuk · 2019-11-05T15:24:21Z

@skabashnyuk Regarding:
language_server_time_count{id="ms-vscode.go" method="textDocument/codeAction" result="success"} 34345
is 34345 just the number of times that we've measured time for a succesful request?

yes

Would that be any different from this metric?

language_server_success_metrics{id="ms-vscode.go" method="textDocument/codeAction" result="success" } 47629

I'm not sure metrics language_server_success_metrics with a result attribute is a good thing to do.

Signed-off-by: Josh Pinkney <joshpinkney@gmail.com>

JPinkney · 2019-11-07T17:24:04Z

@skabashnyuk

I've updated the full metrics to be:

# HELP language_server_time_count Number of language server requests 
# TYPE language_server_time_count gauge
language_server_time_count{id="redhat.vscode-yaml" method="textDocument/documentSymbol" result="success"} 211
language_server_time_count{id="redhat.vscode-yaml" method="textDocument/documentSymbol" result="fail"} 0
language_server_time_count{id="redhat.vscode-yaml" method="textDocument/completion" result="success"} 100
language_server_time_count{id="redhat.vscode-yaml" method="textDocument/completion" 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/documentSymbol" result="success"} 7032.7149999943795
language_server_time_sum{id="redhat.vscode-yaml" method="textDocument/documentSymbol" result="failure"} 0
language_server_time_sum{id="redhat.vscode-yaml" method="textDocument/completion" result="success"} 2778.539999999688
language_server_time_sum{id="redhat.vscode-yaml" method="textDocument/completion" result="failure"} 0

Do those capture all the metrics needed?

skabashnyuk · 2019-11-07T18:12:02Z

Do those capture all the metrics needed?

I don't know, but they look fantastic.

skabashnyuk · 2019-11-13T07:39:01Z

Any plans for this pr do be merged?

JPinkney · 2019-11-13T14:16:14Z

@skabashnyuk Its just waiting for a +1

@akosyakov @benoitf Can you take a quick look again

akosyakov · 2019-11-13T15:30:59Z

Some files don't match types or don't have Plugin prefix, but it is minor. Generally I'm fine with changes.

skabashnyuk · 2019-11-18T09:23:53Z

@JPinkney can you take a look at @akosyakov's comment #6303 (comment) ?

akosyakov

@skabashnyuk it can be fixed as follow-ups, I forgot to select approve last time 🙈

tsmaeder · 2019-11-18T12:28:21Z

@akosyakov, @skabashnyuk @JPinkney is away this week. Should I press "rebase and merge" before it needs another rebase?

akosyakov · 2019-11-18T14:43:11Z

Should I press "rebase and merge" before it needs another rebase?

+1 It looks like it can be auto-rebased. The build failure on Mac seems to be accidental.

tolusha · 2019-11-18T15:31:31Z

PR was outdated and build failed

akosyakov · 2019-11-18T15:39:32Z

@tolusha Oh, it broke the master:

 yarn run clean && yarn run build
$ theiaext clean
$ theiaext build
[compile] src/browser/plugin-metrics-frontend-module.ts(31,34): error TS2345: Argument of type 'typeof LanguagesMainPluginMetrics' is not assignable to parameter of type 'new (...args: any[]) => LanguagesMainImpl'.
[compile]   Type 'LanguagesMainPluginMetrics' is not assignable to type 'LanguagesMainImpl'.
[compile]     Types of property 'provideCodeActions' are incompatible.
[compile]       Type '(handle: number, model: ITextModel, rangeOrSelection: Range, context: CodeActionContext, token: CancellationToken) => Promise<CodeActionList | Promise<CodeActionList>>' is not assignable to type '(handle: number, model: ITextModel, rangeOrSelection: Range, context: CodeActionContext, token: CancellationToken) => Promise<CodeActionList>'.
[compile]         Type 'Promise<CodeActionList | Promise<CodeActionList>>' is not assignable to type 'Promise<CodeActionList>'.
[compile] src/browser/plugin-metrics-languages-main.ts(194,21): error TS2416: Property 'provideCodeActions' in type 'LanguagesMainPluginMetrics' is not assignable to the same property in base type 'LanguagesMainImpl'.
[compile]   Type '(handle: number, model: ITextModel, rangeOrSelection: Range, context: CodeActionContext, token: CancellationToken) => Promise<CodeActionList | Promise<CodeActionList>>' is not assignable to type '(handle: number, model: ITextModel, rangeOrSelection: Range, context: CodeActionContext, token: CancellationToken) => Promise<CodeActionList>'.
[compile]     Type 'Promise<CodeActionList | Promise<CodeActionList>>' is not assignable to type 'Promise<CodeActionList>'.
[compile]       Type 'CodeActionList | Promise<CodeActionList>' is not assignable to type 'CodeActionList'.
[compile]         Type 'Promise<CodeActionList>' is missing the following properties from type 'CodeActionList': actions, dispose
[compile] theiaext compile exited with code 2
[lint] theiaext lint exited with code 0
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Could you have a look at it or revert please?

tsmaeder · 2019-11-18T15:54:58Z

Looks like the type of LanguagesMainImpl changed 3 days ago to

Promise<monaco.languages.CodeActionList | monaco.languages.CodeActionList>

Maybe @svenefftinge knows the correct fix to apply then?

tsmaeder · 2019-11-18T16:07:55Z

Changing the return type in LanguagesMainPluginMetrics to Promise<monaco.languages.CodeActionList> fixes the build

tsmaeder · 2019-11-18T16:08:16Z

@akosyakov how to best provide a fix for this?

tolusha · 2019-11-18T16:08:35Z

I will create a PR. The fix seems simple

tolusha · 2019-11-18T16:11:44Z

#6571

paul-marechal · 2019-11-19T15:09:37Z

packages/plugin-metrics/src/browser/plugin-metrics-creator.ts

+
+    private _extensionIDAnalytics: MetricsMap;
+
+    private NODE_BASED_REGEX = /(?<=Request)(.*?)(?=failed)/;


Why not /Request(.*)failed/?

JPinkney force-pushed the plugin-ext-metrics branch from 59b95c4 to dff03f3 Compare October 1, 2019 14:24

benoitf reviewed Oct 1, 2019

View reviewed changes

packages/plugin-ext-metrics/.gitignore Outdated Show resolved Hide resolved

JPinkney force-pushed the plugin-ext-metrics branch from dff03f3 to 803fc2f Compare October 1, 2019 17:22

akosyakov added metrics issues related to metrics and logging plug-in system issues related to the plug-in system labels Oct 2, 2019

akosyakov reviewed Oct 2, 2019

View reviewed changes

JPinkney force-pushed the plugin-ext-metrics branch from 803fc2f to 832ca8a Compare October 2, 2019 13:44

kittaakos reviewed Oct 7, 2019

View reviewed changes

packages/plugin-ext-metrics/README.md Outdated Show resolved Hide resolved

packages/plugin-ext-metrics/README.md Outdated Show resolved Hide resolved

packages/plugin-ext-metrics/README.md Outdated Show resolved Hide resolved

JPinkney force-pushed the plugin-ext-metrics branch from 832ca8a to 7866d94 Compare October 11, 2019 17:47

akosyakov reviewed Oct 15, 2019

View reviewed changes

akosyakov requested review from evidolob, azatsarynnyy, benoitf, tolusha and tsmaeder October 15, 2019 07:58

benoitf reviewed Oct 15, 2019

View reviewed changes

packages/plugin-ext-metrics/src/common/metrics-protocol.ts Outdated Show resolved Hide resolved

benoitf reviewed Oct 15, 2019

View reviewed changes

packages/plugin-ext-metrics/src/common/plugin-metrics-interfaces.ts Outdated Show resolved Hide resolved

skabashnyuk mentioned this pull request Oct 15, 2019

[MVP] Collecting Che Workspace metrics in single cluster mode with Prometheus eclipse-che/che#14888

Closed

JPinkney force-pushed the plugin-ext-metrics branch from 7866d94 to ca66fcf Compare October 21, 2019 14:17

akosyakov reviewed Oct 22, 2019

View reviewed changes

packages/plugin-metrics/src/browser/index.ts Outdated Show resolved Hide resolved

JPinkney force-pushed the plugin-ext-metrics branch from ca66fcf to 660916f Compare October 22, 2019 20:36

JPinkney force-pushed the plugin-ext-metrics branch from 81c0952 to 428b13a Compare October 23, 2019 20:36

tsmaeder mentioned this pull request Nov 6, 2019

Languages Sprint 175 eclipse-che/che#15049

Closed

25 tasks

Created plugin-ext-metrics for creating metrics from language plugins

b1a666a

Signed-off-by: Josh Pinkney <joshpinkney@gmail.com>

JPinkney force-pushed the plugin-ext-metrics branch from 428b13a to b1a666a Compare November 7, 2019 16:57

akosyakov approved these changes Nov 18, 2019

View reviewed changes

tsmaeder merged commit 7a1aa74 into eclipse-theia:master Nov 18, 2019

paul-marechal reviewed Nov 19, 2019

View reviewed changes

This was referenced Nov 20, 2019

plugin metrics breaks application start up in Firefox #6582

Closed

Don't use lookbehind regex #6591

Merged

Languages Sprint 176 eclipse-che/che#15269

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plugin-ext-metrics extension for creating metrics from language plugins #6303

plugin-ext-metrics extension for creating metrics from language plugins #6303

JPinkney commented Oct 1, 2019

akosyakov commented Oct 2, 2019

akosyakov left a comment

kittaakos commented Oct 15, 2019

skabashnyuk commented Oct 15, 2019

JPinkney commented Oct 21, 2019

skabashnyuk commented Oct 21, 2019

JPinkney commented Oct 21, 2019

skabashnyuk commented Oct 22, 2019

JPinkney commented Oct 22, 2019

skabashnyuk commented Oct 22, 2019

skabashnyuk commented Oct 23, 2019

tsmaeder commented Oct 24, 2019

skabashnyuk commented Nov 5, 2019

JPinkney commented Nov 5, 2019

skabashnyuk commented Nov 5, 2019 •

edited

Loading

JPinkney commented Nov 7, 2019

skabashnyuk commented Nov 7, 2019

skabashnyuk commented Nov 13, 2019

JPinkney commented Nov 13, 2019

akosyakov commented Nov 13, 2019

skabashnyuk commented Nov 18, 2019

akosyakov left a comment •

edited

Loading

tsmaeder commented Nov 18, 2019

akosyakov commented Nov 18, 2019 •

edited

Loading

tolusha commented Nov 18, 2019

akosyakov commented Nov 18, 2019

tsmaeder commented Nov 18, 2019

tsmaeder commented Nov 18, 2019 •

edited

Loading

tsmaeder commented Nov 18, 2019

tolusha commented Nov 18, 2019

tolusha commented Nov 18, 2019

paul-marechal Nov 19, 2019 •

edited

Loading


		private _extensionIDAnalytics: MetricsMap;

		private NODE_BASED_REGEX = /(?<=Request)(.*?)(?=failed)/;

plugin-ext-metrics extension for creating metrics from language plugins #6303

plugin-ext-metrics extension for creating metrics from language plugins #6303

Conversation

JPinkney commented Oct 1, 2019

What it does

How to test

Review checklist

Reminder for reviewers

akosyakov commented Oct 2, 2019

akosyakov left a comment

Choose a reason for hiding this comment

kittaakos commented Oct 15, 2019

skabashnyuk commented Oct 15, 2019

JPinkney commented Oct 21, 2019

skabashnyuk commented Oct 21, 2019

JPinkney commented Oct 21, 2019

skabashnyuk commented Oct 22, 2019

JPinkney commented Oct 22, 2019

skabashnyuk commented Oct 22, 2019

skabashnyuk commented Oct 23, 2019

tsmaeder commented Oct 24, 2019

skabashnyuk commented Nov 5, 2019

JPinkney commented Nov 5, 2019

skabashnyuk commented Nov 5, 2019 • edited Loading

JPinkney commented Nov 7, 2019

skabashnyuk commented Nov 7, 2019

skabashnyuk commented Nov 13, 2019

JPinkney commented Nov 13, 2019

akosyakov commented Nov 13, 2019

skabashnyuk commented Nov 18, 2019

akosyakov left a comment • edited Loading

Choose a reason for hiding this comment

tsmaeder commented Nov 18, 2019

akosyakov commented Nov 18, 2019 • edited Loading

tolusha commented Nov 18, 2019

akosyakov commented Nov 18, 2019

tsmaeder commented Nov 18, 2019

tsmaeder commented Nov 18, 2019 • edited Loading

tsmaeder commented Nov 18, 2019

tolusha commented Nov 18, 2019

tolusha commented Nov 18, 2019

paul-marechal Nov 19, 2019 • edited Loading

Choose a reason for hiding this comment

skabashnyuk commented Nov 5, 2019 •

edited

Loading

akosyakov left a comment •

edited

Loading

akosyakov commented Nov 18, 2019 •

edited

Loading

tsmaeder commented Nov 18, 2019 •

edited

Loading

paul-marechal Nov 19, 2019 •

edited

Loading