feat(core): add id, start and end time to lifecycle hooks #32583
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meeroslav
changed the title
|
View your CI Pipeline Execution ↗ for commit e409e9e
☁️ Nx Cloud last updated this comment at |
| @@ -1,5 +1,5 @@ | |||
| import type { ProjectGraph } from '../../config/project-graph'; | |||
| import { readNxJson, type PluginConfiguration } from '../../config/nx-json'; | |||
| groupId: number; | ||
| } | ||
|
|
||
| interface RustRunningTask extends RunningTask { |
FrozenPandaz
left a comment
FrozenPandaz
left a comment
There was a problem hiding this comment.
This seems very easy to get if users run Date.now() in their pre and post hooks. I don't think we need to be the one providing this.
@FrozenPandaz, it might not be trivial when you run several commands in parallel. Providing this information out of the box instead of forcing the user to track the collection of start times in memory per args combo is better, in my opinion. Also, it prevents other (potentially long-running) Pre and Post hooks from skewing the numbers. |
| ].join('\n')}\n` | ||
| ); | ||
| } | ||
| const taskId = hashArray([...process.argv, Date.now().toString()]); |
There was a problem hiding this comment.
Please call this a tasksExecutionId or executionId... or even just id. Just definitely not task.id
| }; | ||
|
|
||
| export type PreTasksExecutionContext = { | ||
| readonly taskId: string; |
There was a problem hiding this comment.
We can make this just id
There was a problem hiding this comment.
I changed it to just id.
| import { shouldStreamOutput } from './utils'; | ||
| import { signalToCode } from '../utils/exit-codes'; | ||
| import chalk = require('chalk'); | ||
| import { randomUUID } from 'node:crypto'; |
There was a problem hiding this comment.
Doesn't look to be used.
There was a problem hiding this comment.
Oops, AI autocomplete leftover. Let me kick that out.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
meeroslav
changed the title
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
Pre and post hooks lack corellation Id which might make it difficult to culculate stats for task using pre and post hooks.
Additionally, post hook does not expose the duration or time span for the task. Using pre/post hook might not be precise enough.
Expected Behavior
Hooks expose the unique taskId so we can corellate pre hook of a task to the post hook of the same task. Post hook should expose start and end time of the task run.
Related Issue(s)
Resolves discussion 31076