From aceaeaa13e5c3667cf53e4d464985854f82aca36 Mon Sep 17 00:00:00 2001 From: Anish Ramasekar Date: Wed, 17 May 2023 19:48:40 +0000 Subject: [PATCH] update err status code check in transformation metrics Signed-off-by: Anish Ramasekar Kubernetes-commit: 526d45416fb2748f3027ec8735de32d460bd25a8 --- pkg/storage/value/metrics.go | 24 +++++++++++++++++++++++- pkg/storage/value/metrics_test.go | 21 +++++++++++++++++++-- pkg/storage/value/transformer_test.go | 6 +++--- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/pkg/storage/value/metrics.go b/pkg/storage/value/metrics.go index c8fd2f4c0..9b29b1bd6 100644 --- a/pkg/storage/value/metrics.go +++ b/pkg/storage/value/metrics.go @@ -17,9 +17,11 @@ limitations under the License. package value import ( + "errors" "sync" "time" + "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "k8s.io/component-base/metrics" @@ -112,7 +114,7 @@ func RegisterMetrics() { // RecordTransformation records latencies and count of TransformFromStorage and TransformToStorage operations. // Note that transformation_failures_total metric is deprecated, use transformation_operations_total instead. func RecordTransformation(transformationType, transformerPrefix string, elapsed time.Duration, err error) { - transformerOperationsTotal.WithLabelValues(transformationType, transformerPrefix, status.Code(err).String()).Inc() + transformerOperationsTotal.WithLabelValues(transformationType, transformerPrefix, getErrorCode(err)).Inc() if err == nil { transformerLatencies.WithLabelValues(transformationType, transformerPrefix).Observe(elapsed.Seconds()) @@ -138,3 +140,23 @@ func RecordDataKeyGeneration(start time.Time, err error) { func sinceInSeconds(start time.Time) float64 { return time.Since(start).Seconds() } + +type gRPCError interface { + GRPCStatus() *status.Status +} + +func getErrorCode(err error) string { + if err == nil { + return codes.OK.String() + } + + // handle errors wrapped with fmt.Errorf and similar + var s gRPCError + if errors.As(err, &s) { + return s.GRPCStatus().Code().String() + } + + // This is not gRPC error. The operation must have failed before gRPC + // method was called, otherwise we would get gRPC error. + return "unknown-non-grpc" +} diff --git a/pkg/storage/value/metrics_test.go b/pkg/storage/value/metrics_test.go index acd21c721..67ad9e171 100644 --- a/pkg/storage/value/metrics_test.go +++ b/pkg/storage/value/metrics_test.go @@ -19,6 +19,7 @@ package value import ( "context" "errors" + "fmt" "strings" "testing" "time" @@ -34,10 +35,13 @@ func TestTotals(t *testing.T) { nonStatusErr := errors.New("test error") failedPreconditionErr := status.Error(codes.FailedPrecondition, "test error") internalErr := status.Error(codes.Internal, "test error") + wrappedErr := fmt.Errorf("some low level thing failed: %w", status.Error(codes.NotFound, "some error")) + nonStatusErrTransformer := PrefixTransformer{Prefix: []byte("k8s:enc:kms:v1:"), Transformer: &testTransformer{err: nonStatusErr}} failedPreconditionErrTransformer := PrefixTransformer{Prefix: []byte("k8s:enc:kms:v1:"), Transformer: &testTransformer{err: failedPreconditionErr}} internalErrTransformer := PrefixTransformer{Prefix: []byte("k8s:enc:kms:v1:"), Transformer: &testTransformer{err: internalErr}} okTransformer := PrefixTransformer{Prefix: []byte("k8s:enc:kms:v1:"), Transformer: &testTransformer{from: []byte("value")}} + wrappedErrTransformer := PrefixTransformer{Prefix: []byte("k8s:enc:kms:v1:"), Transformer: &testTransformer{err: wrappedErr}} testCases := []struct { desc string @@ -54,8 +58,8 @@ func TestTotals(t *testing.T) { want: ` # HELP apiserver_storage_transformation_operations_total [ALPHA] Total number of transformations. # TYPE apiserver_storage_transformation_operations_total counter - apiserver_storage_transformation_operations_total{status="Unknown",transformation_type="from_storage",transformer_prefix="k8s:enc:kms:v1:"} 1 - apiserver_storage_transformation_operations_total{status="Unknown",transformation_type="to_storage",transformer_prefix="k8s:enc:kms:v1:"} 1 + apiserver_storage_transformation_operations_total{status="unknown-non-grpc",transformation_type="from_storage",transformer_prefix="k8s:enc:kms:v1:"} 1 + apiserver_storage_transformation_operations_total{status="unknown-non-grpc",transformation_type="to_storage",transformer_prefix="k8s:enc:kms:v1:"} 1 `, }, { @@ -97,6 +101,19 @@ func TestTotals(t *testing.T) { apiserver_storage_transformation_operations_total{status="Internal",transformation_type="to_storage",transformer_prefix="k8s:enc:kms:v1:"} 1 `, }, + { + desc: "wrapped not found error", + prefix: NewPrefixTransformers(nil, wrappedErrTransformer), + metrics: []string{ + "apiserver_storage_transformation_operations_total", + }, + want: ` + # HELP apiserver_storage_transformation_operations_total [ALPHA] Total number of transformations. + # TYPE apiserver_storage_transformation_operations_total counter + apiserver_storage_transformation_operations_total{status="NotFound",transformation_type="from_storage",transformer_prefix="k8s:enc:kms:v1:"} 1 + apiserver_storage_transformation_operations_total{status="NotFound",transformation_type="to_storage",transformer_prefix="k8s:enc:kms:v1:"} 1 + `, + }, } RegisterMetrics() diff --git a/pkg/storage/value/transformer_test.go b/pkg/storage/value/transformer_test.go index c204c8c8c..166a9af9d 100644 --- a/pkg/storage/value/transformer_test.go +++ b/pkg/storage/value/transformer_test.go @@ -159,7 +159,7 @@ func TestPrefixFromMetrics(t *testing.T) { want: ` # HELP apiserver_storage_transformation_operations_total [ALPHA] Total number of transformations. # TYPE apiserver_storage_transformation_operations_total counter - apiserver_storage_transformation_operations_total{status="Unknown",transformation_type="from_storage",transformer_prefix="other:"} 1 + apiserver_storage_transformation_operations_total{status="unknown-non-grpc",transformation_type="from_storage",transformer_prefix="other:"} 1 `, err: nil, }, @@ -173,7 +173,7 @@ func TestPrefixFromMetrics(t *testing.T) { want: ` # HELP apiserver_storage_transformation_operations_total [ALPHA] Total number of transformations. # TYPE apiserver_storage_transformation_operations_total counter - apiserver_storage_transformation_operations_total{status="Unknown",transformation_type="from_storage",transformer_prefix="unknown"} 1 + apiserver_storage_transformation_operations_total{status="unknown-non-grpc",transformation_type="from_storage",transformer_prefix="unknown"} 1 `, err: nil, }, @@ -231,7 +231,7 @@ func TestPrefixToMetrics(t *testing.T) { want: ` # HELP apiserver_storage_transformation_operations_total [ALPHA] Total number of transformations. # TYPE apiserver_storage_transformation_operations_total counter - apiserver_storage_transformation_operations_total{status="Unknown",transformation_type="to_storage",transformer_prefix="other:"} 1 + apiserver_storage_transformation_operations_total{status="unknown-non-grpc",transformation_type="to_storage",transformer_prefix="other:"} 1 `, err: nil, },