Skip to content

Commit c8f1fb3

Browse files
Roland Bracewell Shoemakerjsha
Roland Bracewell Shoemaker
authored and
jsha
committed
Remove direct usages of go-statsd-client in favor of using metrics.Scope (letsencrypt#2136)
Fixes letsencrypt#2118, fixes letsencrypt#2082.
1 parent a9d476e commit c8f1fb3

File tree

39 files changed

+316
-316
lines changed

39 files changed

+316
-316
lines changed

‎akamai/cache-client.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,11 @@ import (
1414
"strings"
1515
"time"
1616

17-
"github.com/cactus/go-statsd-client/statsd"
1817
"github.com/jmhodges/clock"
19-
"github.com/letsencrypt/boulder/core"
2018

19+
"github.com/letsencrypt/boulder/core"
2120
blog "github.com/letsencrypt/boulder/log"
21+
"github.com/letsencrypt/boulder/metrics"
2222
)
2323

2424
const (
@@ -52,7 +52,7 @@ type CachePurgeClient struct {
5252
retries int
5353
retryBackoff time.Duration
5454
log blog.Logger
55-
stats statsd.Statter
55+
stats metrics.Scope
5656
clk clock.Clock
5757
}
5858

@@ -77,8 +77,9 @@ func NewCachePurgeClient(
7777
retries int,
7878
retryBackoff time.Duration,
7979
log blog.Logger,
80-
stats statsd.Statter,
80+
stats metrics.Scope,
8181
) (*CachePurgeClient, error) {
82+
stats = stats.NewScope("CCU")
8283
if strings.HasSuffix(endpoint, "/") {
8384
endpoint = endpoint[:len(endpoint)-1]
8485
}
@@ -181,7 +182,7 @@ func (cpc *CachePurgeClient) purge(urls []string) error {
181182

182183
rS := cpc.clk.Now()
183184
resp, err := cpc.client.Do(req)
184-
cpc.stats.TimingDuration("CCU.PurgeRequestLatency", time.Since(rS), 1.0)
185+
cpc.stats.TimingDuration("PurgeRequestLatency", time.Since(rS))
185186
if err != nil {
186187
return err
187188
}
@@ -229,21 +230,21 @@ func (cpc *CachePurgeClient) Purge(urls []string) error {
229230
if err != nil {
230231
if _, ok := err.(errFatal); ok {
231232
cpc.log.AuditErr(err.Error())
232-
cpc.stats.Inc("CCU.FatalFailures", 1, 1.0)
233+
cpc.stats.Inc("FatalFailures", 1)
233234
return err
234235
}
235-
cpc.stats.Inc("CCU.RetryableFailures", 1, 1.0)
236+
cpc.stats.Inc("RetryableFailures", 1)
236237
continue
237238
}
238239
successful = true
239240
break
240241
}
241242

242243
if !successful {
243-
cpc.stats.Inc("CCU.FatalFailures", 1, 1.0)
244+
cpc.stats.Inc("FatalFailures", 1)
244245
return ErrAllRetriesFailed
245246
}
246247

247-
cpc.stats.Inc("CCU.SuccessfulPurges", 1, 1.0)
248+
cpc.stats.Inc("SuccessfulPurges", 1)
248249
return nil
249250
}

‎akamai/cache-client_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import (
77
"testing"
88
"time"
99

10-
"github.com/cactus/go-statsd-client/statsd"
1110
"github.com/jmhodges/clock"
1211

12+
"github.com/letsencrypt/boulder/metrics"
1313
"github.com/letsencrypt/boulder/test"
1414
)
1515

1616
func TestConstructAuthHeader(t *testing.T) {
17-
stats, _ := statsd.NewNoopClient(nil)
17+
stats := metrics.NewNoopScope()
1818
cpc, err := NewCachePurgeClient(
1919
"https://akaa-baseurl-xxxxxxxxxxx-xxxxxxxxxxxxx.luna.akamaiapis.net",
2020
"akab-client-token-xxx-xxxxxxxxxxxxxxxx",

‎bdns/dns.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ func NewDNSResolverImpl(
184184
clk clock.Clock,
185185
maxTries int,
186186
) *DNSResolverImpl {
187+
stats = stats.NewScope("DNS")
187188
// TODO(jmhodges): make constructor use an Option func pattern
188189
dnsClient := new(dns.Client)
189190

‎bdns/dns_test.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212

1313
"golang.org/x/net/context"
1414

15-
"github.com/cactus/go-statsd-client/statsd"
1615
"github.com/jmhodges/clock"
1716
"github.com/letsencrypt/boulder/metrics"
1817
"github.com/letsencrypt/boulder/test"
@@ -220,8 +219,7 @@ func TestMain(m *testing.M) {
220219
}
221220

222221
func newTestStats() metrics.Scope {
223-
c, _ := statsd.NewNoopClient()
224-
return metrics.NewStatsdScope(c, "fakesvc")
222+
return metrics.NewNoopScope()
225223
}
226224

227225
var testStats = newTestStats()

‎ca/ca.go‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"strings"
1818
"time"
1919

20-
"github.com/cactus/go-statsd-client/statsd"
2120
cfsslConfig "github.com/cloudflare/cfssl/config"
2221
cferr "github.com/cloudflare/cfssl/errors"
2322
"github.com/cloudflare/cfssl/ocsp"
@@ -32,6 +31,7 @@ import (
3231
csrlib "github.com/letsencrypt/boulder/csr"
3332
"github.com/letsencrypt/boulder/goodkey"
3433
blog "github.com/letsencrypt/boulder/log"
34+
"github.com/letsencrypt/boulder/metrics"
3535
)
3636

3737
// Miscellaneous PKIX OIDs that we need to refer to
@@ -74,25 +74,25 @@ var (
7474
// Metrics for CA statistics
7575
const (
7676
// Increments when CA observes an HSM or signing error
77-
metricSigningError = "CA.SigningError"
77+
metricSigningError = "SigningError"
7878
metricHSMError = metricSigningError + ".HSMError"
7979

8080
// Increments when CA handles a CSR requesting a "basic" extension:
8181
// authorityInfoAccess, authorityKeyIdentifier, extKeyUsage, keyUsage,
8282
// basicConstraints, certificatePolicies, crlDistributionPoints,
8383
// subjectAlternativeName, subjectKeyIdentifier,
84-
metricCSRExtensionBasic = "CA.CSRExtensions.Basic"
84+
metricCSRExtensionBasic = "CSRExtensions.Basic"
8585

8686
// Increments when CA handles a CSR requesting a TLS Feature extension
87-
metricCSRExtensionTLSFeature = "CA.CSRExtensions.TLSFeature"
87+
metricCSRExtensionTLSFeature = "CSRExtensions.TLSFeature"
8888

8989
// Increments when CA handles a CSR requesting a TLS Feature extension with
9090
// an invalid value
91-
metricCSRExtensionTLSFeatureInvalid = "CA.CSRExtensions.TLSFeatureInvalid"
91+
metricCSRExtensionTLSFeatureInvalid = "CSRExtensions.TLSFeatureInvalid"
9292

9393
// Increments when CA handles a CSR requesting an extension other than those
9494
// listed above
95-
metricCSRExtensionOther = "CA.CSRExtensions.Other"
95+
metricCSRExtensionOther = "CSRExtensions.Other"
9696
)
9797

9898
type certificateStorage interface {
@@ -114,7 +114,7 @@ type CertificateAuthorityImpl struct {
114114
keyPolicy goodkey.KeyPolicy
115115
clk clock.Clock
116116
log blog.Logger
117-
stats statsd.Statter
117+
stats metrics.Scope
118118
prefix int // Prepended to the serial number
119119
validityPeriod time.Duration
120120
maxNames int
@@ -179,7 +179,7 @@ func makeInternalIssuers(
179179
func NewCertificateAuthorityImpl(
180180
config cmd.CAConfig,
181181
clk clock.Clock,
182-
stats statsd.Statter,
182+
stats metrics.Scope,
183183
issuers []Issuer,
184184
keyPolicy goodkey.KeyPolicy,
185185
logger blog.Logger,
@@ -255,9 +255,9 @@ func NewCertificateAuthorityImpl(
255255
func (ca *CertificateAuthorityImpl) noteSignError(err error) {
256256
if err != nil {
257257
if _, ok := err.(*pkcs11.Error); ok {
258-
ca.stats.Inc(metricHSMError, 1, 1.0)
258+
ca.stats.Inc(metricHSMError, 1)
259259
} else if cfErr, ok := err.(*cferr.Error); ok {
260-
ca.stats.Inc(fmt.Sprintf("%s.%d", metricSigningError, cfErr.ErrorCode), 1, 1.0)
260+
ca.stats.Inc(fmt.Sprintf("%s.%d", metricSigningError, cfErr.ErrorCode), 1)
261261
}
262262
}
263263
return
@@ -292,14 +292,14 @@ func (ca *CertificateAuthorityImpl) extensionsFromCSR(csr *x509.CertificateReque
292292

293293
switch {
294294
case ext.Type.Equal(oidTLSFeature):
295-
ca.stats.Inc(metricCSRExtensionTLSFeature, 1, 1.0)
295+
ca.stats.Inc(metricCSRExtensionTLSFeature, 1)
296296
value, ok := ext.Value.([]byte)
297297
if !ok {
298298
msg := fmt.Sprintf("Malformed extension with OID %v", ext.Type)
299299
return nil, core.MalformedRequestError(msg)
300300
} else if !bytes.Equal(value, mustStapleFeatureValue) {
301301
msg := fmt.Sprintf("Unsupported value for extension with OID %v", ext.Type)
302-
ca.stats.Inc(metricCSRExtensionTLSFeatureInvalid, 1, 1.0)
302+
ca.stats.Inc(metricCSRExtensionTLSFeatureInvalid, 1)
303303
return nil, core.MalformedRequestError(msg)
304304
}
305305

@@ -324,11 +324,11 @@ func (ca *CertificateAuthorityImpl) extensionsFromCSR(csr *x509.CertificateReque
324324
}
325325

326326
if hasBasic {
327-
ca.stats.Inc(metricCSRExtensionBasic, 1, 1.0)
327+
ca.stats.Inc(metricCSRExtensionBasic, 1)
328328
}
329329

330330
if hasOther {
331-
ca.stats.Inc(metricCSRExtensionOther, 1, 1.0)
331+
ca.stats.Inc(metricCSRExtensionOther, 1)
332332
}
333333

334334
return extensions, nil

‎ca/ca_test.go‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,17 @@ import (
1313

1414
cfsslConfig "github.com/cloudflare/cfssl/config"
1515
"github.com/cloudflare/cfssl/helpers"
16+
"github.com/golang/mock/gomock"
1617
"github.com/jmhodges/clock"
18+
"github.com/letsencrypt/boulder/metrics/mock_metrics"
1719
"golang.org/x/crypto/ocsp"
1820
"golang.org/x/net/context"
1921

2022
"github.com/letsencrypt/boulder/cmd"
2123
"github.com/letsencrypt/boulder/core"
2224
"github.com/letsencrypt/boulder/goodkey"
2325
blog "github.com/letsencrypt/boulder/log"
26+
"github.com/letsencrypt/boulder/metrics"
2427
"github.com/letsencrypt/boulder/mocks"
2528
"github.com/letsencrypt/boulder/policy"
2629
"github.com/letsencrypt/boulder/test"
@@ -133,7 +136,7 @@ type testCtx struct {
133136
issuers []Issuer
134137
keyPolicy goodkey.KeyPolicy
135138
fc clock.FakeClock
136-
stats *mocks.Statter
139+
stats metrics.Scope
137140
logger blog.Logger
138141
}
139142

@@ -236,8 +239,6 @@ func setup(t *testing.T) *testCtx {
236239
},
237240
}
238241

239-
stats := mocks.NewStatter()
240-
241242
issuers := []Issuer{{caKey, caCert}}
242243

243244
keyPolicy := goodkey.KeyPolicy{
@@ -254,7 +255,7 @@ func setup(t *testing.T) *testCtx {
254255
issuers,
255256
keyPolicy,
256257
fc,
257-
stats,
258+
metrics.NewNoopScope(),
258259
logger,
259260
}
260261
}
@@ -684,10 +685,15 @@ func countMustStaple(t *testing.T, cert *x509.Certificate) (count int) {
684685
func TestExtensions(t *testing.T) {
685686
testCtx := setup(t)
686687
testCtx.caConfig.MaxNames = 3
688+
689+
ctrl := gomock.NewController(t)
690+
defer ctrl.Finish()
691+
stats := mock_metrics.NewMockScope(ctrl)
692+
687693
ca, err := NewCertificateAuthorityImpl(
688694
testCtx.caConfig,
689695
testCtx.fc,
690-
testCtx.stats,
696+
stats,
691697
testCtx.issuers,
692698
testCtx.keyPolicy,
693699
testCtx.logger)
@@ -717,36 +723,34 @@ func TestExtensions(t *testing.T) {
717723

718724
// With enableMustStaple = false, should issue successfully and not add
719725
// Must Staple.
726+
stats.EXPECT().Inc(metricCSRExtensionTLSFeature, int64(1)).Return(nil)
720727
noStapleCert := sign(mustStapleCSR)
721728
test.AssertEquals(t, countMustStaple(t, noStapleCert), 0)
722729

723730
// With enableMustStaple = true, a TLS feature extension should put a must-staple
724731
// extension into the cert
725732
ca.enableMustStaple = true
733+
stats.EXPECT().Inc(metricCSRExtensionTLSFeature, int64(1)).Return(nil)
726734
singleStapleCert := sign(mustStapleCSR)
727735
test.AssertEquals(t, countMustStaple(t, singleStapleCert), 1)
728-
test.AssertEquals(t, testCtx.stats.Counters[metricCSRExtensionTLSFeature], int64(2))
729736

730737
// Even if there are multiple TLS Feature extensions, only one extension should be included
738+
stats.EXPECT().Inc(metricCSRExtensionTLSFeature, int64(1)).Return(nil)
731739
duplicateMustStapleCert := sign(duplicateMustStapleCSR)
732740
test.AssertEquals(t, countMustStaple(t, duplicateMustStapleCert), 1)
733-
test.AssertEquals(t, testCtx.stats.Counters[metricCSRExtensionTLSFeature], int64(3))
734741

735742
// ... but if it doesn't ask for stapling, there should be an error
743+
stats.EXPECT().Inc(metricCSRExtensionTLSFeature, int64(1)).Return(nil)
744+
stats.EXPECT().Inc(metricCSRExtensionTLSFeatureInvalid, int64(1)).Return(nil)
736745
_, err = ca.IssueCertificate(ctx, *tlsFeatureUnknownCSR, 1001)
737746
test.AssertError(t, err, "Allowed a CSR with an empty TLS feature extension")
738747
if _, ok := err.(core.MalformedRequestError); !ok {
739748
t.Errorf("Wrong error type when rejecting a CSR with empty TLS feature extension")
740749
}
741-
test.AssertEquals(t, testCtx.stats.Counters[metricCSRExtensionTLSFeature], int64(4))
742-
test.AssertEquals(t, testCtx.stats.Counters[metricCSRExtensionTLSFeatureInvalid], int64(1))
743750

744751
// Unsupported extensions should be silently ignored, having the same
745752
// extensions as the TLS Feature cert above, minus the TLS Feature Extension
753+
stats.EXPECT().Inc(metricCSRExtensionOther, int64(1)).Return(nil)
746754
unsupportedExtensionCert := sign(unsupportedExtensionCSR)
747755
test.AssertEquals(t, len(unsupportedExtensionCert.Extensions), len(singleStapleCert.Extensions)-1)
748-
test.AssertEquals(t, testCtx.stats.Counters[metricCSRExtensionOther], int64(1))
749-
750-
// None of the above CSRs have basic extensions
751-
test.AssertEquals(t, testCtx.stats.Counters[metricCSRExtensionBasic], int64(0))
752756
}

‎cmd/admin-revoker/main.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"golang.org/x/net/context"
1313

14-
"github.com/cactus/go-statsd-client/statsd"
1514
gorp "gopkg.in/gorp.v1"
1615

1716
"github.com/letsencrypt/boulder/cmd"
@@ -55,23 +54,24 @@ type config struct {
5554
Syslog cmd.SyslogConfig
5655
}
5756

58-
func setupContext(c config) (rpc.RegistrationAuthorityClient, blog.Logger, *gorp.DbMap, rpc.StorageAuthorityClient, statsd.Statter) {
57+
func setupContext(c config) (rpc.RegistrationAuthorityClient, blog.Logger, *gorp.DbMap, rpc.StorageAuthorityClient, metrics.Scope) {
5958
stats, logger := cmd.StatsAndLogging(c.Statsd, c.Syslog)
59+
scope := metrics.NewStatsdScope(stats, "AdminRevoker")
6060

6161
amqpConf := c.Revoker.AMQP
62-
rac, err := rpc.NewRegistrationAuthorityClient(clientName, amqpConf, stats)
62+
rac, err := rpc.NewRegistrationAuthorityClient(clientName, amqpConf, scope)
6363
cmd.FailOnError(err, "Unable to create CA client")
6464

6565
dbURL, err := c.Revoker.DBConfig.URL()
6666
cmd.FailOnError(err, "Couldn't load DB URL")
6767
dbMap, err := sa.NewDbMap(dbURL, c.Revoker.DBConfig.MaxDBConns)
6868
cmd.FailOnError(err, "Couldn't setup database connection")
69-
go sa.ReportDbConnCount(dbMap, metrics.NewStatsdScope(stats, "AdminRevoker"))
69+
go sa.ReportDbConnCount(dbMap, scope)
7070

71-
sac, err := rpc.NewStorageAuthorityClient(clientName, amqpConf, stats)
71+
sac, err := rpc.NewStorageAuthorityClient(clientName, amqpConf, scope)
7272
cmd.FailOnError(err, "Failed to create SA client")
7373

74-
return *rac, logger, dbMap, *sac, stats
74+
return *rac, logger, dbMap, *sac, scope
7575
}
7676

7777
func revokeBySerial(ctx context.Context, serial string, reasonCode revocation.Reason, rac rpc.RegistrationAuthorityClient, logger blog.Logger, tx *gorp.Transaction) (err error) {
@@ -224,8 +224,8 @@ func main() {
224224
authsRevoked,
225225
pendingAuthsRevoked,
226226
))
227-
stats.Inc("admin-revoker.revokedAuthorizations", authsRevoked, 1.0)
228-
stats.Inc("admin-revoker.revokedPendingAuthorizations", pendingAuthsRevoked, 1.0)
227+
stats.Inc("RevokedAuthorizations", authsRevoked)
228+
stats.Inc("RevokedPendingAuthorizations", pendingAuthsRevoked)
229229

230230
default:
231231
usage()

0 commit comments

Comments
 (0)