feat: Telemetry include basic llm optional promps, trigger on save workflow event #8981
Conversation
| const isCloudDeployment = config.getEnv('deployment.type') === 'cloud'; | ||
|
|
||
| const { nodeGraph } = TelemetryHelpers.generateNodesGraph(workflow, this.nodeTypes, { | ||
| isCloudDeployment, | ||
| }); |
There was a problem hiding this comment.
Why are we filtering for cloud? The story doesn't mention this?
There was a problem hiding this comment.
this is followup to this issue, should be enabled on cloud only
There was a problem hiding this comment.
As discussed, let's use config inside the function itself.
| nodeItem.prompts = | ||
| (((node.parameters?.messages as IDataObject) || {}).messageValues as IDataObject[]) || []; | ||
| } |
There was a problem hiding this comment.
Can we please find a way to avoid so many assertions? And use nullish coalescing instead of ||? Also is there really a case where prompts will be an empty array?
There was a problem hiding this comment.
we had to use those assertions as parameter would not be included if value is default, yes by default in basic llm it is an empty array
packages/cli/src/InternalHooks.ts
Outdated
| @@ -488,7 +492,7 @@ export class InternalHooks { | |||
| ? this.eventBus.sendWorkflowEvent({ | |||
| eventName: 'n8n.workflow.success', | |||
| payload: sharedEventPayload, | |||
| }) | |||
| }) | |||
There was a problem hiding this comment.
Some formatting issues to autofix.
There was a problem hiding this comment.
prettier got got confused here, I did refactor code slightly
…lemetry-prompts-missing-in-some-events
2 flaky tests on run #4481 ↗︎
Details:
|
|||||||||||||||||||||||||||
| Test | Artifacts | |
|---|---|---|
| Sharing > should work for admin role on credentials created by others (also can share it with themselves) |
Test Replay
Screenshots
Video
|
|
5-ndv.cy.ts • 1 flaky test
| Test | Artifacts | |
|---|---|---|
| NDV > should not retrieve remote options when required params throw errors |
Test Replay
Screenshots
Video
|
|
Review all test suite changes for PR #8981 ↗︎
|
✅ All Cypress E2E specs passed |
|
Got released with |
Summary
Telemetry prompts missing in some events
Related tickets and issues
https://linear.app/n8n/issue/AI-161/telemetry-prompts-missing-in-some-events