From 099484ee5fb185e92cd154e29c63cf34201e803f Mon Sep 17 00:00:00 2001 From: Shihang Zhang Date: Thu, 1 Aug 2019 14:49:37 -0700 Subject: [PATCH] inject transformer prefix into metric Change-Id: Iacab685a710d8f8d5b80ed0d35e5ccc22bd929cb --- .../apiserver/pkg/storage/value/metrics.go | 6 +- .../pkg/storage/value/metrics_test.go | 8 +- .../pkg/storage/value/transformer.go | 17 +- .../pkg/storage/value/transformer_test.go | 150 ++++++++++++++++++ 4 files changed, 167 insertions(+), 14 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/metrics.go b/staging/src/k8s.io/apiserver/pkg/storage/value/metrics.go index f827bffaf92..9da584bcdc9 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/metrics.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/metrics.go @@ -63,7 +63,7 @@ var ( Name: "transformation_operations_total", Help: "Total number of transformations.", }, - []string{"transformation_type", "status"}, + []string{"transformation_type", "transformer_prefix", "status"}, ) deprecatedTransformerFailuresTotal = prometheus.NewCounterVec( @@ -130,8 +130,8 @@ 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 string, start time.Time, err error) { - transformerOperationsTotal.WithLabelValues(transformationType, status.Code(err).String()).Inc() +func RecordTransformation(transformationType, transformerPrefix string, start time.Time, err error) { + transformerOperationsTotal.WithLabelValues(transformationType, transformerPrefix, status.Code(err).String()).Inc() switch { case err == nil: diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/metrics_test.go b/staging/src/k8s.io/apiserver/pkg/storage/value/metrics_test.go index aba5454591b..bfdbb404962 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/metrics_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/metrics_test.go @@ -49,7 +49,7 @@ func TestTotals(t *testing.T) { apiserver_storage_transformation_failures_total{transformation_type="encrypt"} 1 # HELP apiserver_storage_transformation_operations_total Total number of transformations. # TYPE apiserver_storage_transformation_operations_total counter - apiserver_storage_transformation_operations_total{status="Unknown",transformation_type="encrypt"} 1 + apiserver_storage_transformation_operations_total{status="Unknown",transformation_type="encrypt",transformer_prefix="k8s:enc:kms:v1:"} 1 `, }, { @@ -61,7 +61,7 @@ func TestTotals(t *testing.T) { want: ` # HELP apiserver_storage_transformation_operations_total Total number of transformations. # TYPE apiserver_storage_transformation_operations_total counter - apiserver_storage_transformation_operations_total{status="OK",transformation_type="encrypt"} 1 + apiserver_storage_transformation_operations_total{status="OK",transformation_type="encrypt",transformer_prefix="k8s:enc:kms:v1:"} 1 `, }, { @@ -77,7 +77,7 @@ func TestTotals(t *testing.T) { apiserver_storage_transformation_failures_total{transformation_type="encrypt"} 1 # HELP apiserver_storage_transformation_operations_total Total number of transformations. # TYPE apiserver_storage_transformation_operations_total counter - apiserver_storage_transformation_operations_total{status="FailedPrecondition",transformation_type="encrypt"} 1 + apiserver_storage_transformation_operations_total{status="FailedPrecondition",transformation_type="encrypt",transformer_prefix="k8s:enc:kms:v1:"} 1 `, }, } @@ -86,7 +86,7 @@ func TestTotals(t *testing.T) { for _, tt := range testCases { t.Run(tt.desc, func(t *testing.T) { - RecordTransformation("encrypt", time.Now(), tt.error) + RecordTransformation("encrypt", "k8s:enc:kms:v1:", time.Now(), tt.error) defer transformerOperationsTotal.Reset() defer deprecatedTransformerFailuresTotal.Reset() if err := testutil.GatherAndCompare(prometheus.DefaultGatherer, strings.NewReader(tt.want), tt.metrics...); err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/transformer.go b/staging/src/k8s.io/apiserver/pkg/storage/value/transformer.go index bad6ed58e33..bb0f1157c34 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/transformer.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/transformer.go @@ -85,18 +85,12 @@ func (t *MutableTransformer) Set(transformer Transformer) { } func (t *MutableTransformer) TransformFromStorage(data []byte, context Context) (out []byte, stale bool, err error) { - defer func(start time.Time) { - RecordTransformation("from_storage", start, err) - }(time.Now()) t.lock.RLock() transformer := t.transformer t.lock.RUnlock() return transformer.TransformFromStorage(data, context) } func (t *MutableTransformer) TransformToStorage(data []byte, context Context) (out []byte, err error) { - defer func(start time.Time) { - RecordTransformation("to_storage", start, err) - }(time.Now()) t.lock.RLock() transformer := t.transformer t.lock.RUnlock() @@ -134,28 +128,37 @@ func NewPrefixTransformers(err error, transformers ...PrefixTransformer) Transfo // the result of transforming the value. It will always mark any transformation as stale that is not using // the first transformer. func (t *prefixTransformers) TransformFromStorage(data []byte, context Context) ([]byte, bool, error) { + start := time.Now() for i, transformer := range t.transformers { if bytes.HasPrefix(data, transformer.Prefix) { result, stale, err := transformer.Transformer.TransformFromStorage(data[len(transformer.Prefix):], context) // To migrate away from encryption, user can specify an identity transformer higher up // (in the config file) than the encryption transformer. In that scenario, the identity transformer needs to // identify (during reads from disk) whether the data being read is encrypted or not. If the data is encrypted, - // it shall throw an error, but that error should not prevent subsequent transformers from being tried. + // it shall throw an error, but that error should not prevent the next subsequent transformer from being tried. if len(transformer.Prefix) == 0 && err != nil { continue } + if len(transformer.Prefix) == 0 { + RecordTransformation("from_storage", "identity", start, err) + } else { + RecordTransformation("from_storage", string(transformer.Prefix), start, err) + } return result, stale || i != 0, err } } + RecordTransformation("from_storage", "unknown", start, t.err) return nil, false, t.err } // TransformToStorage uses the first transformer and adds its prefix to the data. func (t *prefixTransformers) TransformToStorage(data []byte, context Context) ([]byte, error) { + start := time.Now() transformer := t.transformers[0] prefixedData := make([]byte, len(transformer.Prefix), len(data)+len(transformer.Prefix)) copy(prefixedData, transformer.Prefix) result, err := transformer.Transformer.TransformToStorage(data, context) + RecordTransformation("to_storage", string(transformer.Prefix), start, err) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/apiserver/pkg/storage/value/transformer_test.go b/staging/src/k8s.io/apiserver/pkg/storage/value/transformer_test.go index 63b9e0a7543..3ae9e3e35b3 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/value/transformer_test.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/value/transformer_test.go @@ -19,7 +19,11 @@ package value import ( "bytes" "fmt" + "strings" "testing" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" ) type testTransformer struct { @@ -99,3 +103,149 @@ func TestPrefixTo(t *testing.T) { } } } + +func TestPrefixFromMetrics(t *testing.T) { + testErr := fmt.Errorf("test error") + transformerErr := fmt.Errorf("test error") + identityTransformer := PrefixTransformer{Prefix: []byte{}, Transformer: &testTransformer{from: []byte("value1")}} + identityTransformerErr := PrefixTransformer{Prefix: []byte{}, Transformer: &testTransformer{err: transformerErr}} + otherTransformer := PrefixTransformer{Prefix: []byte("other:"), Transformer: &testTransformer{from: []byte("value1")}} + otherTransformerErr := PrefixTransformer{Prefix: []byte("other:"), Transformer: &testTransformer{err: transformerErr}} + + testCases := []struct { + desc string + input []byte + prefix Transformer + metrics []string + want string + err error + }{ + { + desc: "identity prefix", + input: []byte("value"), + prefix: NewPrefixTransformers(testErr, identityTransformer, otherTransformer), + metrics: []string{ + "apiserver_storage_transformation_operations_total", + }, + want: ` + # HELP apiserver_storage_transformation_operations_total Total number of transformations. + # TYPE apiserver_storage_transformation_operations_total counter + apiserver_storage_transformation_operations_total{status="OK",transformation_type="from_storage",transformer_prefix="identity"} 1 + `, + err: nil, + }, + { + desc: "other prefix (ok)", + input: []byte("other:value"), + prefix: NewPrefixTransformers(testErr, identityTransformerErr, otherTransformer), + metrics: []string{ + "apiserver_storage_transformation_operations_total", + }, + want: ` + # HELP apiserver_storage_transformation_operations_total Total number of transformations. + # TYPE apiserver_storage_transformation_operations_total counter + apiserver_storage_transformation_operations_total{status="OK",transformation_type="from_storage",transformer_prefix="other:"} 1 + `, + err: nil, + }, + { + desc: "other prefix (error)", + input: []byte("other:value"), + prefix: NewPrefixTransformers(testErr, identityTransformerErr, otherTransformerErr), + metrics: []string{ + "apiserver_storage_transformation_operations_total", + }, + want: ` + # HELP apiserver_storage_transformation_operations_total 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 + `, + err: nil, + }, + { + desc: "unknown prefix", + input: []byte("foo:value"), + prefix: NewPrefixTransformers(testErr, identityTransformerErr, otherTransformer), + metrics: []string{ + "apiserver_storage_transformation_operations_total", + }, + want: ` + # HELP apiserver_storage_transformation_operations_total 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 + `, + err: nil, + }, + } + + RegisterMetrics() + transformerOperationsTotal.Reset() + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + tc.prefix.TransformFromStorage(tc.input, nil) + defer transformerOperationsTotal.Reset() + if err := testutil.GatherAndCompare(prometheus.DefaultGatherer, strings.NewReader(tc.want), tc.metrics...); err != nil { + t.Fatal(err) + } + }) + } +} + +func TestPrefixToMetrics(t *testing.T) { + testErr := fmt.Errorf("test error") + transformerErr := fmt.Errorf("test error") + otherTransformer := PrefixTransformer{Prefix: []byte("other:"), Transformer: &testTransformer{from: []byte("value1")}} + otherTransformerErr := PrefixTransformer{Prefix: []byte("other:"), Transformer: &testTransformer{err: transformerErr}} + + testCases := []struct { + desc string + input []byte + prefix Transformer + metrics []string + want string + err error + }{ + { + desc: "ok", + input: []byte("value"), + prefix: NewPrefixTransformers(testErr, otherTransformer), + metrics: []string{ + "apiserver_storage_transformation_operations_total", + }, + want: ` + # HELP apiserver_storage_transformation_operations_total Total number of transformations. + # TYPE apiserver_storage_transformation_operations_total counter + apiserver_storage_transformation_operations_total{status="OK",transformation_type="to_storage",transformer_prefix="other:"} 1 + `, + err: nil, + }, + { + desc: "error", + input: []byte("value"), + prefix: NewPrefixTransformers(testErr, otherTransformerErr), + metrics: []string{ + "apiserver_storage_transformation_operations_total", + }, + want: ` + # HELP apiserver_storage_transformation_operations_total 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 + `, + err: nil, + }, + } + + RegisterMetrics() + transformerOperationsTotal.Reset() + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + tc.prefix.TransformToStorage(tc.input, nil) + defer transformerOperationsTotal.Reset() + if err := testutil.GatherAndCompare(prometheus.DefaultGatherer, strings.NewReader(tc.want), tc.metrics...); err != nil { + t.Fatal(err) + } + }) + } +}