fix: lose cold start error counter for the poolmgr functions (#2755)

* fix: lose cold start error counter for the poolmgr functions

Co-authored-by: ZhengHe-MD <ranchardzheng@gmail.com>
Co-authored-by: gw123 <iamakillerforyou@gmail.com>
Signed-off-by: saltbo <saltbo@foxmail.com>

* fix: miss code for the metric

Signed-off-by: saltbo <saltbo@foxmail.com>

---------

Signed-off-by: saltbo <saltbo@foxmail.com>
Co-authored-by: ZhengHe-MD <ranchardzheng@gmail.com>
Co-authored-by: gw123 <iamakillerforyou@gmail.com>

pkg/executor/executortype/container/containermgr.go

@@ -457,7 +457,7 @@ func (caaf *Container) fnCreate(ctx context.Context, fn *fv1.Function) (*fscache
 	_, err = caaf.fsCache.Add(*fsvc)
 	if err != nil {
 		caaf.logger.Error("error adding function to cache", zap.Error(err), zap.Any("function", fsvc.Function))
-		metrics.FuncError.WithLabelValues(fn.ObjectMeta.Name, fn.ObjectMeta.Namespace).Inc()
+		metrics.ColdStartsError.WithLabelValues(fn.ObjectMeta.Name, fn.ObjectMeta.Namespace).Inc()
 		return fsvc, err
 	}
 

pkg/executor/executortype/newdeploy/newdeploymgr.go

@@ -500,7 +500,7 @@ func (deploy *NewDeploy) fnCreate(ctx context.Context, fn *fv1.Function) (*fscac
 	_, err = deploy.fsCache.Add(*fsvc)
 	if err != nil {
 		deploy.logger.Error("error adding function to cache", zap.Error(err), zap.Any("function", fsvc.Function))
-		metrics.FuncError.WithLabelValues(fn.ObjectMeta.Name, fn.ObjectMeta.Namespace).Inc()
+		metrics.ColdStartsError.WithLabelValues(fn.ObjectMeta.Name, fn.ObjectMeta.Namespace).Inc()
 		return fsvc, err
 	}
 

pkg/executor/executortype/poolmgr/gp.go

@@ -47,7 +47,6 @@ import (
 	fv1 "github.com/fission/fission/pkg/apis/core/v1"
 	"github.com/fission/fission/pkg/crd"
 	"github.com/fission/fission/pkg/executor/fscache"
-	"github.com/fission/fission/pkg/executor/metrics"
 	fetcherClient "github.com/fission/fission/pkg/fetcher/client"
 	fetcherConfig "github.com/fission/fission/pkg/fetcher/config"
 	"github.com/fission/fission/pkg/generated/clientset/versioned"
@@ -517,7 +516,7 @@ func (gp *GenericPool) getFuncSvc(ctx context.Context, fn *fv1.Function) (*fscac
 		// Remove old versions function pods
 		for _, pod := range podList.Items {
 			// Delete pod no matter what status it is
-			gp.kubernetesClient.CoreV1().Pods(gp.fnNamespace).Delete(ctx, pod.ObjectMeta.Name, metav1.DeleteOptions{}) //nolint errcheck
+			gp.kubernetesClient.CoreV1().Pods(gp.fnNamespace).Delete(ctx, pod.ObjectMeta.Name, metav1.DeleteOptions{}) // nolint errcheck
 		}
 	}
 
@@ -613,7 +612,6 @@ func (gp *GenericPool) getFuncSvc(ctx context.Context, fn *fv1.Function) (*fscac
 	gp.fsCache.PodToFsvc.Store(pod.GetObjectMeta().GetName(), fsvc)
 	gp.podFSVCMap.Store(pod.ObjectMeta.Name, []interface{}{crd.CacheKey(fsvc.Function), fsvc.Address})
 	gp.fsCache.AddFunc(ctx, *fsvc, fn.GetRequestPerPod())
-	metrics.ColdStarts.WithLabelValues(fn.ObjectMeta.Name, fn.ObjectMeta.Namespace).Inc()
 
 	logger.Info("added function service",
 		zap.String("pod", pod.ObjectMeta.Name),

pkg/executor/executortype/poolmgr/gpm.go

@@ -26,6 +26,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/fission/fission/pkg/executor/metrics"
 	"github.com/hashicorp/go-multierror"
 	"go.opentelemetry.io/otel/attribute"
 	"go.uber.org/zap"
@@ -186,7 +187,16 @@ func (gpm *GenericPoolManager) GetTypeName(ctx context.Context) fv1.ExecutorType
 	return fv1.ExecutorTypePoolmgr
 }
 
-func (gpm *GenericPoolManager) GetFuncSvc(ctx context.Context, fn *fv1.Function) (*fscache.FuncSvc, error) {
+func (gpm *GenericPoolManager) GetFuncSvc(ctx context.Context, fn *fv1.Function) (fnSvc *fscache.FuncSvc, fErr error) {
+	defer func() {
+		if fErr != nil {
+			metrics.ColdStartsError.WithLabelValues(fn.ObjectMeta.Name, fn.ObjectMeta.Namespace).Inc()
+			return
+		}
+
+		metrics.ColdStarts.WithLabelValues(fn.ObjectMeta.Name, fn.ObjectMeta.Namespace).Inc()
+	}()
+
 	otelUtils.SpanTrackEvent(ctx, "GetFuncSvc", otelUtils.GetAttributesForFunction(fn)...)
 	logger := otelUtils.LoggerWithTraceID(ctx, gpm.logger)
 
@@ -194,12 +204,14 @@ func (gpm *GenericPoolManager) GetFuncSvc(ctx context.Context, fn *fv1.Function)
 	logger.Debug("getting environment for function", zap.String("function", fn.ObjectMeta.Name))
 	env, err := gpm.getFunctionEnv(ctx, fn)
 	if err != nil {
-		return nil, err
+		fErr = err
+		return
 	}
 
 	pool, created, err := gpm.getPool(ctx, env)
 	if err != nil {
-		return nil, err
+		fErr = err
+		return
 	}
 
 	if created {
@@ -209,7 +221,8 @@ func (gpm *GenericPoolManager) GetFuncSvc(ctx context.Context, fn *fv1.Function)
 	// from GenericPool -> get one function container
 	// (this also adds to the cache)
 	logger.Debug("getting function service from pool", zap.String("function", fn.ObjectMeta.Name))
-	return pool.getFuncSvc(ctx, fn)
+	fnSvc, fErr = pool.getFuncSvc(ctx, fn)
+	return fnSvc, fErr
 }
 
 func (gpm *GenericPoolManager) GetFuncSvcFromCache(ctx context.Context, fn *fv1.Function) (*fscache.FuncSvc, error) {

pkg/executor/metrics/metrics.go

@@ -41,7 +41,7 @@ var (
 		},
 		functionLabels,
 	)
-	FuncError = prometheus.NewCounterVec(
+	ColdStartsError = prometheus.NewCounterVec(
 		prometheus.CounterOpts{
 			Name: "fission_function_cold_start_errors_total",
 			Help: "Count of fission cold start errors",
@@ -54,5 +54,5 @@ func init() {
 	registry := metrics.Registry
 	registry.MustRegister(ColdStarts)
 	registry.MustRegister(FuncRunningSummary)
-	registry.MustRegister(FuncError)
+	registry.MustRegister(ColdStartsError)
 }