-
Notifications
You must be signed in to change notification settings - Fork 407
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
Add metrics to commands being executed #549
Conversation
@@ -9,11 +9,13 @@ export class Command { | |||
public readonly command: string; | |||
public readonly description?: string; | |||
public readonly args: string[]; | |||
public readonly logName?: string; |
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.
Expanding the command builder to support adding log name when we build the command.
@@ -338,6 +339,10 @@ export abstract class SfdxCommandletExecutor<T> | |||
taskViewService.addCommandExecution(execution, cancellationTokenSource); | |||
} | |||
|
|||
public logMetric(logName?: string) { |
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.
New method that will log command metrics if a logName was declared. Will be "automatically" called if execute
is not overwritten, else we'll have to call it from the new execute
.
@@ -68,6 +69,7 @@ export class ForceApexLogGetExecutor extends SfdxCommandletExecutor< | |||
cwd: vscode.workspace.rootPath | |||
}).execute(cancellationToken); | |||
this.attachExecution(execution, cancellationTokenSource, cancellationToken); | |||
this.logMetric(execution.command.logName); |
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.
Directly calling logMetric since we are not leveraging SfdxCommandletExecutor
.
There are certain commands we use that are built & executed on the utilities packages that I'm currently not addressing since I didn't want to add a circular dependency because of where we have the telemetryService. E.g. FauxCommandGenerator executes a couple of commands and it is part of @vazexqi do you have any suggestions for the scenario described above ? |
@@ -79,7 +79,8 @@ export class ForceApexTestRunCodeActionExecutor extends SfdxCommandletExecutor<{ | |||
.withFlag('--tests', this.test) | |||
.withFlag('--resultformat', 'human') | |||
.withArg('--synchronous') | |||
.withFlag('--loglevel', 'error'); | |||
.withFlag('--loglevel', 'error') | |||
.withLogName('force_apex_test_run_codeaction'); |
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.
Should this be 'force_apex_test_run_code_action'?
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.
Yeah, let me change that
Codecov Report
@@ Coverage Diff @@
## develop #549 +/- ##
===========================================
- Coverage 72.27% 72.09% -0.18%
===========================================
Files 155 155
Lines 6128 6157 +29
Branches 953 953
===========================================
+ Hits 4429 4439 +10
- Misses 1466 1485 +19
Partials 233 233
Continue to review full report at Codecov.
|
e21477a
to
fe3de9c
Compare
… setting metric log name while building a command.
fe3de9c
to
1bb6cce
Compare
What does this PR do?
Adds telemetry for command execution.
What issues does this PR fix or reference?
@W-5280764@