embed: enable extensive metrics if specified

[Quentin-M]

Following up on #7030.

The extensive metrics were only enabled for the binary form of etcd, and not when using the embed server. This PR alleviate exactly this.

[xiang90]

@Quentin-M can you define this as a const? now extensive string is used at two places.

[xiang90]

actually it seems that https://github.com/coreos/etcd/blob/2fb972847346d838b7395d9b5305ad45c124ff1b/etcdmain/etcd.go#L182 can be removed?

[Quentin-M]

Aren't those two independent mains / code paths? One for standalone etcd and one for embed. That was my understanding anyways. I noticed that the histograms were not available in the embed case, so made that PR.

[xiang90]

@Quentin-M

binary one with main is a wrapper around embed one: see https://github.com/coreos/etcd/blob/2fb972847346d838b7395d9b5305ad45c124ff1b/etcdmain/etcd.go#L181-L186

[Quentin-M]

Ooooh I am blind as a monkey.. I should have used my brain for that PR.

[xiang90]

CI failed with

../vendor/github.com/coreos/etcd/embed/etcd.go:527:3: undefined: grpc_prometheus

[Quentin-M]

Sorry about that, forgot to stage the import.

[xiang90]

@Quentin-M have you manually tested it again for both binary and embed case(we do not have test coverage on this one from automated test yet unfortunately)?

If yes, then LGTM.

[Quentin-M]

Binary

$ git rev-parse --verify HEAD
a535c01050d9241f985ea1ffdba2b4a1fe25ce5
$ docker build -t etcd-ext-metrics .

$ docker run -it --rm -p 2381:2381 etcd-ext-metrics -metrics=extensive -listen-metrics-urls=http://0.0.0.0:2381
$ curl -sS localhost:2381/metrics | grep grpc_server_handling_seconds_bucket | wc -l
     468

$ docker run -it --rm -p 2381:238 etcd-ext-metrics -metrics=basic -listen-metrics-urls=http://0.0.0.0:2381
$ curl -sS localhost:2381/metrics | grep grpc_server_handling_seconds_bucket | wc -l
       0

Embed

// Terrible code, do not use for anything..
package main

import (
	"flag"
	"fmt"
        "net/url"
	"os"
	"os/signal"
	"syscall"

	"github.com/coreos/etcd/embed"
)

func main() {
	metricsURL, _ := url.Parse("http://127.0.0.1:2381")
	metrics := flag.String("metrics", "basic", "basic/extensive")
	flag.Parse()

	cfg := embed.NewConfig()
	cfg.Dir = "/tmp/etcd"
	cfg.ListenMetricsUrls = []url.URL{*metricsURL}
	cfg.Metrics = *metrics
	
	server, err := embed.StartEtcd(cfg)
	if err != nil {
		fmt.Println(err)
		os.Exit(1)
	}

	go func() {
		for {
			select {
			case err := <-server.Err():
				fmt.Println(err)
			}
		}
	}()

	<-server.Server.ReadyNotify()
	fmt.Println("Server Ready")

	sigs := make(chan os.Signal, 1)
	signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
	<-sigs
	
	server.Server.Stop()
	server.Close()
}

$ go run embed.go -metrics=basic
$ curl -sS localhost:2381/metrics | grep grpc_server_handling_seconds_bucket | wc -l
     0

$ go run embed.go -metrics=extensive
$ curl -sS localhost:2381/metrics | grep grpc_server_handling_seconds_bucket | wc -l
     468

[xiang90]

@gyuho @jpbetz

this needs to be backported to 3.2, 3.3 if possible.

[gyuho]

@xiang90 Sure, will backport/release next week.

[gyuho]

fyi, only backport to 3.3, since 3.2 doesn't support metrics URLs

[marksugar]

hello @gyuho ,

I am using the 3.3.10 cluster, but I don't see the ‘grpc_server_handling_seconds_bucket’ indicator in https://etcd.readthedocs.io/en/latest/operate.html#v3-3
Is it replaced or deleted?
thanks

[root@Node ~]# curl -Lks --cert ./server.pem --key ./server-key.pem  https://localhost:2379/metrics|grep grpc_server_handling_seconds_bucket|wc -l
0

[wenjiaswe]

@marksuger did you set the flag --metrics=extensive: https://github.com/etcd-io/etcd/blob/efd1fc634b58a629903990e605f2cb9d5633706d/Documentation/op-guide/configuration.md#--metrics?

[marksugar]

@wenjiaswe
ok
I will try。
thanks