Document Antrea component metrics #726

ksamoray · 2020-05-21T07:14:06Z

Add the list of metrics which are exposed by Antrea agent and controller
in the integration doc for reference.
The metric list and descriptions added were pulled out of the metrics help strings from the code.

antrea-bot · 2020-05-21T07:14:18Z

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-05-21T13:41:14Z

/test-all

srikartati · 2020-05-22T19:24:14Z

docs/prometheus-integration.md

+by the Antrea components.
+
+Below is a list of metrics, provided by the components and by 3rd parties.
+


Hi Kobi, These help strings are visible through API right? Are these not visible from Prometheus server/Grafana? In addition, some of them are standard metrics and not Antrea specific.
Overall question: Is it useful to add these in Antrea documentation?

@srikartati Hi,
You're correct: you could obtain that info via API and they're visible via Prometheus/Grafana.
However, from a user/admin perspective, there's no difference between Antrea and 3rd party metrics: they're all used for monitoring the Antrea solution. When a certain metric is required for monitoring nobody cares if it was provided by the Antrea codebase, Go, or something else. And it's much easier to look these up within a doc than browsing through listboxes in Prometheus.
Listing metrics within a document is very common, and as there were multiple questions regarding supported metrics, I think that it's a good idea to have such doc.

@ksamoray I see a couple of issues:

Every time a metric is added. This has to be continuously extended. This document may blow up in content and size.

I feel most of the metrics "help" field is almost similar to what name is offering and not offering anything beyond.

I think the questions on slack came up because of missing network policy documentation, which will be added in the near future.
IMO, a statement about looking for "help" field through API/listboxes in Prometheus is probably good enough. I will leave it you and others to decide.

Hi @srikartati,
Yeah that is correct. Documentation has to be maintained - and maybe organized differently as here or here but that doesn't make it less needed.
About (2): that only implies that our "help" requires more attention :)
Can you imagine, when we will hopefully have a whole lot of metrics, that a user will have to scroll a listbox through the help text of hundreds of metrics, just to find how the CPU utilization metric is called? That is not useful at all IMO.
I think that the fact that every product in the market offers such doc speaks for itself...

Thanks for the links. They may also have the additional purpose of showcasing the metrics they are offering :)
Do you think putting metrics in tables is a better representation?
I haven't tried this representation myself but the example instruction seems not so difficult: https://help.github.com/en/github/writing-on-github/organizing-information-with-tables

@srikartati tried a table... tables look awkward in a markdown doc due to the 80 char limit.
Maybe if we ever move this info into the website :)

Weird that multiple lines in the description column will look awkward. Table format would have been nice!
NIT: I looked at the rich diff. Bullet points can be removed as metrics are anyway in bold.

Some of the metrics names are long, e.g antrea_controller_network_policy_sync_duration_milliseconds
which doesn't leave much room for description under 80 char limit...
I'll remove the bullets.

ksamoray · 2020-05-24T11:52:00Z

Also removed a section from the doc which is redundant (ClusterRole has been removed from the yml)

antrea-bot · 2020-05-28T11:47:27Z

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-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance 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-05-29T05:04:12Z

/test-all

ksamoray · 2020-05-29T05:22:22Z

/test-e2e

ksamoray · 2020-05-31T08:04:12Z

/test-e2e

ksamoray · 2020-06-01T17:44:46Z

/test-all

antrea-bot · 2020-07-02T11:24: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-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

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

ksamoray · 2020-07-02T11:31:47Z

@antoninbas @jianjuns @McCodeman
I have updated to the current metrics which are exposed by the agent and controller.
Is it worthwhile to automate the creation of this doc section so it'll be easier to refresh it whenever needed?

jianjuns · 2020-07-02T21:26:24Z

@antoninbas @jianjuns @McCodeman
I have updated to the current metrics which are exposed by the agent and controller.
Is it worthwhile to automate the creation of this doc section so it'll be easier to refresh it whenever needed?

Thanks @ksamoray! Yes it would be ideal to automate generate this doc. Do you already have an idea?

jianjuns · 2020-07-02T22:28:12Z

docs/prometheus-integration.md

+The value of the gauge is always set to 1.
+
+## Common Metrics Provided by Infrastructure
+### Apiserver Metrics


Was chatting with @antoninbas on this PR. He has a good point it is even harder to maintain 3rd party metrics here. If we can not point to any links for these metrics, we should consider auto-generating the list.
At least we can mention here, the list can change as the 3rd party components versions change.

Hi @jianjuns, this list has been mostly autogenerated :)
I can complete it with a few things and commit the generation script into the repo (not sure where this belong though)

hack/

There should also be a Github CI job to make sure that the list is up-to-date.

antrea-bot · 2020-07-29T11:26:56Z

Can one of the admins verify this patch?

antrea-bot · 2020-08-04T10:33:16Z

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-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

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

antoninbas · 2020-09-26T01:42:05Z

@ksamoray could we revive this PR? We need a way to make sure that the doc is always up-to-date.

antrea-bot · 2020-09-26T15:34:45Z

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-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

ksamoray · 2020-09-26T15:38:14Z

@ksamoray could we revive this PR? We need a way to make sure that the doc is always up-to-date.

@antoninbas I've rebased the patch. I do need to add some test to validate that metrics are still valid.
I'm not sure how I could implement this as the test will need access to the document: in the make script I worked around this by adding it as a parameter, and I'm not sure that I like it.
Maybe you have a suggestion?

codecov-commenter · 2020-09-26T15:38:50Z

Codecov Report

Merging #726 into master will decrease coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #726      +/-   ##
==========================================
- Coverage   64.63%   64.48%   -0.16%     
==========================================
  Files         157      159       +2     
  Lines       12626    12657      +31     
==========================================
+ Hits         8161     8162       +1     
- Misses       3620     3644      +24     
- Partials      845      851       +6

Flag	Coverage Δ
#integration-tests	`44.83% <ø> (ø)`
#kind-e2e-tests	`50.55% <ø> (-0.26%)`	⬇️
#unit-tests	`42.14% <ø> (-0.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/controller/networkpolicy/tier.go	`90.00% <0.00%> (-10.00%)`	⬇️
pkg/apiserver/certificate/certificate.go	`71.05% <0.00%> (-6.58%)`	⬇️
pkg/monitor/controller.go	`48.52% <0.00%> (-2.95%)`	⬇️
pkg/antctl/command_list.go	`31.48% <0.00%> (-1.22%)`	⬇️
pkg/antctl/command_definition.go	`41.96% <0.00%> (-0.44%)`	⬇️
pkg/antctl/antctl.go	`100.00% <0.00%> (ø)`
pkg/apiserver/handlers/loglevel/handler.go	`38.46% <0.00%> (ø)`
pkg/log/log_level.go	`10.00% <0.00%> (ø)`
pkg/apiserver/apiserver.go	`84.33% <0.00%> (+0.19%)`	⬆️
pkg/agent/apiserver/apiserver.go	`66.12% <0.00%> (+0.55%)`	⬆️
... and 1 more

ksamoray · 2020-09-27T12:58:53Z

OK added some validation for the doc content, LMK what you think.

antoninbas

Regarding your earlier question, I personally would recommend a "self-contained" script that takes care of creating a Kind cluster (using kind-setup.sh), deploys Antrea with metrics enabled and gets the list of metrics. I like it better than integrating the docs verification in the e2e tests and using an environment variable to access the file.

I understand that actually running Antrea in a cluster is pretty much the only way to determine the list of metrics. I hope that moving forward we won't run into issues where we have Antrea metrics which are platform specific.

antoninbas · 2020-09-28T21:43:27Z

docs/prometheus-integration.md

+Antrea Controller and Agents expose various metrics, some are provided by the 
+Antrea components and others are provided by 3rd party components which are used
+by the Antrea components.


Suggested change

Antrea Controller and Agents expose various metrics, some are provided by the

Antrea components and others are provided by 3rd party components which are used

by the Antrea components.

Antrea Controller and Agents expose various metrics, some of which are provided by the

Antrea components and others which are provided by 3rd party components used

by the Antrea components.

antoninbas · 2020-09-28T21:45:15Z

hack/make_metricsdoc.sh

@@ -0,0 +1,86 @@
+#!/bin/bash


nits:

please consider this shebang instead: #!/usr/bin/env bash

other scripts in the hack folder tend to use dashes in their name instead of underscores

antoninbas · 2020-09-28T21:47:56Z

hack/make_metricsdoc.sh

+	pod_yaml=$(kubectl get pod -n kube-system $pod_name -o yaml)
+	host_ip=$(awk '/[[:space:]]hostIP:/{print $2}' <<< "$pod_yaml")
+	host_port=$(awk '/[[:space:]]hostPort:/{print $2}' <<< "$pod_yaml")


kubectl supports:

kubectl get pod -n kube-system $pod_name --template={{.status.hostIP}}

which is nicer than using awk from a readability / maintainability perspective IMO. Same for hostPort.

Will use -o jsonpath. Would like to use this for the token/cert retrieval, and --template doesn't handle ca.crt very well (dot in the key).
So I'll use the same flag for all kubectl calls.

antoninbas

some pretty minor comments
approach looks good to me now, I am pleased to see it only takes 3 minutes to update the doc!

antoninbas · 2020-10-01T21:39:18Z

ci/kind/test-metrics-doc-kind.sh

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# The script runs kind e2e tests with different traffic encapsulation modes.


comment inaccurate

antoninbas · 2020-10-01T21:43:02Z

ci/kind/test-metrics-doc-kind.sh

+MAKE_CMD=$(dirname $0)"/../../hack/make-metrics-doc.sh"
+METRICS_DOC=$(dirname $0)"/../../docs/prometheus-integration.md"


I recommend THIS_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )" instead of dirname $0, which IIRC does not work well in some scenarios.

OK. $(dirname $0) is used in other places as well, maybe worths changing as well.

antoninbas · 2020-10-01T21:44:39Z

ci/kind/test-metrics-doc-kind.sh

+cmp -s $METRICS_DOC $METRICS_TMP_DOC
+result=$?
+if [ $result -ne 0 ]; then
+    echo "Error: Prometheus metrics document should be updated"


This message will be displayed by CI in case of failure and I think we should make it more useful for users. For example:

echo "Error: Prometheus metrics document should be updated" echo "You can update it by building the Antrea Docker image locally (with `make`), running ./hack/make-metrics-doc.sh and committing the changes"

antoninbas · 2020-10-01T21:45:12Z

ci/kind/test-metrics-doc-kind.sh

@@ -0,0 +1,45 @@
+#!/usr/bin/env bash


maybe "validate-metrics-doc.sh" for the script name?

antoninbas · 2020-10-01T21:45:36Z

.github/workflows/kind_upgrade.yml

@@ -165,3 +165,34 @@ jobs:
      - name: Run test
        run: |
          ./ci/kind/test-upgrade-antrea.sh --from-version-n-minus 2 --controller-only
+
+  test-prometheus-metrics-doc:


maybe "validate-prometheus-metrics-doc"?

hack/make-metrics-doc.sh

antoninbas · 2020-10-01T21:49:13Z

docs/prometheus-integration.md

@@ -116,3 +116,165 @@ The configuration file above can be used to deploy Prometheus Server with
 scraping configuration for Antrea services.
 To deploy this configuration use
 `kubectl apply -f build/yamls/antrea-prometheus.yml`
+
+# Antrea Prometheus Metrics


I don't know if this is the ideal place, but the doc should mention how devs who add metrics can re-generate the list.

This doc is very user-oriented, while doc generation is targeted to developers.
As we have a bunch of other generatables, maybe it's worthwhile to create a dev-oriented code enlisting all of these (e.g yamls via make manifest, testing mocks etc). I can create an issue for this and then handle it separately.
BTW do we need a make target for the metrics generation as well?

We do have a doc that may work already: https://github.com/vmware-tanzu/antrea/blob/master/docs/code-generation.md, although the title may need to be updated

I don't think a make target is needed right now, but we can add one in the future

.github/workflows/kind_upgrade.yml

docs/prometheus-integration.md

antoninbas

LGTM

antoninbas · 2020-10-05T18:30:58Z

ci/kind/validate-metrics-doc.sh

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# The script validated the Prometheus metrics list within docs/prometheus-integration.md againt a Kind cluster.


s/validated/validates

antoninbas · 2020-10-05T18:31:36Z

docs/prometheus-integration.md

@@ -116,3 +116,165 @@ The configuration file above can be used to deploy Prometheus Server with
 scraping configuration for Antrea services.
 To deploy this configuration use
 `kubectl apply -f build/yamls/antrea-prometheus.yml`
+
+# Antrea Prometheus Metrics


We do have a doc that may work already: https://github.com/vmware-tanzu/antrea/blob/master/docs/code-generation.md, although the title may need to be updated

I don't think a make target is needed right now, but we can add one in the future

Add the list of metrics which are exposed by Antrea agent and controller in the integration doc for reference.

antoninbas

LGTM

antoninbas · 2020-10-06T17:48:16Z

/skip-all

ckfwer

MMH GmbH

vmwclabot added the cla-not-required label May 21, 2020

ksamoray linked an issue May 21, 2020 that may be closed by this pull request

Document Antrea Prometheus metrics #719

Closed

ksamoray force-pushed the prometheus-metric-doc branch from 0212cba to 5d1fb09 Compare May 22, 2020 17:45

srikartati reviewed May 22, 2020

View reviewed changes

ksamoray force-pushed the prometheus-metric-doc branch from 5d1fb09 to 635657c Compare May 24, 2020 11:50

ksamoray force-pushed the prometheus-metric-doc branch from 635657c to 8c5ce3a Compare May 28, 2020 11:47

srikartati previously approved these changes May 28, 2020

View reviewed changes

ksamoray dismissed srikartati’s stale review via 4a7264e June 1, 2020 17:16

ksamoray force-pushed the prometheus-metric-doc branch from 8c5ce3a to 4a7264e Compare June 1, 2020 17:16

ksamoray force-pushed the prometheus-metric-doc branch from 4a7264e to 28327fc Compare July 2, 2020 11:24

ksamoray force-pushed the prometheus-metric-doc branch from 28327fc to 83cb311 Compare July 2, 2020 11:29

jianjuns previously approved these changes Jul 2, 2020

View reviewed changes

jianjuns reviewed Jul 2, 2020

View reviewed changes

ksamoray dismissed jianjuns’s stale review via 45589a8 July 13, 2020 09:35

ksamoray force-pushed the prometheus-metric-doc branch from 83cb311 to 45589a8 Compare July 13, 2020 09:35

ksamoray force-pushed the prometheus-metric-doc branch from 45589a8 to 0b67800 Compare August 4, 2020 10:33

ksamoray force-pushed the prometheus-metric-doc branch from 0b67800 to 5205d86 Compare September 26, 2020 15:34

ksamoray force-pushed the prometheus-metric-doc branch from 5205d86 to acba009 Compare September 27, 2020 12:55

ksamoray force-pushed the prometheus-metric-doc branch from acba009 to bc67564 Compare September 27, 2020 13:19

antoninbas reviewed Sep 28, 2020

View reviewed changes

ksamoray force-pushed the prometheus-metric-doc branch 5 times, most recently from fd55e3a to 25977de Compare October 1, 2020 16:34

ksamoray requested a review from antoninbas October 1, 2020 17:16

antoninbas reviewed Oct 1, 2020

View reviewed changes

ksamoray force-pushed the prometheus-metric-doc branch 3 times, most recently from b8cbc85 to 76bd3b9 Compare October 4, 2020 07:34

antoninbas previously approved these changes Oct 5, 2020

View reviewed changes

ksamoray dismissed antoninbas’s stale review via 4d68225 October 6, 2020 10:16

ksamoray force-pushed the prometheus-metric-doc branch 2 times, most recently from 4d68225 to 5da161c Compare October 6, 2020 10:18

Document Antrea component metrics

da52dd8

Add the list of metrics which are exposed by Antrea agent and controller in the integration doc for reference.

ksamoray force-pushed the prometheus-metric-doc branch from 5da161c to da52dd8 Compare October 6, 2020 13:25

antoninbas approved these changes Oct 6, 2020

View reviewed changes

ksamoray merged commit fe6e7f6 into antrea-io:master Oct 7, 2020

ksamoray deleted the prometheus-metric-doc branch November 16, 2020 14:42

ckfwer approved these changes May 28, 2021

View reviewed changes

		by the Antrea components.

		Below is a list of metrics, provided by the components and by 3rd parties.

		MAKE_CMD=$(dirname $0)"/../../hack/make-metrics-doc.sh"
		METRICS_DOC=$(dirname $0)"/../../docs/prometheus-integration.md"

Document Antrea component metrics #726

Document Antrea component metrics #726

Conversation

ksamoray commented May 21, 2020

antrea-bot commented May 21, 2020

ksamoray commented May 21, 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

ksamoray commented May 24, 2020

antrea-bot commented May 28, 2020

ksamoray commented May 29, 2020

ksamoray commented May 29, 2020

ksamoray commented May 31, 2020

ksamoray commented Jun 1, 2020

antrea-bot commented Jul 2, 2020

ksamoray commented Jul 2, 2020

jianjuns commented Jul 2, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antrea-bot commented Jul 29, 2020

antrea-bot commented Aug 4, 2020

antoninbas commented Sep 26, 2020

antrea-bot commented Sep 26, 2020

ksamoray commented Sep 26, 2020

codecov-commenter commented Sep 26, 2020 • edited Loading

Codecov Report

ksamoray commented Sep 27, 2020

antoninbas 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

antoninbas 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

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

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antoninbas left a comment

Choose a reason for hiding this comment

antoninbas commented Oct 6, 2020

ckfwer left a comment

Choose a reason for hiding this comment

codecov-commenter commented Sep 26, 2020 •

edited

Loading