fix(editor): Reduce telemetry events for builder workflow changes (no-changelog) (#19310)

This commit is contained in:
Mutasem Aldmour
2025-09-09 10:57:50 +02:00
committed by GitHub
parent e87344d97d
commit 99293ea400
7 changed files with 471 additions and 62 deletions

View File

@@ -1355,4 +1355,203 @@ describe('AskAssistantBuild', () => {
expect(chatInput).not.toHaveAttribute('disabled');
});
});
it('should handle multiple canvas generations correctly', async () => {
const originalWorkflow = {
nodes: [],
connections: {},
};
builderStore.getWorkflowSnapshot.mockReturnValue(JSON.stringify(originalWorkflow));
const intermediaryWorkflow = {
nodes: [
{
id: 'node1',
name: 'Start',
type: 'n8n-nodes-base.start',
position: [0, 0],
typeVersion: 1,
parameters: {},
} as INodeUi,
],
connections: {},
};
const finalWorkflow = {
nodes: [
{
id: 'node1',
name: 'Start',
type: 'n8n-nodes-base.start',
position: [0, 0],
typeVersion: 1,
parameters: {},
} as INodeUi,
{
id: 'node2',
name: 'HttpReuqest',
type: 'n8n-nodes-base.httpRequest',
position: [0, 0],
typeVersion: 1,
parameters: {},
} as INodeUi,
],
connections: {},
};
workflowsStore.$patch({
workflow: originalWorkflow,
});
renderComponent();
builderStore.$patch({ streaming: true });
await flushPromises();
// Trigger the watcher by updating workflowMessages
builderStore.workflowMessages = [
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'workflow-updated' as const,
codeSnippet: JSON.stringify(intermediaryWorkflow),
},
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'workflow-updated' as const,
codeSnippet: JSON.stringify(finalWorkflow),
},
];
const toolCallId1_1 = '1234';
const toolCallId1_2 = '3333';
const toolCallId2 = '4567';
const toolCallId3 = '8901';
builderStore.toolMessages = [
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'tool' as const,
toolName: 'first-tool',
toolCallId: toolCallId1_1,
status: 'completed',
updates: [],
},
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'tool' as const,
toolName: 'first-tool',
toolCallId: toolCallId1_2,
status: 'completed',
updates: [],
},
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'tool' as const,
toolName: 'second-tool',
toolCallId: toolCallId2,
status: 'running',
updates: [],
},
];
builderStore.$patch({ streaming: false });
await flushPromises();
expect(trackMock).toHaveBeenCalledOnce();
expect(trackMock).toHaveBeenCalledWith('Workflow modified by builder', {
end_workflow_json: JSON.stringify(finalWorkflow),
session_id: 'app_session_id',
start_workflow_json: JSON.stringify(originalWorkflow),
// first-tool is added once, even though it completed twice
// second-tool is ignored because it's running
tools_called: ['first-tool'],
workflow_id: 'abc123',
});
trackMock.mockClear();
builderStore.$patch({ streaming: true });
await flushPromises();
// second run after new messages
const updatedWorkflow2 = {
...finalWorkflow,
nodes: [
...finalWorkflow.nodes,
{
id: 'node1',
name: 'Updated',
type: 'n8n-nodes-base.updated',
position: [0, 0],
typeVersion: 1,
parameters: {},
},
],
};
builderStore.workflowMessages = [
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'workflow-updated' as const,
codeSnippet: JSON.stringify(updatedWorkflow2),
},
];
builderStore.toolMessages = [
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'tool' as const,
toolName: 'first-tool',
toolCallId: toolCallId1_1,
status: 'completed',
updates: [],
},
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'tool' as const,
toolName: 'first-tool',
toolCallId: toolCallId1_2,
status: 'completed',
updates: [],
},
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'tool' as const,
toolName: 'second-tool',
toolCallId: toolCallId2,
status: 'completed',
updates: [],
},
{
id: faker.string.uuid(),
role: 'assistant' as const,
type: 'tool' as const,
toolName: 'third-tool',
toolCallId: toolCallId3,
status: 'completed',
updates: [],
},
];
builderStore.$patch({ streaming: false });
await flushPromises();
expect(trackMock).toHaveBeenCalledOnce();
expect(trackMock).toHaveBeenCalledWith('Workflow modified by builder', {
end_workflow_json: JSON.stringify(updatedWorkflow2),
session_id: 'app_session_id',
start_workflow_json: JSON.stringify(originalWorkflow),
// first-tool is ignored, because it was tracked in first run (same tool call id)
tools_called: ['second-tool', 'third-tool'],
workflow_id: 'abc123',
});
});
});

View File

@@ -33,6 +33,7 @@ const processedWorkflowUpdates = ref(new Set<string>());
const trackedTools = ref(new Set<string>());
const planStatus = ref<'pending' | 'approved' | 'rejected'>();
const assistantChatRef = ref<InstanceType<typeof AskAssistantChat> | null>(null);
const workflowUpdated = ref<{ start: string; end: string } | undefined>();
const user = computed(() => ({
firstName: usersStore.currentUser?.firstName ?? '',
@@ -60,6 +61,7 @@ function onNewWorkflow() {
builderStore.resetBuilderChat();
processedWorkflowUpdates.value.clear();
trackedTools.value.clear();
workflowUpdated.value = undefined;
}
function onFeedback(feedback: RatingFeedback) {
@@ -113,6 +115,33 @@ function shouldShowPlanControls(message: NodesPlanMessageType) {
return planMessageIndex === builderStore.chatMessages.length - 1;
}
function dedupeToolNames(toolNames: string[]): string[] {
return [...new Set(toolNames)];
}
function trackWorkflowModifications() {
if (workflowUpdated.value) {
// Track tool usage for telemetry
const newToolMessages = builderStore.toolMessages.filter(
(toolMsg) =>
toolMsg.status !== 'running' &&
toolMsg.toolCallId &&
!trackedTools.value.has(toolMsg.toolCallId),
);
newToolMessages.forEach((toolMsg) => trackedTools.value.add(toolMsg.toolCallId ?? ''));
telemetry.track('Workflow modified by builder', {
tools_called: dedupeToolNames(newToolMessages.map((toolMsg) => toolMsg.toolName)),
session_id: builderStore.trackingSessionId,
start_workflow_json: workflowUpdated.value.start,
end_workflow_json: workflowUpdated.value.end,
workflow_id: workflowsStore.workflowId,
});
workflowUpdated.value = undefined;
}
}
// Watch for workflow updates and apply them
watch(
() => builderStore.workflowMessages,
@@ -125,7 +154,8 @@ watch(
if (msg.id && isWorkflowUpdatedMessage(msg)) {
processedWorkflowUpdates.value.add(msg.id);
const currentWorkflowJson = builderStore.getWorkflowSnapshot();
const originalWorkflowJson =
workflowUpdated.value?.start ?? builderStore.getWorkflowSnapshot();
const result = builderStore.applyWorkflowUpdate(msg.codeSnippet);
if (result.success) {
@@ -135,25 +165,13 @@ watch(
tidyUp: true,
nodesIdsToTidyUp: result.newNodeIds,
regenerateIds: false,
trackEvents: false,
});
// Track tool usage for telemetry
const newToolMessages = builderStore.toolMessages.filter(
(toolMsg) =>
toolMsg.status !== 'running' &&
toolMsg.toolCallId &&
!trackedTools.value.has(toolMsg.toolCallId),
);
newToolMessages.forEach((toolMsg) => trackedTools.value.add(toolMsg.toolCallId ?? ''));
telemetry.track('Workflow modified by builder', {
tools_called: newToolMessages.map((toolMsg) => toolMsg.toolName),
session_id: builderStore.trackingSessionId,
start_workflow_json: currentWorkflowJson,
end_workflow_json: msg.codeSnippet,
workflow_id: workflowsStore.workflowId,
});
workflowUpdated.value = {
start: originalWorkflowJson,
end: msg.codeSnippet,
};
}
}
});
@@ -171,10 +189,14 @@ watch(
// we want to save the workflow
watch(
() => builderStore.streaming,
async () => {
async (isStreaming) => {
if (!isStreaming) {
trackWorkflowModifications();
}
if (
builderStore.initialGeneration &&
!builderStore.streaming &&
!isStreaming &&
workflowsStore.workflow.nodes.length > 0
) {
// Check if the generation completed successfully (no error or cancellation)

View File

@@ -1,6 +1,6 @@
<script lang="ts" setup>
import ContextMenu from '@/components/ContextMenu/ContextMenu.vue';
import type { CanvasLayoutEvent, CanvasLayoutSource } from '@/composables/useCanvasLayout';
import type { CanvasLayoutEvent } from '@/composables/useCanvasLayout';
import { useCanvasLayout } from '@/composables/useCanvasLayout';
import { useCanvasNodeHover } from '@/composables/useCanvasNodeHover';
import { useCanvasTraversal } from '@/composables/useCanvasTraversal';
@@ -103,7 +103,7 @@ const emit = defineEmits<{
'save:workflow': [];
'create:workflow': [];
'drag-and-drop': [position: XYPosition, event: DragEvent];
'tidy-up': [CanvasLayoutEvent];
'tidy-up': [CanvasLayoutEvent, { trackEvents?: boolean }];
'toggle:focus-panel': [];
'viewport:change': [viewport: ViewportTransform, dimensions: Dimensions];
'selection:end': [position: XYPosition];
@@ -757,7 +757,7 @@ async function onContextMenuAction(action: ContextMenuAction, nodeIds: string[])
}
}
async function onTidyUp(payload: { source: CanvasLayoutSource; nodeIdsFilter?: string[] }) {
async function onTidyUp(payload: CanvasEventBusEvents['tidyUp']) {
if (payload.nodeIdsFilter && payload.nodeIdsFilter.length > 0) {
clearSelectedNodes();
addSelectedNodes(payload.nodeIdsFilter.map(findNode).filter(isPresent));
@@ -766,7 +766,7 @@ async function onTidyUp(payload: { source: CanvasLayoutSource; nodeIdsFilter?: s
const target = applyOnSelection ? 'selection' : 'all';
const result = layout(target);
emit('tidy-up', { result, target, source: payload.source });
emit('tidy-up', { result, target, source: payload.source }, { trackEvents: payload.trackEvents });
if (!applyOnSelection) {
await nextTick();

View File

@@ -119,10 +119,10 @@ describe('useCanvasOperations', () => {
};
beforeEach(() => {
vi.clearAllMocks();
const pinia = createTestingPinia({ initialState });
setActivePinia(pinia);
vi.clearAllMocks();
});
describe('requireNodeTypeDescription', () => {
@@ -717,6 +717,71 @@ describe('useCanvasOperations', () => {
target: 'all',
});
});
it('should send telemetry event when trackEvents is true', () => {
const event: CanvasLayoutEvent = {
source: 'canvas-button',
target: 'all',
result: {
nodes: [
{ id: 'node1', x: 96, y: 96 },
{ id: 'node2', x: 208, y: 208 },
],
boundingBox: { height: 96, width: 96, x: 0, y: 0 },
},
};
const { tidyUp } = useCanvasOperations();
tidyUp(event, { trackEvents: true });
expect(useTelemetry().track).toHaveBeenCalledWith('User tidied up canvas', {
nodes_count: 2,
source: 'canvas-button',
target: 'all',
});
});
it('should not send telemetry event when trackEvents is false', () => {
const event: CanvasLayoutEvent = {
source: 'canvas-button',
target: 'all',
result: {
nodes: [
{ id: 'node1', x: 96, y: 96 },
{ id: 'node2', x: 208, y: 208 },
],
boundingBox: { height: 96, width: 96, x: 0, y: 0 },
},
};
const { tidyUp } = useCanvasOperations();
tidyUp(event, { trackEvents: false });
expect(useTelemetry().track).not.toHaveBeenCalled();
});
it('should send telemetry event when trackEvents is undefined (default behavior)', () => {
const event: CanvasLayoutEvent = {
source: 'canvas-button',
target: 'all',
result: {
nodes: [
{ id: 'node1', x: 96, y: 96 },
{ id: 'node2', x: 208, y: 208 },
],
boundingBox: { height: 96, width: 96, x: 0, y: 0 },
},
};
const { tidyUp } = useCanvasOperations();
tidyUp(event, {}); // No trackEvents specified, should default to true
expect(useTelemetry().track).toHaveBeenCalledWith('User tidied up canvas', {
nodes_count: 2,
source: 'canvas-button',
target: 'all',
});
});
});
describe('updateNodePosition', () => {
@@ -3398,6 +3463,116 @@ describe('useCanvasOperations', () => {
});
});
describe('importWorkflowData', () => {
const workflowData = {
nodes: [], //buildImportNodes(),
connections: {},
};
it('should track telemetry when trackEvents is true (default)', async () => {
const telemetry = useTelemetry();
const canvasOperations = useCanvasOperations();
await canvasOperations.importWorkflowData(workflowData, 'paste');
expect(telemetry.track).toHaveBeenCalledWith('User pasted nodes', {
workflow_id: expect.any(String),
node_graph_string: expect.any(String),
});
});
it('should track telemetry when trackEvents is explicitly true', async () => {
const telemetry = useTelemetry();
const canvasOperations = useCanvasOperations();
await canvasOperations.importWorkflowData(workflowData, 'duplicate', {
trackEvents: true,
});
expect(telemetry.track).toHaveBeenCalledWith('User duplicated nodes', {
workflow_id: expect.any(String),
node_graph_string: expect.any(String),
});
});
it('should not track telemetry when trackEvents is false', async () => {
const telemetry = useTelemetry();
const canvasOperations = useCanvasOperations();
await canvasOperations.importWorkflowData(workflowData, 'paste', {
trackEvents: false,
});
expect(telemetry.track).not.toHaveBeenCalledWith('User pasted nodes', expect.any(Object));
});
it('should track different telemetry events for different sources with trackEvents true', async () => {
const telemetry = useTelemetry();
const canvasOperations = useCanvasOperations();
// Test 'paste' source
await canvasOperations.importWorkflowData(workflowData, 'paste', { trackEvents: true });
expect(telemetry.track).toHaveBeenCalledWith('User pasted nodes', {
workflow_id: expect.any(String),
node_graph_string: expect.any(String),
});
});
it('should track duplicate telemetry when source is duplicate with trackEvents true', async () => {
const telemetry = useTelemetry();
const canvasOperations = useCanvasOperations();
// Test 'duplicate' source
await canvasOperations.importWorkflowData(workflowData, 'duplicate', { trackEvents: true });
expect(telemetry.track).toHaveBeenCalledWith('User duplicated nodes', {
workflow_id: expect.any(String),
node_graph_string: expect.any(String),
});
});
it('should track import telemetry when source is file with trackEvents true', async () => {
const telemetry = useTelemetry();
const canvasOperations = useCanvasOperations();
// Test other source
await canvasOperations.importWorkflowData(workflowData, 'file', { trackEvents: true });
expect(telemetry.track).toHaveBeenCalledWith('User imported workflow', {
source: 'file',
workflow_id: expect.any(String),
node_graph_string: expect.any(String),
});
});
it('should not track any telemetry events when trackEvents is false regardless of source', async () => {
const telemetry = useTelemetry();
const canvasOperations = useCanvasOperations();
// Test 'paste' source with trackEvents: false
await canvasOperations.importWorkflowData(workflowData, 'paste', {
trackEvents: false,
});
expect(telemetry.track).not.toHaveBeenCalledWith('User pasted nodes', expect.any(Object));
// Test 'duplicate' source with trackEvents: false
await canvasOperations.importWorkflowData(workflowData, 'duplicate', {
trackEvents: false,
});
expect(telemetry.track).not.toHaveBeenCalledWith('User duplicated nodes', expect.any(Object));
// Test other source with trackEvents: false
await canvasOperations.importWorkflowData(workflowData, 'file', {
trackEvents: false,
});
expect(telemetry.track).not.toHaveBeenCalledWith(
'User imported workflow',
expect.any(Object),
);
// Ensure no telemetry was called at all
expect(telemetry.track).not.toHaveBeenCalled();
});
});
describe('duplicateNodes', () => {
it('should duplicate nodes', async () => {
const workflowsStore = mockedStore(useWorkflowsStore);
@@ -3693,7 +3868,7 @@ describe('useCanvasOperations', () => {
nodeTypesStore.getNodeType = vi.fn().mockReturnValue(nodeTypeDescription);
});
afterEach(() => {
vi.clearAllMocks();
// Mock cleanup handled automatically by test isolation
});
describe('common cases', () => {

View File

@@ -186,12 +186,18 @@ export function useCanvasOperations() {
* Node operations
*/
function tidyUp({ result, source, target }: CanvasLayoutEvent) {
function tidyUp(
{ result, source, target }: CanvasLayoutEvent,
{ trackEvents = true }: { trackEvents?: boolean } = {},
) {
updateNodesPosition(
result.nodes.map(({ id, x, y }) => ({ id, position: { x, y } })),
{ trackBulk: true, trackHistory: true },
);
trackTidyUp({ result, source, target });
if (trackEvents) {
trackTidyUp({ result, source, target });
}
}
function trackTidyUp({ result, source, target }: CanvasLayoutEvent) {
@@ -1874,12 +1880,14 @@ export function useCanvasOperations() {
trackHistory = true,
viewport,
regenerateIds = true,
trackEvents = true,
}: {
importTags?: boolean;
trackBulk?: boolean;
trackHistory?: boolean;
regenerateIds?: boolean;
viewport?: ViewportBoundaries;
trackEvents?: boolean;
} = {},
): Promise<WorkflowDataUpdate> {
uiStore.resetLastInteractedWith();
@@ -1937,37 +1945,39 @@ export function useCanvasOperations() {
removeUnknownCredentials(workflowData);
try {
const nodeGraph = JSON.stringify(
TelemetryHelpers.generateNodesGraph(
workflowData as IWorkflowBase,
workflowHelpers.getNodeTypes(),
{
nodeIdMap,
sourceInstanceId:
workflowData.meta && workflowData.meta.instanceId !== rootStore.instanceId
? workflowData.meta.instanceId
: '',
isCloudDeployment: settingsStore.isCloudDeployment,
},
).nodeGraph,
);
if (trackEvents) {
const nodeGraph = JSON.stringify(
TelemetryHelpers.generateNodesGraph(
workflowData as IWorkflowBase,
workflowHelpers.getNodeTypes(),
{
nodeIdMap,
sourceInstanceId:
workflowData.meta && workflowData.meta.instanceId !== rootStore.instanceId
? workflowData.meta.instanceId
: '',
isCloudDeployment: settingsStore.isCloudDeployment,
},
).nodeGraph,
);
if (source === 'paste') {
telemetry.track('User pasted nodes', {
workflow_id: workflowsStore.workflowId,
node_graph_string: nodeGraph,
});
} else if (source === 'duplicate') {
telemetry.track('User duplicated nodes', {
workflow_id: workflowsStore.workflowId,
node_graph_string: nodeGraph,
});
} else {
telemetry.track('User imported workflow', {
source,
workflow_id: workflowsStore.workflowId,
node_graph_string: nodeGraph,
});
if (source === 'paste') {
telemetry.track('User pasted nodes', {
workflow_id: workflowsStore.workflowId,
node_graph_string: nodeGraph,
});
} else if (source === 'duplicate') {
telemetry.track('User duplicated nodes', {
workflow_id: workflowsStore.workflowId,
node_graph_string: nodeGraph,
});
} else {
telemetry.track('User imported workflow', {
source,
workflow_id: workflowsStore.workflowId,
node_graph_string: nodeGraph,
});
}
}
} catch {
// If telemetry fails, don't throw an error

View File

@@ -183,7 +183,7 @@ export type CanvasEventBusEvents = {
action: keyof CanvasNodeEventBusEvents;
payload?: CanvasNodeEventBusEvents[keyof CanvasNodeEventBusEvents];
};
tidyUp: { source: CanvasLayoutSource; nodeIdsFilter?: string[] };
tidyUp: { source: CanvasLayoutSource; nodeIdsFilter?: string[]; trackEvents?: boolean };
};
export interface CanvasNodeInjectionData {

View File

@@ -710,8 +710,8 @@ const allTriggerNodesDisabled = computed(() => {
return disabledTriggerNodes.length === triggerNodes.value.length;
});
function onTidyUp(event: CanvasLayoutEvent) {
tidyUp(event);
function onTidyUp(event: CanvasLayoutEvent, options?: { trackEvents?: boolean }) {
tidyUp(event, options);
}
function onExtractWorkflow(nodeIds: string[]) {
@@ -1114,9 +1114,11 @@ async function importWorkflowExact({ workflow: workflowData }: { workflow: Workf
async function onImportWorkflowDataEvent(data: IDataObject) {
const workflowData = data.data as WorkflowDataUpdate;
const trackEvents = typeof data.trackEvents === 'boolean' ? data.trackEvents : undefined;
await importWorkflowData(workflowData, 'file', {
viewport: viewportBoundaries.value,
regenerateIds: data.regenerateIds === true || data.regenerateIds === undefined,
trackEvents,
});
fitView();
@@ -1127,6 +1129,7 @@ async function onImportWorkflowDataEvent(data: IDataObject) {
canvasEventBus.emit('tidyUp', {
source: 'import-workflow-data',
nodeIdsFilter: nodesIdsToTidyUp,
trackEvents,
});
}, 0);
}