-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add metrics to Elastic Package Registry #827
Changes from all commits
3e7a8a6
352170c
f5132cd
4e4cea2
5e02260
0fb89c3
54f8d76
73cca37
a1853ee
4cef3f1
7d41766
105e637
9aa7948
e83079a
3fbc736
b510843
6d29990
871a863
8906a03
4d8226c
b508695
42ecdd7
0031521
679bb07
7d9ff68
14ad680
4913a1f
bc44972
8492d33
f05309e
584bd06
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ Endpoints: | |
* `/search`: Search for packages. By default returns all the most recent packages available. | ||
* `/categories`: List of the existing package categories and how many packages are in each category. | ||
* `/package/{name}/{version}`: Info about a package | ||
* `/epr/{name}/{name}-{version}.tar.gz`: Download a package | ||
* `/epr/{name}/{name}-{version}.zip`: Download a package | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to the artifact handler, just zip extension is allowed in the regex There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, good catch :) |
||
|
||
Examples for each API endpoint can be found here: https://github.com/elastic/package-registry/tree/main/docs/api | ||
|
||
|
@@ -236,6 +236,19 @@ It will be listening in the given address. | |
|
||
You can read more about this profiler and the available endpoints in the [pprof documentation](https://pkg.go.dev/net/http/pprof). | ||
|
||
## Metrics | ||
mrodm marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Package registry can be instrumented to expose Prometheus metrics under `/metrics` endpoint. | ||
By default this endpoint is disabled. | ||
|
||
To enable this instrumentation, the required address (host and port) where this endpoint needs | ||
to run must be set using the parameter `metrics-address` (or the `EPR_METRICS_ADDRESS` environment variable). | ||
For example: | ||
|
||
``` | ||
package-registry --metrics-address 0.0.0.0:9000 | ||
``` | ||
|
||
## Release | ||
|
||
New versions of the package registry need to be released from time to time. The following steps should be followed to create a new release: | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,32 +19,39 @@ import ( | |
gstorage "cloud.google.com/go/storage" | ||
"github.com/gorilla/mux" | ||
"github.com/pkg/errors" | ||
"github.com/prometheus/client_golang/prometheus" | ||
"github.com/prometheus/client_golang/prometheus/promhttp" | ||
"go.elastic.co/apm" | ||
"go.elastic.co/apm/module/apmgorilla" | ||
"go.uber.org/zap" | ||
|
||
ucfgYAML "github.com/elastic/go-ucfg/yaml" | ||
|
||
"github.com/elastic/package-registry/metrics" | ||
"github.com/elastic/package-registry/packages" | ||
"github.com/elastic/package-registry/storage" | ||
"github.com/elastic/package-registry/util" | ||
) | ||
|
||
const ( | ||
serviceName = "package-registry" | ||
version = "1.9.1" | ||
serviceName = "package-registry" | ||
version = "1.9.1" | ||
defaultInstanceName = "localhost" | ||
) | ||
|
||
var ( | ||
address string | ||
httpProfAddress string | ||
metricsAddress string | ||
|
||
tlsCertFile string | ||
tlsKeyFile string | ||
|
||
dryRun bool | ||
configPath string | ||
|
||
printVersionInfo bool | ||
|
||
featureStorageIndexer bool | ||
storageIndexerBucketInternal string | ||
storageEndpoint string | ||
|
@@ -59,7 +66,9 @@ var ( | |
) | ||
|
||
func init() { | ||
flag.BoolVar(&printVersionInfo, "version", false, "Print Elastic Package Registry version") | ||
flag.StringVar(&address, "address", "localhost:8080", "Address of the package-registry service.") | ||
flag.StringVar(&metricsAddress, "metrics-address", "", "Address to expose the Prometheus metrics.") | ||
flag.StringVar(&tlsCertFile, "tls-cert", "", "Path of the TLS certificate.") | ||
flag.StringVar(&tlsKeyFile, "tls-key", "", "Path of the TLS key.") | ||
flag.StringVar(&configPath, "config", "config.yml", "Path to the configuration file.") | ||
|
@@ -86,22 +95,36 @@ type Config struct { | |
func main() { | ||
parseFlags() | ||
|
||
if printVersionInfo { | ||
fmt.Printf("Elastic Package Registry version %v\n", version) | ||
os.Exit(0) | ||
} | ||
|
||
logger := util.Logger() | ||
defer logger.Sync() | ||
|
||
config := mustLoadConfig(logger) | ||
if dryRun { | ||
logger.Info("Running dry-run mode") | ||
_ = initIndexers(context.Background(), logger, config) | ||
os.Exit(0) | ||
} | ||
|
||
logger.Info("Package registry started") | ||
defer logger.Info("Package registry stopped") | ||
|
||
initHttpProf(logger) | ||
|
||
server := initServer(logger) | ||
server := initServer(logger, config) | ||
go func() { | ||
err := runServer(server) | ||
if err != nil && err != http.ErrServerClosed { | ||
logger.Fatal("error occurred while serving", zap.Error(err)) | ||
} | ||
}() | ||
|
||
initMetricsServer(logger) | ||
jsoriano marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
stop := make(chan os.Signal, 1) | ||
signal.Notify(stop, os.Interrupt, syscall.SIGTERM) | ||
<-stop | ||
|
@@ -126,17 +149,38 @@ func initHttpProf(logger *zap.Logger) { | |
}() | ||
} | ||
|
||
func initServer(logger *zap.Logger) *http.Server { | ||
apmTracer := initAPMTracer(logger) | ||
tx := apmTracer.StartTransaction("initServer", "backend.init") | ||
defer tx.End() | ||
func getHostname() string { | ||
hostname, err := os.Hostname() | ||
if err != nil { | ||
return defaultInstanceName | ||
} | ||
return hostname | ||
} | ||
|
||
ctx := apm.ContextWithTransaction(context.TODO(), tx) | ||
func initMetricsServer(logger *zap.Logger) { | ||
if metricsAddress == "" { | ||
return | ||
} | ||
|
||
config := mustLoadConfig(logger) | ||
hostname := getHostname() | ||
|
||
metrics.ServiceInfo.With(prometheus.Labels{"version": version, "instance": hostname}).Set(1) | ||
|
||
logger.Info("Starting http metrics in " + metricsAddress) | ||
go func() { | ||
router := http.NewServeMux() | ||
router.Handle("/metrics", promhttp.Handler()) | ||
err := http.ListenAndServe(metricsAddress, router) | ||
if err != nil { | ||
logger.Fatal("failed to start Prometheus metrics endpoint", zap.Error(err)) | ||
} | ||
}() | ||
} | ||
|
||
func initIndexers(ctx context.Context, logger *zap.Logger, config *Config) CombinedIndexer { | ||
packagesBasePaths := getPackagesBasePaths(config) | ||
|
||
var indexers []Indexer | ||
var indexers CombinedIndexer | ||
if featureStorageIndexer { | ||
storageClient, err := gstorage.NewClient(ctx) | ||
if err != nil { | ||
|
@@ -154,10 +198,17 @@ func initServer(logger *zap.Logger) *http.Server { | |
combinedIndexer := NewCombinedIndexer(indexers...) | ||
ensurePackagesAvailable(ctx, logger, combinedIndexer) | ||
|
||
// If -dry-run=true is set, service stops here after validation | ||
if dryRun { | ||
os.Exit(0) | ||
} | ||
return combinedIndexer | ||
} | ||
|
||
func initServer(logger *zap.Logger, config *Config) *http.Server { | ||
apmTracer := initAPMTracer(logger) | ||
tx := apmTracer.StartTransaction("initServer", "backend.init") | ||
defer tx.End() | ||
|
||
ctx := apm.ContextWithTransaction(context.TODO(), tx) | ||
|
||
combinedIndexer := initIndexers(ctx, logger, config) | ||
|
||
router := mustLoadRouter(logger, config, combinedIndexer) | ||
apmgorilla.Instrument(router, apmgorilla.WithTracer(apmTracer)) | ||
|
@@ -223,6 +274,8 @@ func getPackagesBasePaths(config *Config) []string { | |
|
||
func printConfig(logger *zap.Logger, config *Config) { | ||
logger.Info("Packages paths: " + strings.Join(config.PackagePaths, ", ")) | ||
logger.Info("Cache time for /: " + config.CacheTimeIndex.String()) | ||
logger.Info("Cache time for /index.json: " + config.CacheTimeIndex.String()) | ||
Comment on lines
+277
to
+278
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just added these info messages for completeness |
||
logger.Info("Cache time for /search: " + config.CacheTimeSearch.String()) | ||
logger.Info("Cache time for /categories: " + config.CacheTimeCategories.String()) | ||
logger.Info("Cache time for all others: " + config.CacheTimeCatchAll.String()) | ||
|
@@ -244,6 +297,7 @@ func ensurePackagesAvailable(ctx context.Context, logger *zap.Logger, indexer In | |
} | ||
|
||
logger.Info(fmt.Sprintf("%v package manifests loaded", len(packages))) | ||
metrics.NumberIndexedPackages.Set(float64(len(packages))) | ||
} | ||
|
||
func mustLoadRouter(logger *zap.Logger, config *Config, indexer Indexer) *mux.Router { | ||
|
@@ -270,7 +324,6 @@ func getRouter(logger *zap.Logger, config *Config, indexer Indexer) (*mux.Router | |
staticHandler := staticHandler(indexer, config.CacheTimeCatchAll) | ||
|
||
router := mux.NewRouter().StrictSlash(true) | ||
|
||
router.HandleFunc("/", indexHandlerFunc) | ||
router.HandleFunc("/index.json", indexHandlerFunc) | ||
router.HandleFunc("/search", searchHandler(indexer, config.CacheTimeSearch)) | ||
|
@@ -282,6 +335,9 @@ func getRouter(logger *zap.Logger, config *Config, indexer Indexer) (*mux.Router | |
router.HandleFunc(packageIndexRouterPath, packageIndexHandler) | ||
router.HandleFunc(staticRouterPath, staticHandler) | ||
router.Use(util.LoggingMiddleware(logger)) | ||
if metricsAddress != "" { | ||
router.Use(metrics.MetricsMiddleware()) | ||
} | ||
router.NotFoundHandler = http.Handler(notFoundHandler(fmt.Errorf("404 page not found"))) | ||
return router, nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I mark this as experimental here and in the Readme? I was thinking in case some metrics (e.g. namings or buckets) need to be updated later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, I would say that we can go without special labels like experimental or beta. It's just yet another feature. If it starts failing, we will bugfix it.