From 452c9be06df58d0344756417ed71b77fbdffd19e Mon Sep 17 00:00:00 2001 From: bobby <1544881+desimone@users.noreply.github.com> Date: Mon, 22 Jun 2020 06:59:04 -0700 Subject: [PATCH] cache: remove unused metrics and options (#957) Signed-off-by: Bobby DeSimone --- cache/cache_test.go | 6 +- config/options.go | 25 +----- config/options_test.go | 2 +- go.mod | 3 - go.sum | 6 -- internal/telemetry/metrics/kv.go | 107 ------------------------- internal/telemetry/metrics/kv_test.go | 88 -------------------- internal/telemetry/metrics/registry.go | 36 --------- 8 files changed, 7 insertions(+), 266 deletions(-) delete mode 100644 internal/telemetry/metrics/kv.go delete mode 100644 internal/telemetry/metrics/kv_test.go diff --git a/cache/cache_test.go b/cache/cache_test.go index e03a6fec6..500136781 100644 --- a/cache/cache_test.go +++ b/cache/cache_test.go @@ -23,10 +23,10 @@ func TestNew(t *testing.T) { opts config.Options wantErr bool }{ - {"good - autocache", config.Options{CacheStore: "autocache", SharedKey: cryptutil.NewBase64Key(), DataBrokerURL: &url.URL{Scheme: "http", Host: "example"}}, false}, - {"bad shared secret", config.Options{CacheStorePath: dir + "/bolt.db", CacheStore: "bolt", SharedKey: string([]byte(cryptutil.NewBase64Key())[:31]), DataBrokerURL: &url.URL{Scheme: "http", Host: "example"}}, true}, + {"good - autocache", config.Options{SharedKey: cryptutil.NewBase64Key(), DataBrokerURL: &url.URL{Scheme: "http", Host: "example"}}, false}, + {"bad shared secret", config.Options{SharedKey: string([]byte(cryptutil.NewBase64Key())[:31]), DataBrokerURL: &url.URL{Scheme: "http", Host: "example"}}, true}, {"bad cache url", config.Options{SharedKey: cryptutil.NewBase64Key(), DataBrokerURL: &url.URL{}}, true}, - {"good - bolt", config.Options{CacheStorePath: dir + "/bolt.db", CacheStore: "bolt", SharedKey: cryptutil.NewBase64Key(), DataBrokerURL: &url.URL{Scheme: "http", Host: "example"}}, false}, + {"good - bolt", config.Options{SharedKey: cryptutil.NewBase64Key(), DataBrokerURL: &url.URL{Scheme: "http", Host: "example"}}, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/config/options.go b/config/options.go index 76cac0671..3cf03f001 100644 --- a/config/options.go +++ b/config/options.go @@ -206,14 +206,11 @@ type Options struct { ForwardAuthURLString string `mapstructure:"forward_auth_url" yaml:"forward_auth_url,omitempty"` ForwardAuthURL *url.URL `yaml:",omitempty"` - // CacheStore is the name of session cache backend to use. - // Options are : "bolt", "redis", and "autocache". - // Default is "autocache". - CacheStore string `mapstructure:"cache_store" yaml:"cache_store,omitempty"` - // CacheURL is the routable destination of the cache service's // gRPC endpoint. NOTE: As many load balancers do not support // externally routed gRPC so this may be an internal location. + // + // TODO(BDD): Deprecate and remove in 0.11.0 CacheURLString string `mapstructure:"cache_service_url" yaml:"cache_service_url,omitempty"` CacheURL *url.URL `yaml:",omitempty"` @@ -221,14 +218,6 @@ type Options struct { DataBrokerURLString string `mapstructure:"databroker_service_url" yaml:"databroker_service_url,omitempty"` DataBrokerURL *url.URL `yaml:",omitempty"` - // CacheStoreAddr specifies the host and port on which the cache store - // should connect to. e.g. (localhost:6379) - CacheStoreAddr string `mapstructure:"cache_store_address" yaml:"cache_store_address,omitempty"` - // CacheStorePassword is the password used to connect to the cache store. - CacheStorePassword string `mapstructure:"cache_store_password" yaml:"cache_store_password,omitempty"` - // CacheStorePath is the path to use for a given cache store. e.g. /etc/bolt.db - CacheStorePath string `mapstructure:"cache_store_path" yaml:"cache_store_path,omitempty"` - // ClientCA is the base64-encoded certificate authority to validate client mTLS certificates against. ClientCA string `mapstructure:"client_ca" yaml:"client_ca,omitempty"` // ClientCAFile points to a file that contains the certificate authority to validate client mTLS certificates against. @@ -270,7 +259,6 @@ var defaultOptions = Options{ GRPCClientDNSRoundRobin: true, GRPCServerMaxConnectionAge: 5 * time.Minute, GRPCServerMaxConnectionAgeGrace: 5 * time.Minute, - CacheStore: "autocache", AuthenticateCallbackPath: "/oauth2/callback", TracingSampleRate: 0.0001, @@ -486,6 +474,7 @@ func (o *Options) Validate() error { } if o.DataBrokerURLString == "" { + log.Warn().Msg("config: cache url will be deprecated in v0.11.0") o.DataBrokerURLString = o.CacheURLString } @@ -523,14 +512,6 @@ func (o *Options) Validate() error { o.AuthorizeURL = u } - if o.CacheURLString != "" { - u, err := urlutil.ParseAndValidateURL(o.CacheURLString) - if err != nil { - return fmt.Errorf("config: bad cache service url %s : %w", o.CacheURLString, err) - } - o.CacheURL = u - } - if o.DataBrokerURLString != "" { u, err := urlutil.ParseAndValidateURL(o.DataBrokerURLString) if err != nil { diff --git a/config/options_test.go b/config/options_test.go index e34c8fe2c..ad69e7b92 100644 --- a/config/options_test.go +++ b/config/options_test.go @@ -207,7 +207,7 @@ func Test_Checksum(t *testing.T) { func TestOptionsFromViper(t *testing.T) { t.Parallel() opts := []cmp.Option{ - cmpopts.IgnoreFields(Options{}, "CacheStore", "CookieSecret", "GRPCInsecure", "GRPCAddr", "CacheURLString", "CacheURL", "DataBrokerURLString", "DataBrokerURL", "AuthorizeURL", "AuthorizeURLString", "DefaultUpstreamTimeout", "CookieExpire", "Services", "Addr", "RefreshCooldown", "LogLevel", "KeyFile", "CertFile", "SharedKey", "ReadTimeout", "IdleTimeout", "GRPCClientTimeout", "GRPCClientDNSRoundRobin", "TracingSampleRate"), + cmpopts.IgnoreFields(Options{}, "CookieSecret", "GRPCInsecure", "GRPCAddr", "CacheURLString", "CacheURL", "DataBrokerURLString", "DataBrokerURL", "AuthorizeURL", "AuthorizeURLString", "DefaultUpstreamTimeout", "CookieExpire", "Services", "Addr", "RefreshCooldown", "LogLevel", "KeyFile", "CertFile", "SharedKey", "ReadTimeout", "IdleTimeout", "GRPCClientTimeout", "GRPCClientDNSRoundRobin", "TracingSampleRate"), cmpopts.IgnoreFields(Policy{}, "Source", "Destination"), cmpOptIgnoreUnexported, } diff --git a/go.mod b/go.mod index 169b81cab..2df1b3af1 100644 --- a/go.mod +++ b/go.mod @@ -13,8 +13,6 @@ require ( github.com/envoyproxy/go-control-plane v0.9.5 github.com/fsnotify/fsnotify v1.4.9 github.com/go-chi/chi v4.1.2+incompatible - github.com/go-redis/redis/v7 v7.4.0 - github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e github.com/golang/mock v1.4.3 github.com/golang/protobuf v1.4.2 github.com/google/btree v1.0.0 @@ -52,7 +50,6 @@ require ( github.com/stretchr/testify v1.6.1 github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 github.com/uber/jaeger-client-go v2.20.1+incompatible // indirect - go.etcd.io/bbolt v1.3.4 go.opencensus.io v0.22.3 golang.org/x/crypto v0.0.0-20200604202706-70a84ac30bf9 golang.org/x/net v0.0.0-20200602114024-627f9648deb9 diff --git a/go.sum b/go.sum index da7cb6f25..c5f6eeca8 100644 --- a/go.sum +++ b/go.sum @@ -145,8 +145,6 @@ github.com/go-kit/kit v0.8.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2 github.com/go-kit/kit v0.9.0/go.mod h1:xBxKIO96dXMWWy0MnWVtmwkA9/13aqxPnvrjFYMA2as= github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9GBnD5lWE= github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk= -github.com/go-redis/redis/v7 v7.4.0 h1:7obg6wUoj05T0EpY0o8B59S9w5yeMWql7sw2kwNW1x4= -github.com/go-redis/redis/v7 v7.4.0/go.mod h1:JDNMw23GTyLNC4GZu9njt15ctBQVn7xjRfnwdHj/Dcg= github.com/go-sql-driver/mysql v1.5.0/go.mod h1:DCzpHaOWr8IXmIStZouvnhqoel9Qv2LBy8hT2VhHyBg= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y= @@ -351,13 +349,11 @@ github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn github.com/olekukonko/tablewriter v0.0.1/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo= github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.7.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= -github.com/onsi/ginkgo v1.10.1/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.11.0 h1:JAKSXpt1YjtLA7YpPiqO9ss6sNXEsPfSGdwN0UHqzrw= github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/gocleanup v0.0.0-20140331211545-c1a5478700b5 h1:uuhPqmc+m7Nj7btxZEjdEUv+uFoBHNf2Tk/E7gGM+kY= github.com/onsi/gocleanup v0.0.0-20140331211545-c1a5478700b5/go.mod h1:tHaogb+iP6wJXwCqVUlmxYuJb4XDyEKxxs3E4DvMBK0= github.com/onsi/gomega v1.4.3/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= -github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.8.1 h1:C5Dqfs/LeauYDX0jJXIe2SWmwCbGzx9yF8C8xy3Lh34= github.com/onsi/gomega v1.8.1/go.mod h1:Ho0h+IUsWyvy1OpqCwxlQ/21gkhVunqlU8fDGcoTdcA= github.com/open-policy-agent/opa v0.20.5 h1:1zEofrGa+a1Tb186yflIVMkkdAQujG3ySPbeTmR+py0= @@ -504,8 +500,6 @@ github.com/yashtewari/glob-intersection v0.0.0-20180916065949-5c77d914dd0b h1:vV github.com/yashtewari/glob-intersection v0.0.0-20180916065949-5c77d914dd0b/go.mod h1:HptNXiXVDcJjXe9SqMd0v2FsL9f8dz4GnXgltU6q/co= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= go.etcd.io/bbolt v1.3.2/go.mod h1:IbVyRI1SCnLcuJnV2u8VeU0CEYM7e686BmAb1XKL+uU= -go.etcd.io/bbolt v1.3.4 h1:hi1bXHMVrlQh6WwxAy+qZCV/SYIlqo+Ushwdpa4tAKg= -go.etcd.io/bbolt v1.3.4/go.mod h1:G5EMThwa9y8QZGBClrRx5EY+Yw9kAhnjy3bSjsnlVTQ= go.opencensus.io v0.20.1/go.mod h1:6WKK9ahsWS3RSO+PY9ZHZUfv2irvY6gN279GOPZjmmk= go.opencensus.io v0.20.2/go.mod h1:6WKK9ahsWS3RSO+PY9ZHZUfv2irvY6gN279GOPZjmmk= go.opencensus.io v0.21.0/go.mod h1:mSImk1erAIZhrmZN+AvHh14ztQfjbGwt4TtuofqLduU= diff --git a/internal/telemetry/metrics/kv.go b/internal/telemetry/metrics/kv.go deleted file mode 100644 index f0a5bd3f9..000000000 --- a/internal/telemetry/metrics/kv.go +++ /dev/null @@ -1,107 +0,0 @@ -package metrics - -import ( - "github.com/go-redis/redis/v7" - "github.com/golang/groupcache" - "go.etcd.io/bbolt" -) - -// AddGroupCacheMetrics registers a metrics handler against a *groupcache.Group -func AddGroupCacheMetrics(gc *groupcache.Group) { - - cumulativeMetrics := []struct { - name string - desc string - f func() int64 - }{ - {"groupcache_gets_total", "Total get request, including from peers", gc.Stats.Gets.Get}, - {"groupcache_cache_hits_total", "Total cache hits in local or cluster cache", gc.Stats.CacheHits.Get}, - {"groupcache_cache_hits_total", "Total cache hits in local or cluster cache", gc.Stats.CacheHits.Get}, - {"groupcache_peer_loads_total", "Total remote loads or cache hits without error", gc.Stats.PeerLoads.Get}, - {"groupcache_peer_errors_total", "Total errors from peers", gc.Stats.PeerErrors.Get}, - {"groupcache_loads_total", "Total gets without cache hits", gc.Stats.Loads.Get}, - {"groupcache_loads_deduped_total", "gets without cache hits after duplicate suppression", gc.Stats.LoadsDeduped.Get}, - {"groupcache_local_loads_total", "Total good local loads", gc.Stats.LocalLoads.Get}, - {"groupcache_local_load_errs_total", "Total local load errors", gc.Stats.LocalLoadErrs.Get}, - {"groupcache_server_requests_total", "Total gets from peers", gc.Stats.ServerRequests.Get}, - } - - for _, m := range cumulativeMetrics { - registry.addInt64DerivedCumulativeMetric(m.name, m.desc, "autocache", m.f) - } -} - -// AddBoltDBMetrics registers a metrics handler against a *bbolt.DB -func AddBoltDBMetrics(stats func() bbolt.Stats) { - gaugeMetrics := []struct { - name string - desc string - f func() int64 - }{ - {"boltdb_free_page_n", "Number of free pages on the freelist", func() int64 { return int64(stats().FreePageN) }}, - {"boltdb_pending_page_n", "Number of pending pages on the freelist", func() int64 { return int64(stats().PendingPageN) }}, - {"boltdb_free_alloc_size_bytes", "Bytes allocated in free pages", func() int64 { return int64(stats().FreeAlloc) }}, - {"boltdb_freelist_inuse_size_bytes", "Bytes used by the freelist", func() int64 { return int64(stats().FreelistInuse) }}, - {"boltdb_txn", "total number of started read transactions", func() int64 { return int64(stats().TxN) }}, - {"boltdb_open_txn", "number of currently open read transactions", func() int64 { return int64(stats().OpenTxN) }}, - } - - for _, m := range gaugeMetrics { - registry.addInt64DerivedGaugeMetric(m.name, m.desc, "boltdb", m.f) - } - - cumulativeMetrics := []struct { - name string - desc string - f func() int64 - }{ - {"boltdb_txn_page_total", "Total number of page allocations", func() int64 { return int64(stats().TxStats.PageCount) }}, - {"boltdb_txn_page_alloc_size_bytes_total", "Total bytes allocated", func() int64 { return int64(stats().TxStats.PageAlloc) }}, - {"boltdb_txn_cursor_total", "Total number of cursors created", func() int64 { return int64(stats().TxStats.CursorCount) }}, - {"boltdb_txn_node_total", "Total number of node allocations", func() int64 { return int64(stats().TxStats.NodeCount) }}, - {"boltdb_txn_node_deref_total", "Total number of node dereferences", func() int64 { return int64(stats().TxStats.NodeDeref) }}, - {"boltdb_txn_rebalance_total", "Total number of node rebalances", func() int64 { return int64(stats().TxStats.Rebalance) }}, - {"boltdb_txn_rebalance_duration_ms_total", "Total time spent rebalancing", func() int64 { return stats().TxStats.RebalanceTime.Milliseconds() }}, - {"boltdb_txn_split_total", "Total number of nodes split", func() int64 { return int64(stats().TxStats.Split) }}, - {"boltdb_txn_spill_total", "Total number of nodes spilled", func() int64 { return int64(stats().TxStats.Spill) }}, - {"boltdb_txn_spill_duration_ms_total", "Total time spent spilling", func() int64 { return stats().TxStats.SpillTime.Milliseconds() }}, - {"boltdb_txn_write_total", "Total number of writes performed", func() int64 { return int64(stats().TxStats.Write) }}, - {"boltdb_txn_write_duration_ms_total", "Total time spent writing to disk", func() int64 { return stats().TxStats.WriteTime.Milliseconds() }}, - } - - for _, m := range cumulativeMetrics { - registry.addInt64DerivedCumulativeMetric(m.name, m.desc, "boltdb", m.f) - } - -} - -// AddRedisMetrics registers a metrics handler against a redis Client's PoolStats() method -func AddRedisMetrics(stats func() *redis.PoolStats) { - gaugeMetrics := []struct { - name string - desc string - f func() int64 - }{ - {"redis_conns", "Number of total connections in the pool", func() int64 { return int64(stats().TotalConns) }}, - {"redis_idle_conns", "Number of idle connections in the pool", func() int64 { return int64(stats().IdleConns) }}, - } - - for _, m := range gaugeMetrics { - registry.addInt64DerivedGaugeMetric(m.name, m.desc, "redis", m.f) - } - - cumulativeMetrics := []struct { - name string - desc string - f func() int64 - }{ - {"redis_hits_total", "Total number of times free connection was found in the pool", func() int64 { return int64(stats().Hits) }}, - {"redis_misses_total", "Total number of times free connection was NOT found in the pool", func() int64 { return int64(stats().Misses) }}, - {"redis_timeouts_total", "Total number of times a wait timeout occurred", func() int64 { return int64(stats().Timeouts) }}, - {"redis_stale_conns_total", "Total number of stale connections removed from the pool", func() int64 { return int64(stats().StaleConns) }}, - } - - for _, m := range cumulativeMetrics { - registry.addInt64DerivedCumulativeMetric(m.name, m.desc, "redis", m.f) - } -} diff --git a/internal/telemetry/metrics/kv_test.go b/internal/telemetry/metrics/kv_test.go deleted file mode 100644 index 083986200..000000000 --- a/internal/telemetry/metrics/kv_test.go +++ /dev/null @@ -1,88 +0,0 @@ -package metrics - -import ( - "testing" - "time" - - "github.com/go-redis/redis/v7" - "github.com/golang/groupcache" - "go.etcd.io/bbolt" - "go.opencensus.io/metric/metricdata" -) - -func Test_AddGroupCacheMetrics(t *testing.T) { - gc := &groupcache.Group{} - AddGroupCacheMetrics(gc) - - tests := []struct { - name string - stat *groupcache.AtomicInt - want int64 - }{ - {"groupcache_gets_total", &gc.Stats.Gets, 4}, - {"groupcache_loads_total", &gc.Stats.Loads, 42}, - {"groupcache_server_requests_total", &gc.Stats.ServerRequests, 8}, - } - - labelValues := []metricdata.LabelValue{ - metricdata.NewLabelValue("autocache"), - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tt.stat.Add(tt.want) - testMetricRetrieval(registry.registry.Read(), t, labelValues, tt.want, tt.name) - }) - } - -} - -func Test_AddBoltDBMetrics(t *testing.T) { - tests := []struct { - name string - stat bbolt.Stats - want int64 - }{ - {"boltdb_free_page_n", bbolt.Stats{FreePageN: 14}, 14}, - {"boltdb_txn", bbolt.Stats{TxN: 88}, 88}, - - {"boltdb_txn_rebalance_duration_ms_total", bbolt.Stats{TxStats: bbolt.TxStats{RebalanceTime: 42 * time.Millisecond}}, 42}, - {"boltdb_txn_write_total", bbolt.Stats{TxStats: bbolt.TxStats{Write: 42}}, 42}, - } - - labelValues := []metricdata.LabelValue{ - metricdata.NewLabelValue("boltdb"), - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - AddBoltDBMetrics(func() bbolt.Stats { return tt.stat }) - testMetricRetrieval(registry.registry.Read(), t, labelValues, tt.want, tt.name) - }) - } - -} - -func Test_AddRedisMetrics(t *testing.T) { - tests := []struct { - name string - stat *redis.PoolStats - want int64 - }{ - {"redis_conns", &redis.PoolStats{TotalConns: 7}, 7}, - {"redis_hits_total", &redis.PoolStats{Hits: 78}, 78}, - {"redis_timeouts_total", &redis.PoolStats{Timeouts: 2}, 2}, - } - - labelValues := []metricdata.LabelValue{ - metricdata.NewLabelValue("redis"), - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - AddRedisMetrics(func() *redis.PoolStats { return tt.stat }) - testMetricRetrieval(registry.registry.Read(), t, labelValues, tt.want, tt.name) - }) - } - -} diff --git a/internal/telemetry/metrics/registry.go b/internal/telemetry/metrics/registry.go index 1aef576d3..79c2a921a 100644 --- a/internal/telemetry/metrics/registry.go +++ b/internal/telemetry/metrics/registry.go @@ -106,39 +106,3 @@ func (r *metricRegistry) setConfigChecksum(service string, checksum uint64) { } m.Set(float64(checksum)) } - -func (r *metricRegistry) addInt64DerivedGaugeMetric(name string, desc string, service string, f func() int64) { - - m, err := r.registry.AddInt64DerivedGauge(name, metric.WithDescription(desc), metric.WithLabelKeys("service")) - if err != nil { - log.Error().Err(err).Str("service", service).Msg("telemetry/metrics: failed to register metric") - return - } - - err = m.UpsertEntry( - f, - metricdata.NewLabelValue(service), - ) - if err != nil { - log.Error().Err(err).Str("service", service).Msg("telemetry/metrics: failed to update metric") - return - } -} - -func (r *metricRegistry) addInt64DerivedCumulativeMetric(name string, desc string, service string, f func() int64) { - - m, err := r.registry.AddInt64DerivedCumulative(name, metric.WithDescription(desc), metric.WithLabelKeys("service")) - if err != nil { - log.Error().Err(err).Str("service", service).Msg("telemetry/metrics: failed to register metric") - return - } - - err = m.UpsertEntry( - f, - metricdata.NewLabelValue(service), - ) - if err != nil { - log.Error().Err(err).Str("service", service).Msg("telemetry/metrics: failed to update metric") - return - } -}