Skip to content

Commit

Permalink
refactor: Review metrics endpoints (#1770)
Browse files Browse the repository at this point in the history
Signed-off-by: Alexei Dodon <[email protected]>
  • Loading branch information
adodon2go authored Sep 15, 2023
1 parent aae8b7b commit 14206dd
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 80 deletions.
102 changes: 51 additions & 51 deletions examples/config-allextensions.json
Original file line number Diff line number Diff line change
@@ -1,53 +1,53 @@
{
"distSpecVersion": "1.1.0-dev",
"storage": {
"rootDirectory": "/tmp/zot"
},
"http": {
"address": "127.0.0.1",
"port": "8080"
},
"log": {
"level": "debug"
},
"extensions": {
"metrics": {},
"sync": {
"credentialsFile": "./examples/sync-auth-filepath.json",
"registries": [
{
"urls": [
"https://registry1:5000"
],
"onDemand": false,
"pollInterval": "6h",
"tlsVerify": true,
"certDir": "/home/user/certs",
"maxRetries": 3,
"retryDelay": "15m",
"content": [
{
"prefix": "/repo1/repo",
"tags": {
"regex": "4.*",
"semver": true
}
},
{
"prefix": "/repo2/repo"
}
]
}
]
},
"search": {
"cve": {
"updateInterval": "2h"
}
},
"scrub": {
"enable": true,
"interval": "24h"
}
}
"distSpecVersion": "1.1.0-dev",
"storage": {
"rootDirectory": "/tmp/zot"
},
"http": {
"address": "127.0.0.1",
"port": "8080"
},
"log": {
"level": "debug"
},
"extensions": {
"metrics": {},
"sync": {
"credentialsFile": "./examples/sync-auth-filepath.json",
"registries": [
{
"urls": [
"https://registry1:5000"
],
"onDemand": false,
"pollInterval": "6h",
"tlsVerify": true,
"certDir": "/home/user/certs",
"maxRetries": 3,
"retryDelay": "15m",
"content": [
{
"prefix": "/repo1/repo",
"tags": {
"regex": "4.*",
"semver": true
}
},
{
"prefix": "/repo2/repo"
}
]
}
]
},
"search": {
"cve": {
"updateInterval": "2h"
}
},
"scrub": {
"enable": true,
"interval": "24h"
}
}
}
2 changes: 1 addition & 1 deletion examples/config-anonymous-authz.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
"read"
]
}
}
}
}
},
"log": {
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,10 @@ func isOpenIDAuthProviderEnabled(config *Config, provider string) bool {
return false
}

func (c *Config) IsMetricsEnabled() bool {
return c.Extensions != nil && c.Extensions.Metrics != nil && *c.Extensions.Metrics.Enable
}

func (c *Config) IsSearchEnabled() bool {
return c.Extensions != nil && c.Extensions.Search != nil && *c.Extensions.Search.Enable
}
Expand Down
19 changes: 1 addition & 18 deletions pkg/api/routes.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,20 +179,8 @@ func (rh *RouteHandler) SetupRoutes() {
// gql playground
gqlPlayground.SetupGQLPlaygroundRoutes(prefixedRouter, rh.c.StoreController, rh.c.Log)

// setup extension routes
if rh.c.Config != nil {
// This logic needs to be reviewed, it should depend on build options
// not the general presence of the extensions in config
if rh.c.Config.Extensions == nil {
// minimal build
prefixedRouter.HandleFunc("/metrics", rh.GetMetrics).Methods("GET")
} else {
// extended build
ext.SetupMetricsRoutes(rh.c.Config, rh.c.Router, authHandler, rh.c.Log)
}
}

// Preconditions for enabling the actual extension routes are part of extensions themselves
ext.SetupMetricsRoutes(rh.c.Config, rh.c.Router, authHandler, rh.c.Log, rh.c.Metrics)
ext.SetupSearchRoutes(rh.c.Config, prefixedRouter, rh.c.StoreController, rh.c.MetaDB, rh.c.CveInfo,
rh.c.Log)
ext.SetupImageTrustRoutes(rh.c.Config, prefixedRouter, rh.c.MetaDB, rh.c.Log)
Expand Down Expand Up @@ -1860,11 +1848,6 @@ func (rh *RouteHandler) OpenIDCodeExchangeCallback() rp.CodeExchangeUserinfoCall
}
}

func (rh *RouteHandler) GetMetrics(w http.ResponseWriter, r *http.Request) {
m := rh.c.Metrics.ReceiveMetrics()
zcommon.WriteJSON(w, http.StatusOK, m)
}

// helper routines

func getContentRange(r *http.Request) (int64 /* from */, int64 /* to */, error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/api/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func SessionLogger(ctlr *Controller) mux.MiddlewareFunc {
path = path + "?" + raw
}

if path != "/v2/metrics" {
if path != "/metrics" {
// In order to test metrics feture,the instrumentation related to node exporter
// should be handled by node exporter itself (ex: latency)
monitoring.IncHTTPConnRequests(ctlr.Metrics, method, strconv.Itoa(statusCode))
Expand Down
3 changes: 2 additions & 1 deletion pkg/exporter/api/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ func TestNewExporter(t *testing.T) {
So(servercConfig, ShouldNotBeNil)
baseURL := fmt.Sprintf(BaseURL, serverPort)
servercConfig.HTTP.Port = serverPort
servercConfig.BinaryType = "minimal"
serverController := zotapi.NewController(servercConfig)
So(serverController, ShouldNotBeNil)

Expand Down Expand Up @@ -149,7 +150,7 @@ func TestNewExporter(t *testing.T) {
}

// Side effect of calling this endpoint is that it will enable metrics
resp, err := resty.R().Get(baseURL + "/v2/metrics")
resp, err := resty.R().Get(baseURL + "/metrics")
So(resp, ShouldNotBeNil)
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, 200)
Expand Down
8 changes: 4 additions & 4 deletions pkg/extensions/extension_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ import (
"github.com/prometheus/client_golang/prometheus/promhttp"

"zotregistry.io/zot/pkg/api/config"
"zotregistry.io/zot/pkg/extensions/monitoring"
"zotregistry.io/zot/pkg/log"
)

func EnableMetricsExtension(config *config.Config, log log.Logger, rootDir string) {
if config.Extensions.Metrics != nil &&
*config.Extensions.Metrics.Enable &&
if config.IsMetricsEnabled() &&
config.Extensions.Metrics.Prometheus != nil {
if config.Extensions.Metrics.Prometheus.Path == "" {
config.Extensions.Metrics.Prometheus.Path = "/metrics"
Expand All @@ -26,11 +26,11 @@ func EnableMetricsExtension(config *config.Config, log log.Logger, rootDir strin
}

func SetupMetricsRoutes(config *config.Config, router *mux.Router,
authFunc mux.MiddlewareFunc, log log.Logger,
authFunc mux.MiddlewareFunc, log log.Logger, metrics monitoring.MetricServer,
) {
log.Info().Msg("setting up metrics routes")

if config.Extensions.Metrics != nil && *config.Extensions.Metrics.Enable {
if config.IsMetricsEnabled() {
extRouter := router.PathPrefix(config.Extensions.Metrics.Prometheus.Path).Subrouter()
extRouter.Use(authFunc)
extRouter.Methods("GET").Handler(promhttp.Handler())
Expand Down
14 changes: 11 additions & 3 deletions pkg/extensions/extension_metrics_disabled.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@
package extensions

import (
"net/http"

"github.com/gorilla/mux"

"zotregistry.io/zot/pkg/api/config"
zcommon "zotregistry.io/zot/pkg/common"
"zotregistry.io/zot/pkg/extensions/monitoring"
"zotregistry.io/zot/pkg/log"
)

Expand All @@ -18,8 +22,12 @@ func EnableMetricsExtension(config *config.Config, log log.Logger, rootDir strin

// SetupMetricsRoutes ...
func SetupMetricsRoutes(conf *config.Config, router *mux.Router,
authFunc mux.MiddlewareFunc, log log.Logger,
authFunc mux.MiddlewareFunc, log log.Logger, metrics monitoring.MetricServer,
) {
log.Warn().Msg("skipping setting up metrics routes because given zot binary doesn't include this feature," +
"please build a binary that does so")
getMetrics := func(w http.ResponseWriter, r *http.Request) {
m := metrics.ReceiveMetrics()
zcommon.WriteJSON(w, http.StatusOK, m)
}

router.HandleFunc("/metrics", getMetrics).Methods("GET")
}
2 changes: 1 addition & 1 deletion pkg/extensions/monitoring/minimal_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ func NewMetricsClient(config *MetricsConfig, logger log.Logger) *MetricsClient {

func (mc *MetricsClient) GetMetrics() (*MetricsInfo, error) {
metrics := &MetricsInfo{}
if _, err := mc.makeGETRequest(mc.config.Address+"/v2/metrics", metrics); err != nil {
if _, err := mc.makeGETRequest(mc.config.Address+"/metrics", metrics); err != nil {
return nil, err
}

Expand Down

0 comments on commit 14206dd

Please sign in to comment.