BCH-1153: Populate per-step metrics in workflow execution metrics resp#399
Open
saltpy-cs wants to merge 2 commits into
Open
BCH-1153: Populate per-step metrics in workflow execution metrics resp#399saltpy-cs wants to merge 2 commits into
saltpy-cs wants to merge 2 commits into
Conversation
…ponse The metrics endpoint only returned aggregate durations. The UI Step Performance table was always hidden because stepMetrics was never populated. Now each step execution contributes a StepMetric entry with its name, timestamps, and duration in minutes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the workflow execution metrics endpoint to include per-step timing details so the UI can populate the Step Performance table instead of only showing aggregate durations.
Changes:
- Build and return per-step metrics (
StepMetric) from step executions inGetExecutionMetrics. - Add a unit test asserting
StepMetricsis populated. - Update Swagger artifacts to document the new per-step metrics array.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/workflow/manager.go | Adds StepMetric/StepMetrics and populates per-step timing data in GetExecutionMetrics. |
| internal/workflow/manager_test.go | Adds a unit test validating per-step metrics are returned. |
| docs/swagger.yaml | Documents the new step metrics field and schema in Swagger YAML. |
| docs/swagger.json | Documents the new step metrics field and schema in Swagger JSON. |
| docs/docs.go | Updates the embedded Swagger template with the new step metrics schema. |
Comments suppressed due to low confidence (2)
internal/workflow/manager.go:382
- StepMetric JSON field names use snake_case (e.g., started_at, step_definition_id), while the surrounding metrics schemas use lowerCamelCase (e.g., averageStepDuration, executionID). This introduces an inconsistent API surface and likely won’t match the UI expectation described in the PR (stepMetrics). Consider switching these tags to match the existing metrics naming (e.g., startedAt/completedAt, stepDefinitionID, durationMinutes).
// StepMetric holds per-step timing data for the metrics response
type StepMetric struct {
StepDefinitionID uuid.UUID `json:"step_definition_id"`
StepName string `json:"step_name"`
StartedAt *time.Time `json:"started_at,omitempty"`
CompletedAt *time.Time `json:"completed_at,omitempty"`
DurationMinutes *float64 `json:"duration_minutes,omitempty"`
}
internal/workflow/manager.go:392
- ExecutionMetrics now mixes untagged fields (which will marshal as "ExecutionID", "TotalSteps", etc.) with a tagged field ("step_metrics"). This leads to inconsistent JSON keys within the same response. Consider adding consistent json tags for all ExecutionMetrics fields and naming the new field consistently with the existing metrics properties (likely stepMetrics rather than step_metrics).
// ExecutionMetrics represents metrics for a workflow execution
type ExecutionMetrics struct {
ExecutionID uuid.UUID
TotalSteps int
Duration time.Duration
AverageStepDuration time.Duration
LongestStepDuration time.Duration
StepMetrics []StepMetric `json:"step_metrics"`
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Switch StepMetric json tags from snake_case to lowerCamelCase to match the surrounding ExecutionMetrics naming convention - Add explicit lowerCamelCase json tags to all ExecutionMetrics fields for consistency; rename step_metrics to stepMetrics - Initialize StepMetrics as empty slice so zero-step executions serialize as [] rather than null - Regenerate swagger docs to reflect updated field names Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

The metrics endpoint only returned aggregate durations. The UI Step Performance table was always hidden because stepMetrics was never populated. Now each step execution contributes a StepMetric entry with its name, timestamps, and duration in minutes.