Address reviews

Author: rendre-greyling

controllers/main.go

@@ -214,6 +214,8 @@ func newManager(scheme *runtime.Scheme) (ctrl.Manager, error) {
 
 	mgr, err := ctrl.NewManager(cfg, ctrl.Options{
 		Scheme: scheme,
+		// Disable controller-runtime's default :8080 /metrics listener. Port 8080 is reserved for
+		// optional pprof (PORCH_PPROF_PORT); controller-runtime metrics are exposed on :9464 via OpenTelemetry.
 		Metrics: metricsserver.Options{
 			BindAddress: "0",
 		},

deployments/metrics/pyroscope-deployment.yaml

@@ -1,4 +1,4 @@
-# Copyright 2026 The Nephio Authors
+# Copyright 2026 The kpt Authors
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.

deployments/porch/2-function-runner.yaml

@@ -62,8 +62,6 @@ spec:
               value: none
             - name: OTEL_SERVICE_NAME
               value: porch-function-runner
-            - name: PORCH_PPROF_PORT
-              value: "8080"
           ports:
             - containerPort: 9445
             - containerPort: 9464

deployments/porch/3-porch-server.yaml

@@ -54,7 +54,7 @@ spec:
               memory: 256Mi
               cpu: 250m
             limits:
-              memory: 2Gi
+              memory: 512Mi
           volumeMounts:
             - mountPath: /cache
               name: cache-volume
@@ -82,8 +82,6 @@ spec:
               value: "9464" # Default value, showing for visibility
             - name: OTEL_TRACES_EXPORTER
               value: none
-            - name: PORCH_PPROF_PORT
-              value: "8080"
           args:
             - --function-runner=function-runner:9445
             - --cache-directory=/cache

deployments/porch/9-controllers.yaml

@@ -72,8 +72,6 @@ spec:
           value: "9464" # Default value, showing for visibility
         - name: OTEL_TRACES_EXPORTER
           value: none
-        - name: PORCH_PPROF_PORT
-          value: "8080"
         envFrom:
         - configMapRef:
             name: porch-db-config

deployments/porch/README.md

@@ -39,6 +39,7 @@ make deployment-config
 - `IMAGE_REPO`: Set image repository (default: `ghcr.io/kptdev`)
 - `ENABLED_RECONCILERS`: Comma-separated list of reconcilers (default: `packagevariants,packagevariantsets,repositories`)
 - `FN_RUNNER_WARM_UP_POD_CACHE`: Enable/disable pod cache warm-up (default: `true`)
+- `PORCH_GHCR_PREFIX_URL`: KRM function catalog registry prefix (from `.env` or environment). Applied to porch-server (`--default-image-prefix`), function-runner (`--default-image-prefix`), and porch-controllers (`DEFAULT_IMAGE_PREFIX`) for all `make run-in-kind*` targets that use `deployment-config`.
 
 Examples:
 

func/server/server.go

@@ -245,6 +245,8 @@ func buildFnConfigReconciler(o *options, scheme *runtime.Scheme) (*reconciler.Fu
 	mgr, err := ctrl.NewManager(restCfg, ctrl.Options{
 		Scheme: scheme,
 		Cache:  cacheOpts,
+		// Disable controller-runtime's default :8080 /metrics listener. Port 8080 is reserved for
+		// optional pprof (PORCH_PPROF_PORT); controller-runtime metrics are exposed on :9464 via OpenTelemetry.
 		Metrics: metricsserver.Options{
 			BindAddress: "0",
 		},

internal/telemetry/metrics.go

@@ -16,9 +16,10 @@ package telemetry
 
 import (
 	"context"
-	"fmt"
 	"time"
 
+	"github.com/kptdev/porch/api/porch/v1alpha1"
+	"github.com/kptdev/porch/api/porch/v1alpha2"
 	"github.com/kptdev/porch/pkg/repository"
 	"go.opentelemetry.io/otel"
 	"go.opentelemetry.io/otel/attribute"
@@ -30,30 +31,21 @@ import (
 const meterName = "github.com/kptdev/porch"
 
 const (
-	APIVersionV1Alpha1 = "v1alpha1"
-	APIVersionV1Alpha2 = "v1alpha2"
-	ControllerUser     = "packagerevision-controller"
+	ControllerUser = "packagerevision-controller"
 
 	ResourcePackageRevision          = "PackageRevision"
 	ResourcePackageRevisionResources = "PackageRevisionResources"
 	ResourceExternalRepo             = "ExternalRepo"
 )
 
 var (
+	APIVersionV1Alpha1 = v1alpha1.SchemeGroupVersion.Version
+	APIVersionV1Alpha2 = v1alpha2.SchemeGroupVersion.Version
+
 	apiCallDurationSeconds  metric.Float64Histogram
-	RequestsTotal           metric.Float64Counter
+	requestsTotal           metric.Float64Counter
 	prResourceSizeHistogram metric.Int64Histogram
 	prResourceSizeGauge     metric.Int64Gauge
-
-	// Performance tests related vars
-	perfOperationDuration           metric.Float64Histogram
-	perfOperationCounter            metric.Float64Counter
-	perfRepositoryCounter           metric.Float64Counter
-	perfPackageCounter              metric.Float64Counter
-	perfPackageRevisionCounter      metric.Float64Counter
-	perfLifecycleTransitionDuration metric.Float64Histogram
-	perfTestRunInfoGauge            metric.Float64Gauge
-	perfActiveOperations            metric.Float64UpDownCounter
 )
 
 func InitMetrics() (err error) {
@@ -69,15 +61,17 @@ func InitMetrics() (err error) {
 		),
 	)
 	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_api_call_duration_seconds: %v", err))
+		klog.Errorf("failed to create porch_api_call_duration_seconds: %v", err)
+		return
 	}
 
-	RequestsTotal, err = m.Float64Counter(
+	requestsTotal, err = m.Float64Counter(
 		"porch_api_requests_by_user",
 		metric.WithDescription("Total number of requests tracked by BurstCounter, broken down by resource, operation, and user."),
 	)
 	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_api_requests_by_user: %v", err))
+		klog.Errorf("failed to create porch_api_requests_by_user: %v", err)
+		return
 	}
 
 	prResourceSizeHistogram, err = m.Int64Histogram(
@@ -101,81 +95,13 @@ func InitMetrics() (err error) {
 		return
 	}
 
-	// Performance test related metrics
-	perfOperationDuration, err = m.Float64Histogram(
-		"porch_perf_operation_duration_seconds",
-		metric.WithDescription("Duration of Porch performance test operations in seconds"),
-		metric.WithUnit("s"),
-		metric.WithExplicitBucketBoundaries(0.01, 0.05, 0.1, 0.5, 1, 2, 5, 10, 30, 60, 120),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_operation_duration_seconds: %v", err))
-	}
-
-	perfOperationCounter, err = m.Float64Counter(
-		"porch_perf_operations_total",
-		metric.WithDescription("Total number of Porch performance test operations"),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_operations_total: %v", err))
-	}
-
-	perfRepositoryCounter, err = m.Float64Counter(
-		"porch_perf_repositories_created_total",
-		metric.WithDescription("Total number of repositories created in performance tests"),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_repositories_created_total: %v", err))
-	}
-
-	perfPackageCounter, err = m.Float64Counter(
-		"porch_perf_packages_created_total",
-		metric.WithDescription("Total number of packages created in performance tests"),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_packages_created_total: %v", err))
-	}
-
-	perfPackageRevisionCounter, err = m.Float64Counter(
-		"porch_perf_package_revisions_total",
-		metric.WithDescription("Total number of package revisions created in performance tests"),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_package_revisions_total: %v", err))
-	}
-
-	perfLifecycleTransitionDuration, err = m.Float64Histogram(
-		"porch_perf_lifecycle_transition_duration_seconds",
-		metric.WithDescription("Duration of package lifecycle transitions in seconds"),
-		metric.WithUnit("s"),
-		metric.WithExplicitBucketBoundaries(0.1, 0.5, 1, 2, 5, 10, 30, 60),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_lifecycle_transition_duration_seconds: %v", err))
-	}
-
-	perfTestRunInfoGauge, err = m.Float64Gauge(
-		"porch_perf_test_run_info",
-		metric.WithDescription("Information about the current performance test run"),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_test_run_info: %v", err))
-	}
-
-	perfActiveOperations, err = m.Float64UpDownCounter(
-		"porch_perf_active_operations",
-		metric.WithDescription("Number of currently active operations"),
-	)
-	if err != nil {
-		panic(fmt.Sprintf("failed to create porch_perf_active_operations: %v", err))
-	}
-
 	return nil
 }
 
 // Porch server and function runner metric recording functions
 func RecordAPICallDuration(resource, verb, apiVersion string, durationSeconds float64) {
 	if apiCallDurationSeconds == nil {
+		klog.Warning("apiCallDurationSeconds is nil - was InitMetrics() called?")
 		return
 	}
 	apiCallDurationSeconds.Record(context.Background(), durationSeconds,
@@ -188,14 +114,16 @@ func RecordAPICallDuration(resource, verb, apiVersion string, durationSeconds fl
 }
 
 func RecordRequestCount(ctx context.Context, resource, op, apiVersion string) {
-	if RequestsTotal == nil {
+	if requestsTotal == nil {
+		klog.Warning("requestsTotal is nil - was InitMetrics() called?")
 		return
 	}
 	recordRequestCount(resource, op, apiVersion, getK8sUserName(ctx))
 }
 
 func RecordControllerRequestCount(resource, op, apiVersion string) {
-	if RequestsTotal == nil {
+	if requestsTotal == nil {
+		klog.Warning("requestsTotal is nil - was InitMetrics() called?")
 		return
 	}
 	recordRequestCount(resource, op, apiVersion, ControllerUser)
@@ -208,7 +136,7 @@ func RecordControllerOperation(resource, verb string, start time.Time) {
 }
 
 func recordRequestCount(resource, op, apiVersion, user string) {
-	RequestsTotal.Add(context.Background(), 1,
+	requestsTotal.Add(context.Background(), 1,
 		metric.WithAttributes(
 			attribute.String("resource", resource),
 			attribute.String("op", op),
@@ -226,6 +154,7 @@ func RecordExternalRepoOperation(ctx context.Context, op string, start time.Time
 
 func recordExternalRepoDuration(op string, durationSeconds float64) {
 	if apiCallDurationSeconds == nil {
+		klog.Warning("apiCallDurationSeconds is nil - was InitMetrics() called?")
 		return
 	}
 	apiCallDurationSeconds.Record(context.Background(), durationSeconds,
@@ -237,10 +166,11 @@ func recordExternalRepoDuration(op string, durationSeconds float64) {
 }
 
 func RecordExternalRepoRequestCount(ctx context.Context, op string) {
-	if RequestsTotal == nil {
+	if requestsTotal == nil {
+		klog.Warning("requestsTotal is nil - was InitMetrics() called?")
 		return
 	}
-	RequestsTotal.Add(context.Background(), 1,
+	requestsTotal.Add(context.Background(), 1,
 		metric.WithAttributes(
 			attribute.String("resource", ResourceExternalRepo),
 			attribute.String("op", op),
@@ -283,77 +213,6 @@ func RecordPackageRevisionResourcesSize(ctx context.Context, prKey repository.Pa
 	prResourceSizeGauge.Record(ctx, resourcesSize, metric.WithAttributeSet(attributes))
 }
 
-// Performance test metric recording functions
-func PerfTestRecordMetric(operation, apiVersion, repoName, pkgName string, duration time.Duration, err error) {
-	attrs := metric.WithAttributes(
-		attribute.String("operation", operation),
-		attribute.String("api_version", apiVersion),
-		attribute.String("repository", repoName),
-		attribute.String("package", pkgName),
-		attribute.String("status", statusLabel(err)),
-	)
-	ctx := context.Background()
-	perfOperationDuration.Record(ctx, duration.Seconds(), attrs)
-	perfOperationCounter.Add(ctx, 1, attrs)
-}
-
-func PerfTestRecordLifecycleTransition(fromState, toState, apiVersion, repoName, pkgName string, duration time.Duration, err error) {
-	perfLifecycleTransitionDuration.Record(context.Background(), duration.Seconds(),
-		metric.WithAttributes(
-			attribute.String("from_state", fromState),
-			attribute.String("to_state", toState),
-			attribute.String("api_version", apiVersion),
-			attribute.String("repository", repoName),
-			attribute.String("package", pkgName),
-			attribute.String("status", statusLabel(err)),
-		),
-	)
-}
-
-func PerfTestRecordPackageRevision(operation string, err error) {
-	perfPackageRevisionCounter.Add(context.Background(), 1,
-		metric.WithAttributes(
-			attribute.String("operation", operation),
-			attribute.String("status", statusLabel(err)),
-		),
-	)
-}
-
-func PerfTestSetTestRunInfo(testName, namespace, apiVersion string, startTime time.Time) {
-	perfTestRunInfoGauge.Record(context.Background(), 1,
-		metric.WithAttributes(
-			attribute.String("test_name", testName),
-			attribute.String("namespace", namespace),
-			attribute.String("api_version", apiVersion),
-			attribute.String("start_time", startTime.Format(time.RFC3339)),
-		),
-	)
-}
-
-func PerfTestRecordActiveOperation(operation, apiVersion string, delta float64) {
-	perfActiveOperations.Add(context.Background(), delta,
-		metric.WithAttributes(
-			attribute.String("operation", operation),
-			attribute.String("api_version", apiVersion),
-		),
-	)
-}
-
-func PerfTestIncrementRepositoryCounter() {
-	perfRepositoryCounter.Add(context.Background(), 1)
-}
-
-func PerfTestIncrementPackageCounter() {
-	perfPackageCounter.Add(context.Background(), 1)
-}
-
-func statusLabel(err error) string {
-	if err != nil {
-		return "error"
-	}
-	return "success"
-}
-
 func getK8sUserName(ctx context.Context) string {
 	if user, ok := request.UserFrom(ctx); ok {
 		return user.GetName()