Add in-flight and richer duration Prometheus metrics on all Porch API operations - #1198
Open
JamesMcDermott wants to merge 3 commits into
Open
JamesMcDermott wants to merge 3 commits into
JamesMcDermott wants to merge 3 commits into
Conversation
JamesMcDermott
requested review from
efiacor,
kispaljr,
liamfallon and
mozesl-nokia
as code owners
September 21, 2026 16:45
✅ Deploy Preview for kpt-porch ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate findings affect metric correctness, test reliability, and existing performance dashboards.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 3
Open (17)
What changed in this PR
Adds richer Porch API duration metrics and in-flight operation tracking across REST and controller paths, with expanded E2E coverage and monitoring fixes.
Changes:
- Adds operation, outcome, lifecycle, and package-revision labels.
- Instruments REST and controller operations with in-flight gauges.
- Updates telemetry helpers, tests, and monitoring paths.
| File | Description |
|---|---|
test/performance/metrics_utils.go |
Routes performance durations through telemetry. |
test/e2e/suiteutils/suite.go |
Adds logging-oriented client helpers. |
test/e2e/suiteutils/suite_utils.go |
Refactors metrics collection and parsing. |
test/e2e/crd/suite_test.go |
Initializes the Kubernetes client. |
test/e2e/crd/metrics_test.go |
Adds controller metrics tests. |
test/e2e/crd/helpers_test.go |
Adds deletion polling support. |
test/e2e/api/metrics_test.go |
Adds API metrics and in-flight tests. |
scripts/monitoring/deploy-monitoring.sh |
Fixes monitoring resource paths. |
pkg/registry/porch/packagerevisionresources.go |
Instruments resource operations. |
pkg/registry/porch/packagerevisionresources_update_test.go |
Updates resource-operation mocks. |
pkg/registry/porch/packagerevisionresources_test.go |
Updates lifecycle mocks. |
pkg/registry/porch/packagerevision.go |
Instruments PackageRevision operations. |
pkg/registry/porch/packagerevision_test.go |
Updates revision-operation mocks. |
pkg/registry/porch/packagerevision_approval.go |
Instruments approval operations. |
pkg/registry/porch/packagerevision_approval_test.go |
Updates approval-operation mocks. |
pkg/registry/porch/packagecommon.go |
Returns typed updated revisions. |
internal/telemetry/metrics.go |
Defines enriched metrics. |
internal/telemetry/metrics_test.go |
Updates telemetry tests. |
controllers/packagerevisions/pkg/controllers/packagerevision/source.go |
Refactors source action selection. |
controllers/packagerevisions/pkg/controllers/packagerevision/source_test.go |
Tests source action selection. |
controllers/packagerevisions/pkg/controllers/packagerevision/render.go |
Instruments render operations. |
controllers/packagerevisions/pkg/controllers/packagerevision/packagerevision_controller.go |
Instruments reconciliation. |
controllers/packagerevisions/pkg/controllers/packagerevision/ownership.go |
Instruments deletion cleanup. |
controllers/packagerevisions/pkg/controllers/packagerevision/metadata.go |
Instruments metadata updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+70
to
+73
| defer telemetry.TrackInFlightOperation(ctx, prTelemetryName, op.AllCaps, op.TitleCase+prTelemetryName, telemetry.APIVersionV1Alpha1, lifecycle, &key)() | ||
| defer func() { | ||
| span.End() | ||
| telemetry.RecordAPICallDuration(praTelemetryName, "GET", telemetry.APIVersionV1Alpha1, time.Since(start).Seconds()) | ||
| telemetry.RecordAPIOperationDuration(ctx, praTelemetryName, op.AllCaps, op.TitleCase+praTelemetryName, telemetry.APIVersionV1Alpha1, time.Since(start), err, lifecycle, &key) |
| if err == nil { | ||
| return "" | ||
| } | ||
| return err.(*errors.StatusError).ErrStatus.Reason |
Comment on lines
+1127
to
1129
| if porchFunctionRunnerPod == nil { | ||
| return nil, fmt.Errorf("failed to find function-runner pod") | ||
| } |
| op := telemetry.Operations.Update | ||
| start := time.Now() | ||
| key, _ := repository.PkgRevK8sName2Key(pr.Namespace, pr.Name) | ||
| defer telemetry.TrackInFlightControllerOperation(ctx, prTelemetryName, op.AllCaps, op.TitleCase+prrTelemetryName, pr.Spec.Lifecycle, &key)() |
Comment on lines
+86
to
90
| err = r.ContentCache.DeletePackage(ctx, repoKey, pr.Spec.PackageName, pr.Spec.WorkspaceName) | ||
| if repository.IsNotFoundError(err) { | ||
| log.FromContext(ctx).Info("package not found in git, nothing to clean up") | ||
| return nil | ||
| } |
Comment on lines
+188
to
+189
| samples := collectControllerMetrics(env.Ctx, inFlightMetric) | ||
|
|
|
|
||
| func (t *TestSuite) CreateL(obj client.Object, opts ...client.CreateOption) { | ||
| t.T().Helper() | ||
| t.create(obj, opts, t.Errorf) |
| key := repository.PackageRevisionKey{ | ||
| PkgKey: repository.PackageKey{RepoKey: repository.RepositoryKey{Name: repoName}, Package: pkgName}, | ||
| } | ||
| telemetry.RecordAPIOperationDuration(ctx, "PackageRevision", operation, telemetry.ParseOperation(operation).TitleCase, apiVersion, duration, err, v1alpha1.PackageRevisionLifecycleDraft, &key) |
Comment on lines
+179
to
+181
| newApiPkgRev, ok := runtimeObject.(*porchapi.PackageRevision) | ||
| if !ok { | ||
| return nil, apierrors.NewBadRequest(fmt.Sprintf("expected PackageRevision object, got %T", runtimeObject)) |
| } | ||
|
|
||
| key, _ := repository.PkgRevK8sName2Key(namespace, rawName) | ||
| defer telemetry.TrackInFlightOperation(ctx, prrTelemetryName, op.AllCaps, op.TitleCase+prrTelemetryName, telemetry.APIVersionV1Alpha1, lifecycle, &key)() |
- enhance existing API-call-duration histogram to include more information from Porch perspective - making it a Porch-API-operation-duration metric - add new metric to track in-flight (currently-in-progress) Porch operations Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
- wire porch_in_flight_api_operations gauge into v1alpha2 controller
reconciler operations
- track lifecycle, source, metadata-sync, render, and delete
reconcile paths
- Published lifecycle transitions recorded under PackageRevisionApproval.
- refactor applySource into selectPackageSourceAction + execution so the
operation type (init/clone/copy/upgrade) is known for telemetry; update
source_test.go accordingly.
- new E2E tests TestAPIOperationDurationMetricExists, TestAPIOperationDurationLabels,
and TestInFlightOperationsMetric in v1alpha1 suite
- counterparts in v1alpha2 suite testing against PorchControllerMetrics.
- small fix in deploy-monitoring.sh config paths
Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
in case of errors - fix unit tests to account for extra calls to GetPackageRevision(), Lifecycle(), etc. Signed-off-by: James McDermott <james.j.mcdermott@ericsson.com>
JamesMcDermott
force-pushed
the
feature-porch-operation-metrics
branch
from
September 21, 2026 16:53
31ccc38 to
d4d35c0
Compare
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.
Description
porch_api_call_duration_seconds) into a Porch-API-operation-duration metricoperationoperation_outcomelifecycle_afternamespacerepositorypackageworkspace_nameporch_in_flight_api_operationsgauge (anInt64UpDownCounter) to trackcurrently-in-progress Porch operations
scripts/monitoring/deploy-monitoring.shmissed in script directory refactoringper-operation/outcome/lifecycle
operations
RecordAPIOperationDuration/RecordControllerOperationTrackInFlightOperation/TrackInFlightControllerOperation, which increments the in-flight gauge at start and decrements it via a deferred closure on completion (including error paths).Type of Change
Checklist
Testing Instructions (Optional)
make test1.
make setup-dev-env run-in-kind-db-cache test-e2e1.
make setup-dev-env run-in-kind-v1alpha2 test-e2e-crd/metricsendpoint and confirm the presence ofporch_api_call_duration_seconds_{bucket,count,sum}with the new labels andporch_in_flight_api_operationssettling to 0 once operations complete.Additional Notes (Optional)
porch_in_flight_api_operationsin-flight proof (the spammer/scraper approach) is only exercised in the v1alpha1 (API) suite; the v1alpha2 (CRD) suite asserts steady-state and per-operation presence but not mid-flight increment, because controller operations are driven by asynchronous reconciliation rather than synchronous, re-runnable REST calls.source_test.goasserts the selected source action viafmt.Sprintf("%#v", ...)function-valuecomparison; this is intentional but relies on stable function formatting.
AI Disclosure
If so, please describe how: