From ad10691b4b6a17a8e6c8d83fa0bb6d99cedecff9 Mon Sep 17 00:00:00 2001 From: pleasew8t Date: Fri, 29 Nov 2024 13:47:10 +0200 Subject: [PATCH] add error counters and address review comment --- node/pkg/guardiansigner/amazonkms.go | 7 ++- node/pkg/guardiansigner/benchmarksigner.go | 60 ++++++++++++++++++---- node/pkg/guardiansigner/filesigner.go | 5 ++ node/pkg/guardiansigner/generatedsigner.go | 5 ++ node/pkg/guardiansigner/guardiansigner.go | 2 + 5 files changed, 67 insertions(+), 12 deletions(-) diff --git a/node/pkg/guardiansigner/amazonkms.go b/node/pkg/guardiansigner/amazonkms.go index 01f8afb6f3..7001c62803 100644 --- a/node/pkg/guardiansigner/amazonkms.go +++ b/node/pkg/guardiansigner/amazonkms.go @@ -88,7 +88,7 @@ func NewAmazonKmsSigner(ctx context.Context, unsafeDevMode bool, keyPath string) amazonKmsSigner := AmazonKms{ keyId: keyPath, - region: getRegionFromArn(keyPath), + region: region, } // Create a configuration object to create a new KMS client from. The region passed to @@ -208,6 +208,11 @@ func (a *AmazonKms) Verify(ctx context.Context, sig []byte, hash []byte) (bool, return recoveredPubKey.Equal(kmsPublicKey), nil } +// Return the signer type as "amazonkms". +func (a *AmazonKms) TypeAsString() string { + return "amazonkms" +} + // https://bitcoin.stackexchange.com/questions/92680/what-are-the-der-signature-and-sec-format // 1. 0x30 byte: header byte to indicate compound structure // 2. one byte to encode the length of the following data diff --git a/node/pkg/guardiansigner/benchmarksigner.go b/node/pkg/guardiansigner/benchmarksigner.go index 14dae8059f..8af8fdb270 100644 --- a/node/pkg/guardiansigner/benchmarksigner.go +++ b/node/pkg/guardiansigner/benchmarksigner.go @@ -24,25 +24,48 @@ type BenchmarkSigner struct { } var ( + guardianSignerSigningLatency prometheus.Histogram + guardianSignerSigningErrorCount prometheus.Counter + guardianSignerVerifyLatency prometheus.Histogram + guardianSignerVerifyErrorCount prometheus.Counter +) + +func BenchmarkWrappedSigner(innerSigner GuardianSigner) *BenchmarkSigner { + if innerSigner == nil { + return nil + } + + signerType := innerSigner.TypeAsString() + guardianSignerSigningLatency = promauto.NewHistogram( prometheus.HistogramOpts{ - Name: "wormhole_guardian_signer_signing_latency_us", - Help: "Latency histogram for Guardian signing requests", - Buckets: []float64{10.0, 20.0, 50.0, 100.0, 1000.0, 5000.0, 10000.0, 100_000.0, 1_000_000.0, 10_000_000.0, 100_000_000.0, 1_000_000_000.0}, + Name: "wormhole_guardian_signer_signing_latency_us", + Help: "Latency histogram for Guardian signing requests", + Buckets: []float64{10.0, 20.0, 50.0, 100.0, 1000.0, 5000.0, 10000.0, 100_000.0, 1_000_000.0, 10_000_000.0, 100_000_000.0, 1_000_000_000.0}, + ConstLabels: prometheus.Labels{"signer_type": signerType}, + }) + + guardianSignerSigningErrorCount = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "wormhole_guardian_signer_signing_error_count", + Help: "Total number of errors that ocurred during Guardian signing requests", + ConstLabels: prometheus.Labels{"signer_type": signerType}, }) guardianSignerVerifyLatency = promauto.NewHistogram( prometheus.HistogramOpts{ - Name: "wormhole_guardian_signer_sig_verify_latency_us", - Help: "Latency histogram for Guardian signature verification requests", - Buckets: []float64{10.0, 20.0, 50.0, 100.0, 1000.0, 5000.0, 10000.0, 100_000.0, 1_000_000.0, 10_000_000.0, 100_000_000.0, 1_000_000_000.0}, + Name: "wormhole_guardian_signer_sig_verify_latency_us", + Help: "Latency histogram for Guardian signature verification requests", + Buckets: []float64{10.0, 20.0, 50.0, 100.0, 1000.0, 5000.0, 10000.0, 100_000.0, 1_000_000.0, 10_000_000.0, 100_000_000.0, 1_000_000_000.0}, + ConstLabels: prometheus.Labels{"signer_type": signerType}, }) -) -func BenchmarkWrappedSigner(innerSigner GuardianSigner) *BenchmarkSigner { - if innerSigner == nil { - return nil - } + guardianSignerVerifyErrorCount = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "wormhole_guardian_signer_verify_error_count", + Help: "Total number of errors that ocurred during Guardian signature verification requests", + ConstLabels: prometheus.Labels{"signer_type": signerType}, + }) return &BenchmarkSigner{ innerSigner: innerSigner, @@ -57,6 +80,11 @@ func (b *BenchmarkSigner) Sign(ctx context.Context, hash []byte) ([]byte, error) // Add Observation to histogram guardianSignerSigningLatency.Observe(float64(duration.Microseconds())) + // If an error occured, increment the error counter + if err != nil { + guardianSignerSigningErrorCount.Inc() + } + return sig, err } @@ -74,5 +102,15 @@ func (b *BenchmarkSigner) Verify(ctx context.Context, sig []byte, hash []byte) ( // Add observation to histogram guardianSignerVerifyLatency.Observe(float64(duration.Microseconds())) + // If an error occured, increment the error counter + if err != nil { + guardianSignerVerifyErrorCount.Inc() + } + return valid, err } + +// Return the type of signer as "benchmark". +func (b *BenchmarkSigner) TypeAsString() string { + return "benchmark" +} diff --git a/node/pkg/guardiansigner/filesigner.go b/node/pkg/guardiansigner/filesigner.go index f0a02ecfeb..c0cf8c1cd2 100644 --- a/node/pkg/guardiansigner/filesigner.go +++ b/node/pkg/guardiansigner/filesigner.go @@ -106,3 +106,8 @@ func (fs *FileSigner) Verify(ctx context.Context, sig []byte, hash []byte) (bool return recoveredPubKey.Equal(fsPubkey), nil } + +// Return the signer type as "file". +func (fs *FileSigner) TypeAsString() string { + return "file" +} diff --git a/node/pkg/guardiansigner/generatedsigner.go b/node/pkg/guardiansigner/generatedsigner.go index 47052201a1..1324e93621 100644 --- a/node/pkg/guardiansigner/generatedsigner.go +++ b/node/pkg/guardiansigner/generatedsigner.go @@ -57,6 +57,11 @@ func (gs *GeneratedSigner) Verify(ctx context.Context, sig []byte, hash []byte) return recoveredPubKey.Equal(fsPubkey), nil } +// Return the signer type as "generated". +func (gs *GeneratedSigner) TypeAsString() string { + return "generated" +} + // This function is meant to be a helper function that returns a guardian signer for tests // that simply require a private key. The caller can specify a private key to be used, or // pass nil to have `NewGeneratedSigner` generate a random private key. diff --git a/node/pkg/guardiansigner/guardiansigner.go b/node/pkg/guardiansigner/guardiansigner.go index 79b3dcc781..6c9aca9ce8 100644 --- a/node/pkg/guardiansigner/guardiansigner.go +++ b/node/pkg/guardiansigner/guardiansigner.go @@ -32,6 +32,8 @@ type GuardianSigner interface { // Verify is a convenience function that recovers a public key from the sig/hash pair, // and checks if the public key matches that of the guardian signer. Verify(ctx context.Context, sig []byte, hash []byte) (valid bool, err error) + // Return the type of signer as string. + TypeAsString() string } // Create a new GuardianSigner from the given URI. The caller can also specify the