Skip to content

Commit

Permalink
Clean nb context menu telemetry with accurate from fields (#229241)
Browse files Browse the repository at this point in the history
* clean nb context menu telemetry with accurate `from` fields

* comments

* consolidate telemetry

* get telem service if context

* avoid early returns
  • Loading branch information
Yoyokrazy authored Sep 25, 2024
1 parent 3089797 commit fd83a56
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 23 deletions.
49 changes: 27 additions & 22 deletions src/vs/workbench/contrib/notebook/browser/controller/coreActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,20 +156,15 @@ export abstract class NotebookAction extends Action2 {
}

async run(accessor: ServicesAccessor, context?: any, ...additionalArgs: any[]): Promise<void> {
const isFromUI = !!context;
const from = isFromUI ? (this.isNotebookActionContext(context) ? 'notebookToolbar' : 'editorToolbar') : undefined;
sendEntryTelemetry(accessor, this.desc.id, context);

if (!this.isNotebookActionContext(context)) {
context = this.getEditorContextFromArgsOrActive(accessor, context, ...additionalArgs);
if (!context) {
return;
}
}

if (from !== undefined) {
const telemetryService = accessor.get(ITelemetryService);
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: this.desc.id, from: from });
}

return this.runWithContext(accessor, context);
}

Expand Down Expand Up @@ -217,10 +212,6 @@ export abstract class NotebookMultiCellAction extends Action2 {

abstract runWithContext(accessor: ServicesAccessor, context: INotebookCommandContext | INotebookCellToolbarActionContext): Promise<void>;

private isCellToolbarContext(context?: unknown): context is INotebookCellToolbarActionContext {
return !!context && !!(context as INotebookActionContext).notebookEditor && (context as any).$mid === MarshalledId.NotebookCellActionContext;
}

/**
* The action/command args are resolved in following order
* `run(accessor, cellToolbarContext)` from cell toolbar
Expand All @@ -229,21 +220,17 @@ export abstract class NotebookMultiCellAction extends Action2 {
*/
async run(accessor: ServicesAccessor, ...additionalArgs: any[]): Promise<void> {
const context = additionalArgs[0];
const isFromCellToolbar = this.isCellToolbarContext(context);
const isFromEditorToolbar = isEditorCommandsContext(context);
const from = isFromCellToolbar ? 'cellToolbar' : (isFromEditorToolbar ? 'editorToolbar' : 'other');
const telemetryService = accessor.get(ITelemetryService);

sendEntryTelemetry(accessor, this.desc.id, context);

const isFromCellToolbar = isCellToolbarContext(context);
if (isFromCellToolbar) {
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: this.desc.id, from: from });
return this.runWithContext(accessor, context);
}

// handle parsed args

const parsedArgs = this.parseArgs(accessor, ...additionalArgs);
if (parsedArgs) {
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: this.desc.id, from: from });
return this.runWithContext(accessor, parsedArgs);
}

Expand All @@ -252,7 +239,6 @@ export abstract class NotebookMultiCellAction extends Action2 {
if (editor) {
const selectedCellRange: ICellRange[] = editor.getSelections().length === 0 ? [editor.getFocus()] : editor.getSelections();

telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: this.desc.id, from: from });

return this.runWithContext(accessor, {
ui: false,
Expand All @@ -273,10 +259,9 @@ export abstract class NotebookCellAction<T = INotebookCellActionContext> extends
}

override async run(accessor: ServicesAccessor, context?: INotebookCellActionContext, ...additionalArgs: any[]): Promise<void> {
if (this.isCellActionContext(context)) {
const telemetryService = accessor.get(ITelemetryService);
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: this.desc.id, from: 'cellToolbar' });
sendEntryTelemetry(accessor, this.desc.id, context);

if (this.isCellActionContext(context)) {
return this.runWithContext(accessor, context);
}

Expand All @@ -303,6 +288,26 @@ interface IMultiCellArgs {
autoReveal?: boolean;
}

function sendEntryTelemetry(accessor: ServicesAccessor, id: string, context?: any) {
if (context) {
const telemetryService = accessor.get(ITelemetryService);
if (context.source) {
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: id, from: context.source });
} else if (URI.isUri(context)) {
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: id, from: 'cellEditorContextMenu' });
} else if (context && 'from' in context && context.from === 'cellContainer') {
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: id, from: 'cellContainer' });
} else {
const from = isCellToolbarContext(context) ? 'cellToolbar' : (isEditorCommandsContext(context) ? 'editorToolbar' : 'other');
telemetryService.publicLog2<WorkbenchActionExecutedEvent, WorkbenchActionExecutedClassification>('workbenchActionExecuted', { id: id, from: from });
}
}
}

function isCellToolbarContext(context?: unknown): context is INotebookCellToolbarActionContext {
return !!context && !!(context as INotebookActionContext).notebookEditor && (context as any).$mid === MarshalledId.NotebookCellActionContext;
}

function isMultiCellArgs(arg: unknown): arg is IMultiCellArgs {
if (arg === undefined) {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1066,8 +1066,16 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
private showListContextMenu(e: IListContextMenuEvent<CellViewModel>) {
this.contextMenuService.showContextMenu({
menuId: MenuId.NotebookCellTitle,
menuActionOptions: {
shouldForwardArgs: true
},
contextKeyService: this.scopedContextKeyService,
getAnchor: () => e.anchor
getAnchor: () => e.anchor,
getActionsContext: () => {
return {
from: 'cellContainer'
};
}
});
}

Expand Down

0 comments on commit fd83a56

Please sign in to comment.