Antrea Prometheus integration #321

ksamoray · 2020-01-16T17:09:18Z

Integrate with Prometheus monitoring solution.
Integration of the Prometheus client into Antrea controller and agent
allows the exposure of various metrics to Prometheus server.
In addition to Antrea's own set of metrics, Prometheus client will also
expose metrics which are defined by various components which are part of
the Antrea ecosystem, e.g golang, Prometheus itself etc.

antrea-bot · 2020-01-16T17:09:30Z

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-all: to trigger all tests.
/skip-all: to skip all tests.

These commands can only be run by members of the vmware-tanzu organization.

ksamoray · 2020-01-16T17:24:19Z

Still missing the controller listener init and obviously more metrics.
Pushing this to make sure that general direction makes sense (likely not).

pkg/agent/metrics/prometheus.go

build/yamls/antrea-ipsec.yml

cmd/antrea-controller/controller.go

abhiraut · 2020-03-12T23:46:30Z

cmd/antrea-controller/controller.go

+
+	klog.Infof("Initializing prometheus host %s port %s", prometheusHost, prometheusPort)
+	gaugeHost := prometheus.NewGauge(prometheus.GaugeOpts{
+		Name: "antrea_controller_host",


some of these can be constants?

IDK if that's necessary. Using constant strings here is a common practice (e.g k8s itself, calico etc).
These values won't be used anywhere except in metric definitions so consts won't be reused anywhere - hence it won't enhance readability.

pkg/agent/metrics/prometheus.go

abhiraut · 2020-03-12T23:53:20Z

rebase and remove wip?

build/yamls/antrea.yml

ksamoray · 2020-03-22T15:35:43Z

/test-all

ksamoray · 2020-03-22T19:40:17Z

/test-all

ksamoray · 2020-03-23T07:39:14Z

/test-all

edwardbadboy · 2020-03-23T09:06:23Z

/test-all

ksamoray · 2020-03-23T11:34:30Z

/test-e2e

ksamoray · 2020-03-23T16:47:28Z

/test-e2e

build/yamls/base/conf/antrea-agent.conf

build/yamls/base/agent.yml

cmd/antrea-controller/controller.go

ksamoray · 2020-03-24T12:09:52Z

/test-all

srikartati · 2020-03-24T21:40:59Z

pkg/agent/metrics/prometheus.go

+			Name: "antrea_agent_local_pod_count",
+			Help: "Number of pods on local node.",
+		},
+		func() float64 { return float64(ifaceStore.GetContainerInterfaceNum()) },


Hi Kobi,
I believe pod count is captured through number of CNIs on the node.
Will this pod count be different from pod count we get from pod metrics/node metrics from Kubernetes native Metrics API under path /apis/metrics.k8s.io/?

Thanks.

There could pods in hostnetwork which won't be managed by CNI, so might be slightly different.

But the metric description doesn't reflect that, I'll change it.

srikartati · 2020-03-24T22:24:41Z

cmd/antrea-controller/controller.go

 	<-stopCh
 	klog.Info("Stopping Antrea controller")
 	return nil
 }

+// Initialize Prometheus listener and metrics collection.


There is no listener anymore right?

ksamoray · 2020-03-25T09:57:32Z

/test-all

ksamoray · 2020-03-25T10:54:39Z

@srikartati the metric exposed by /apis/metrics.k8s.io/ will show all the pods on the node while this one will show the ones connected via Antrea.

ksamoray · 2020-03-25T11:24:30Z

/test-all

ksamoray · 2020-04-23T11:52:20Z

/test-all

tnqn

LGTM, I'm just uncertain if it's safe to expose metrics without authentication as it could have some process and pods information, I see at least Kubernetes secures all of its /metrics api. We may see how it's like for other softwares.
@jianjuns @McCodeman Do you think whether we should leave the API insecure or force secure it.

tnqn · 2020-04-24T07:48:16Z

pkg/ovs/ovsconfig/ovs_client.go

@@ -92,7 +92,7 @@ func NewOVSDBConnectionUDS(address string) (*ovsdb.OVSDB, Error) {
 	return db, nil
 }

-// NewOVSBridge creates and returns a new OVSBridge struct.
+// NewOVSBridge creates and returns a new ovsBridge struct.


Guess this is a side effect of IDE when you renamed OVSBridge of ovsStatManager to ovsBridge.

Indeed. Fixed.

tnqn · 2020-04-24T08:21:34Z

pkg/controller/metrics/prometheus.go

+		klog.Errorf("Failed to retrieve controller K8S node name: %v", err)
+	}
+
+	klog.Info("Initializing prometheus")


Maybe use "Initializing prometheus metrics" as below to keep log consistency.

ksamoray · 2020-04-24T11:51:06Z

@tnqn I'm not very confident about exposure without authentication.
From one hand - it's same as having metrics on a dedicated HTTP port which is a common practice.
On the other hand: I've looked at k8s itself, it secures the metrics over port 6443. which makes me think of two different options:
a) Secure it, figure out the correct config for Prometheus to consume it (if such exists and it ain't tailored to k8s which I doubt).
b) Add a config option to secure /metrics. I don't like this but it's doable.

jianjuns · 2020-04-24T18:22:00Z

@ksamoray @tnqn : I thought PR #622 enable auth for agent API already. Why you are saying exposing metrics without auth? Maybe you mean we do not know how to let Premetheus authenticate with K8s?

ksamoray · 2020-04-24T18:26:16Z

@jianjuns indeed I've secured the agent but I've also created a pinhole for the metrics URL e.g here:
https://github.com/vmware-tanzu/antrea/blob/0ab4ff6ef2c560a73e01a3dee2f049a4aa9039e4/cmd/antrea-controller/controller.go#L128
I think that @tnqn is right and I should get rid of this and have Prometheus authenticate.
I think that I've got this - I'll document how to configure Prometheus within this PR.

tnqn

LGTM, I assume you will have a doc to explain how to enable prometheus metrics for both configuration and RBAC.

ksamoray · 2020-04-26T04:44:25Z

@tnqn I can add the doc in a separate PR or within this one

tnqn · 2020-04-27T16:54:30Z

@jianjuns @antoninbas The PR LGTM, could you take a look so we catch 0.6.0? There are separate PRs for docs and CI.

tnqn · 2020-04-27T16:54:37Z

/test-all

antoninbas · 2020-04-27T23:59:18Z

build/yamls/base/conf/antrea-agent.conf

@@ -46,3 +46,6 @@
 # Note that if it's set to another value, the `containerPort` of the `api` port of the
 # `antrea-agent` container must be set to the same value.
 #apiPort: 10350
+
+# Enable metrics exposure via Prometheus. Initializes Prometheus metrics listener


Missing period at the end (but not for the controller config).

Still see the same issue in other files.

Yeah, make manifest could help here :/

I'm surprised you didn't get a CI error actually. If the manifests are not up-to-date (i.e. if the committer forgot to run make manifest), one of the CI jobs should fail.

antoninbas · 2020-04-28T00:07:16Z

pkg/agent/metrics/prometheus.go

+		ConstLabels: prometheus.Labels{"k8s_nodename": nodeName, "k8s_podname": env.GetPodName()},
+	})
+	gaugeHost.Set(1)
+	prometheus.MustRegister(gaugeHost)


maybe a stupid question, but why do we use MustRegister here and Register for "antrea_agent_local_pod_count"?

would like to know this as well :) what's the difference ?

The reason to the difference is that I used different sample code for both :)
MustRegister() will panic on failure while Register() returns an error.
I'm guessing that using Register() is the correct approach here unless you think otherwise.

Sounds good

antoninbas · 2020-04-28T00:11:12Z

pkg/agent/metrics/prometheus.go

+	ofClient openflow.Client) {
+
+	klog.Info("Initializing prometheus metrics")
+	if err := prometheus.Register(prometheus.NewGaugeFunc(


I remember there was a conversation about using the k8s wrapper objects instead of the Prometheus ones. Has this been abandoned or postponed to a future PR?

If postponed, please open an issue - would also be good to explain in the issue why we think the wrappers are superior for our use case

The k8s wrapper was proposed by @srikartati and is described in the this doc.
It offers stability tagging for metrics, e.g mark a metric as stable, alpha, deprecated etc, and offers separate endpoint (e.g /metrics/v1) for the stable metrics.
It would be nice to have as:
A) At some point we could need this functionality.
B) It's a part of k8s ecosystem

However at this point (A) isn't very relevant and Prometheus API definitions as within this patch are still widely common.
I would rather have Prometheus support and handle this in a latter time, when this is a priority.

Sounds good to me. Would still like to see a low-priority Github issue with a pointer to the doc.

See Issue #662

pkg/agent/metrics/prometheus.go

abhiraut · 2020-04-28T01:28:49Z

pkg/agent/metrics/prometheus.go

+		ConstLabels: prometheus.Labels{"k8s_nodename": nodeName, "k8s_podname": env.GetPodName()},
+	})
+	gaugeHost.Set(1)
+	prometheus.MustRegister(gaugeHost)


would like to know this as well :) what's the difference ?

abhiraut · 2020-04-28T01:31:31Z

pkg/agent/metrics/prometheus.go

+		klog.Errorf("Failed to retrieve agent K8S node name: %v", err)
+	}
+
+	gaugeHost := prometheus.NewGauge(prometheus.GaugeOpts{


for a newbie reading prometheus related code.. can you add a code comment explaining what NewGauge really does? if it is pretty common and basic then ignore my comment.. i will check it out in prometheus docs

Prometheus offers 4 metrics types: gauge, counter, histogram and summary.
Each can carry additional labels.
e.g, in our case, antrea_agent_ovs_flow_table will be labeled with the source agent,
the table id etc.

More info here.

thanks sounds good

abhiraut · 2020-04-28T01:31:55Z

pkg/agent/metrics/prometheus.go

+
+	gaugeHost := prometheus.NewGauge(prometheus.GaugeOpts{
+		Name:        "antrea_agent_runtime_info",
+		Help:        "Antrea agent runtime info , defined as a labels. The value of the gauge is always set to 1.",


Suggested change

Help: "Antrea agent runtime info , defined as a labels. The value of the gauge is always set to 1.",

Help: "Antrea agent runtime info , defined as labels. The value of the gauge is always set to 1.",

Antrea agent uses apiserver to interact with antctl. However, unlike the controller, the API endpoint is exposed only locally which limits antctl to local execution. Additionally, we would like to reuse the listener for Prometheus metrics, which would require external access to the API endpoint's metrics path. Having authentication in a similar way to the controller's implementation could help here as it will allow external exposure of the API endpoint.

antoninbas · 2020-04-28T17:52:48Z

pkg/agent/metrics/prometheus.go

+
+	ovsStats := newOVSStatManager(ovsBridge, ofClient)
+	if err := prometheus.Register(ovsStats); err != nil {
+		klog.Error("Failed to register antrea_agent_local_pod_count with Prometheus")


@ksamoray do you mean antrea_agent_ovs_flow_table here?

Indeed. Thanks

Integrate with Prometheus monitoring solution. Integration of the Prometheus client into Antrea controller and agent allows the exposure of various metrics to Prometheus server. In addition to Antrea's own set of metrics, Prometheus client will also expose metrics which are defined by various components which are part of the Antrea ecosystem, e.g golang, Prometheus itself etc.

ksamoray · 2020-04-28T19:22:17Z

/test-all

antoninbas

LGTM

antoninbas · 2020-04-28T21:05:46Z

/test-networkpolicy

Integrate with Prometheus monitoring solution. Integration of the Prometheus client into Antrea controller and agent allows the exposure of various metrics to Prometheus server. In addition to Antrea's own set of metrics, Prometheus client will also expose metrics which are defined by various components which are part of the Antrea ecosystem, e.g golang, Prometheus itself etc. See antrea-io#236

Integrate with Prometheus monitoring solution. Integration of the Prometheus client into Antrea controller and agent allows the exposure of various metrics to Prometheus server. In addition to Antrea's own set of metrics, Prometheus client will also expose metrics which are defined by various components which are part of the Antrea ecosystem, e.g golang, Prometheus itself etc. See #236

Integrate with Prometheus monitoring solution. Integration of the Prometheus client into Antrea controller and agent allows the exposure of various metrics to Prometheus server. In addition to Antrea's own set of metrics, Prometheus client will also expose metrics which are defined by various components which are part of the Antrea ecosystem, e.g golang, Prometheus itself etc. See antrea-io#236

vmwclabot added the cla-not-required label Jan 16, 2020

This was referenced Jan 17, 2020

Add prometheus metrics for antrea controller #325

Merged

Exporting prometheus metrics for OVS performance and routes #236

Closed

zyiou reviewed Feb 28, 2020

View reviewed changes

pkg/agent/metrics/prometheus.go Outdated Show resolved Hide resolved

zyiou mentioned this pull request Feb 28, 2020

Add unit test for Prometheus integration #446

Closed

abhiraut reviewed Mar 12, 2020

View reviewed changes

tnqn reviewed Mar 16, 2020

View reviewed changes

build/yamls/antrea.yml Outdated Show resolved Hide resolved

ksamoray force-pushed the prometheus-integration branch from a2fc0fd to e15a3e6 Compare March 22, 2020 14:39

ksamoray force-pushed the prometheus-integration branch from e15a3e6 to e401e3b Compare March 23, 2020 06:44

ksamoray changed the title ~~[wip] Antrea Prometheus integration~~ Antrea Prometheus integration Mar 23, 2020

jianjuns reviewed Mar 23, 2020

View reviewed changes

build/yamls/base/conf/antrea-agent.conf Outdated Show resolved Hide resolved

build/yamls/base/agent.yml Outdated Show resolved Hide resolved

cmd/antrea-controller/controller.go Outdated Show resolved Hide resolved

cmd/antrea-controller/controller.go Outdated Show resolved Hide resolved

ksamoray force-pushed the prometheus-integration branch from e401e3b to a51f9d7 Compare March 24, 2020 09:49

ksamoray linked an issue Mar 24, 2020 that may be closed by this pull request

Exporting prometheus metrics for OVS performance and routes #236

Closed

ksamoray self-assigned this Mar 24, 2020

srikartati reviewed Mar 24, 2020

View reviewed changes

ksamoray force-pushed the prometheus-integration branch from a51f9d7 to f62e560 Compare March 25, 2020 10:13

ksamoray force-pushed the prometheus-integration branch from f62e560 to 742e935 Compare March 25, 2020 10:55

tnqn reviewed Apr 24, 2020

View reviewed changes

ksamoray force-pushed the prometheus-integration branch from 0ab4ff6 to 6abee53 Compare April 25, 2020 06:23

tnqn previously approved these changes Apr 26, 2020

View reviewed changes

antoninbas reviewed Apr 28, 2020

View reviewed changes

abhiraut reviewed Apr 28, 2020

View reviewed changes

ksamoray dismissed tnqn’s stale review via 181d3ed April 28, 2020 15:40

ksamoray force-pushed the prometheus-integration branch from 6abee53 to 181d3ed Compare April 28, 2020 15:40

antoninbas reviewed Apr 28, 2020

View reviewed changes

ksamoray force-pushed the prometheus-integration branch from 181d3ed to 9845b1b Compare April 28, 2020 18:56

antoninbas approved these changes Apr 28, 2020

View reviewed changes

antoninbas added this to the Antrea v0.6.0 release milestone Apr 28, 2020

abhiraut approved these changes Apr 28, 2020

View reviewed changes

antoninbas merged commit ec17ec0 into antrea-io:master Apr 29, 2020

ksamoray deleted the prometheus-integration branch May 29, 2020 12:49

srikartati mentioned this pull request Jun 2, 2020

Remove function callbacks for tracking Antrea agent metrics #781

Merged

	Help: "Antrea agent runtime info , defined as a labels. The value of the gauge is always set to 1.",
	Help: "Antrea agent runtime info , defined as labels. The value of the gauge is always set to 1.",

Antrea Prometheus integration #321

Antrea Prometheus integration #321

Conversation

ksamoray commented Jan 16, 2020 • edited Loading

antrea-bot commented Jan 16, 2020

ksamoray commented Jan 16, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhiraut commented Mar 12, 2020

ksamoray commented Mar 22, 2020

ksamoray commented Mar 22, 2020

ksamoray commented Mar 23, 2020

edwardbadboy commented Mar 23, 2020

ksamoray commented Mar 23, 2020

ksamoray commented Mar 23, 2020

ksamoray commented Mar 24, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksamoray commented Mar 25, 2020

ksamoray commented Mar 25, 2020

ksamoray commented Mar 25, 2020

ksamoray commented Apr 23, 2020

tnqn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksamoray commented Apr 24, 2020

jianjuns commented Apr 24, 2020

ksamoray commented Apr 24, 2020

tnqn left a comment

Choose a reason for hiding this comment

ksamoray commented Apr 26, 2020

tnqn commented Apr 27, 2020

tnqn commented Apr 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ksamoray commented Apr 28, 2020

antoninbas left a comment

Choose a reason for hiding this comment

antoninbas commented Apr 28, 2020

ksamoray commented Jan 16, 2020 •

edited

Loading