Implement Prometheus Metrics #4

viivek46 · 2019-10-20T17:34:52Z

What changes are proposed in this PR?

This PR Implement Prometheus Metrics for kudo Cassandra operator. we have used criteo cassandra_exporter to export Cassandra metrics.

To implement this below decisions are made :

created different container in same pod to run metrics, and added related parameters.
added config file cassandra-exporter-config-yml and downloaded jar for criteo cassandra exporter.
Added label kudo.dev/servicemonitor: "true" and created service-monitor.yaml file to create Service Monitor object for service discovery.

How were these changes tested?

Manually tested and Below are steps to check

Spin up Konvoy cluster and ensure cluster config is applied for kubectl.
Install KUDO Cassandra operator
Run command to create service-monito: kubectl create -f resources/service-monitor.yaml
Wait 10-20 sec, and go to Prometheus dashboard. Type there Cassandra you will see "cassandra_stats".

…a-operator into metrics

viivek46 · 2019-10-21T21:30:25Z

Hi @mpereira / @samvantran,
Few things that I need suggestion on :

Metrics CPU/memory and METRICS PORT - what I should set default values.
I copied cassandra-exporter-config-yml.yaml from criteo, should we go ahead with this or any things which we should remove from blacklist, also i am not sure how we can provide the customer option to enter blacklist values.

currently i am using curl to get cassandra_exporter jar from git, but once all things are ok i will upload this jar to http://downloads.mesosphere.com.

samvantran

Thanks @viivek46 ! I had a couple questions and few typo fixes but rest looks good. Great to have metrics exported. It would be great to add docs on what users need to do/set and where to go in order to view metrics. Similar to this

operator/operator.yaml

operator/params.yaml

operator/templates/cassandra-exporter-config-yml.yaml

samvantran · 2019-10-21T23:25:49Z

operator/templates/stateful-set.yaml

@@ -46,7 +46,7 @@ spec:
          imagePullPolicy: Always
          # Dockerfile based on:
          # https://github.com/docker-library/cassandra/blob/master/3.11/Dockerfile
-          image: mpereira/cassandra:3.11.4
+          image: viivek46/cassandra:3.11.4


hm, can we add tooling to use non-personal images? and can we add additional versioning so that we don't overwrite each other's images?

We don't need to use the Cassandra image, right?

Here's what I propose in terms of KUDO Cassandra depending on the cassandra_exporter repository:

Add cassandra_exporter it as a git submodule under vendor/cassandra_exporter

Create a symlink from images/Dockerfile.prometheus-exporter to vendor/cassandra_exporter/docker/Dockerfile

Build/push mesosphere/cassandra_exporter Docker image based on images/Dockerfile.prometheus-exporter (can be personal for now until I finish implementing Docker image build support) and use that for the prometheus exporter container.

Revert the changes in the Cassandra node Dockerfile and pod.

This sounds kind of involved - why don't we make this a separate ticket for now so that we don't block the metrics work. If we're okay with personal images for now, let's continue using them until the Docker image build support is in place.

Yeah, step 3 above calls out that the image can be personal.

Please revert line 49 for now @viivek46.

I have modified exporter docker file, as cassandra_exporter.jar file was not accessible by non root user, and i skipped step 1,2 from Murilo's suggestion and copy paste run.sh and docker file from crito's repo and modify them in our repo.

operator/templates/stateful-set.yaml

operator/templates/cassandra-exporter-config-yml.yaml

operator/params.yaml

Co-Authored-By: Sam Tran <[email protected]>

operator/operator.yaml

operator/params.yaml

operator/templates/cassandra-exporter-config-yml.yaml

mpereira · 2019-10-22T13:18:19Z

operator/templates/stateful-set.yaml

@@ -46,7 +46,7 @@ spec:
          imagePullPolicy: Always
          # Dockerfile based on:
          # https://github.com/docker-library/cassandra/blob/master/3.11/Dockerfile
-          image: mpereira/cassandra:3.11.4
+          image: viivek46/cassandra:3.11.4


We don't need to use the Cassandra image, right?

Here's what I propose in terms of KUDO Cassandra depending on the cassandra_exporter repository:

Add cassandra_exporter it as a git submodule under vendor/cassandra_exporter

Create a symlink from images/Dockerfile.prometheus-exporter to vendor/cassandra_exporter/docker/Dockerfile

Build/push mesosphere/cassandra_exporter Docker image based on images/Dockerfile.prometheus-exporter (can be personal for now until I finish implementing Docker image build support) and use that for the prometheus exporter container.

Revert the changes in the Cassandra node Dockerfile and pod.

operator/templates/stateful-set.yaml

resources/service-monitor.yaml

mpereira · 2019-10-22T13:37:36Z

Hey @viivek46, thanks for your initial work! Changes mostly look good, I left a few comments regarding some changes.

Also, we'll probably want an integration test to make sure this actually works. Maybe we can work on it together? Since I'm still wrapping up #3.

A useful sanity integration test could be similar as the one we have for the DC/OS Cassandra service: https://github.com/mesosphere/dcos-commons/blob/1809d70db5acd6aa8620a82c6a786e3a25d63f89/frameworks/cassandra/tests/test_sanity.py#L85-L104

mpereira · 2019-10-22T13:48:21Z

Also, I think it's ok that for this PR to not make cassandra_exporter totally configurable through params.yaml. I'll create a separate ticket for that.

mpereira · 2019-10-22T16:11:23Z

Created https://jira.mesosphere.com/browse/DCOS-60293 for the above.

images/Dockerfile

operator/templates/service.yaml

mpereira · 2019-10-24T13:14:27Z

operator/templates/stateful-set.yaml

@@ -46,7 +46,7 @@ spec:
          imagePullPolicy: Always
          # Dockerfile based on:
          # https://github.com/docker-library/cassandra/blob/master/3.11/Dockerfile
-          image: mpereira/cassandra:3.11.4
+          image: viivek46/cassandra:3.11.4


Please revert line 49 for now @viivek46.

docs/README.md

mpereira · 2019-10-24T13:18:54Z

docs/monitoring.md

+
+```
+kubectl create -f resources/service-monitor.yaml
+```


We should add instructions to how to import the Grafana dashboard. And probably remove what's currently written, since the service monitor will be installed by default.

Correct with new changes which include automation I will update this instructions, i just pushed code as "wip" as we were discussing about cassandra_exporter image issue.
Currently I am just updating doc, about adding instructions Grafana dashboard i still need to work out that, as provided json in cassandra_export was not working on my first test.

Co-Authored-By: Murilo Pereira <[email protected]>

* Unify handling of Docker images. * Build Prometheus exporter Docker image too. * Set both Docker image namespaces to "mesosphere". * Make Prometheus exporter image version composite as well. * Change Prometheus exporter Docker image name. * Snapshot Docker image versions. * Improve documentation. Remove KUDO-specific details. * Add automatic formatting for Markdown.

mpereira

Thanks @viivek46!

porridge · 2020-02-05T10:36:51Z

images/Dockerfile.prometheus-exporter

@@ -0,0 +1,3 @@
+FROM criteord/cassandra_exporter:2.2.1
+
+RUN chown 999:999 /opt/cassandra_exporter/cassandra_exporter.jar


@viivek46 can you please explain why we need to chown the file? What runs as UID 999 and why does it need to own this JAR?

@porridge: can you please explain why we need to chown the file? What runs as UID 999 and why does it need to own this JAR?

Because of the pod security context this image container runs under.

Thanks @mpereira . However chown-ing the file gives the process the permission to modify the code to be run, which is frowned upon from security PoV.
I'm guessing the intent probably to chmod a+r it...

-rw-rwx--- 1 root root 4607224 Jan 30 2019 cassandra_exporter.jar ``` Right. So the fix should actually be done upstream, let me take a stab at it.

criteo/cassandra_exporter#68

The permissions are now [fixed](criteo/cassandra_exporter#68) in upstream repo, so we can drop [the chown](#4 (review)): ``` porridge@beczulka:~$ docker run criteord/cassandra_exporter:2.3.2 ls -l /opt/cassandra_exporter Unable to find image 'criteord/cassandra_exporter:2.3.2' locally 2.3.2: Pulling from criteord/cassandra_exporter bc51dd8edc1b: Pull complete d1d06863bb82: Pull complete adf4d10a782b: Pull complete 7db4424ff270: Pull complete e008a301333c: Pull complete 10121b885a52: Pull complete 0d34188e8091: Pull complete fc3c7604e1c5: Pull complete 019737f36353: Pull complete a26e5d6f647b: Pull complete 3b6b59aecfbd: Pull complete Digest: sha256:9fe7cfef9b03ca294c80e5da098add4698c4103bbddf382dd93d2e01a073d6e3 Status: Downloaded newer image for criteord/cassandra_exporter:2.3.2 total 4760 -rw-rw-r-- 1 root root 4871483 Feb 10 13:31 cassandra_exporter.jar porridge@beczulka:~$ ```

The permissions are now [fixed](criteo/cassandra_exporter#68) in upstream repo, so we can drop [the chown](#4 (review)): ``` porridge@beczulka:~$ docker run criteord/cassandra_exporter:2.3.2 ls -l /opt/cassandra_exporter Unable to find image 'criteord/cassandra_exporter:2.3.2' locally 2.3.2: Pulling from criteord/cassandra_exporter bc51dd8edc1b: Pull complete d1d06863bb82: Pull complete adf4d10a782b: Pull complete 7db4424ff270: Pull complete e008a301333c: Pull complete 10121b885a52: Pull complete 0d34188e8091: Pull complete fc3c7604e1c5: Pull complete 019737f36353: Pull complete a26e5d6f647b: Pull complete 3b6b59aecfbd: Pull complete Digest: sha256:9fe7cfef9b03ca294c80e5da098add4698c4103bbddf382dd93d2e01a073d6e3 Status: Downloaded newer image for criteord/cassandra_exporter:2.3.2 total 4760 -rw-rw-r-- 1 root root 4871483 Feb 10 13:31 cassandra_exporter.jar porridge@beczulka:~$ ``` We actually bump to 2.3.3 since the earlier one was non-functional.

The permissions are now [fixed](criteo/cassandra_exporter#68) in upstream repo, so we can drop [the chown](#4 (review)): ``` porridge@beczulka:~$ docker run criteord/cassandra_exporter:2.3.2 ls -l /opt/cassandra_exporter Unable to find image 'criteord/cassandra_exporter:2.3.2' locally 2.3.2: Pulling from criteord/cassandra_exporter bc51dd8edc1b: Pull complete d1d06863bb82: Pull complete adf4d10a782b: Pull complete 7db4424ff270: Pull complete e008a301333c: Pull complete 10121b885a52: Pull complete 0d34188e8091: Pull complete fc3c7604e1c5: Pull complete 019737f36353: Pull complete a26e5d6f647b: Pull complete 3b6b59aecfbd: Pull complete Digest: sha256:9fe7cfef9b03ca294c80e5da098add4698c4103bbddf382dd93d2e01a073d6e3 Status: Downloaded newer image for criteord/cassandra_exporter:2.3.2 total 4760 -rw-rw-r-- 1 root root 4871483 Feb 10 13:31 cassandra_exporter.jar porridge@beczulka:~$ ``` We actually bump to 2.3.3 since the earlier one was non-functional. Also rename a parameter in cassandra config as the old one is not recognized.

The permissions are now [fixed](criteo/cassandra_exporter#68) in upstream repo, so we can drop [the chown](#4 (review)): ``` porridge@beczulka:~$ docker run criteord/cassandra_exporter:2.3.2 ls -l /opt/cassandra_exporter Unable to find image 'criteord/cassandra_exporter:2.3.2' locally 2.3.2: Pulling from criteord/cassandra_exporter bc51dd8edc1b: Pull complete d1d06863bb82: Pull complete adf4d10a782b: Pull complete 7db4424ff270: Pull complete e008a301333c: Pull complete 10121b885a52: Pull complete 0d34188e8091: Pull complete fc3c7604e1c5: Pull complete 019737f36353: Pull complete a26e5d6f647b: Pull complete 3b6b59aecfbd: Pull complete Digest: sha256:9fe7cfef9b03ca294c80e5da098add4698c4103bbddf382dd93d2e01a073d6e3 Status: Downloaded newer image for criteord/cassandra_exporter:2.3.2 total 4760 -rw-rw-r-- 1 root root 4871483 Feb 10 13:31 cassandra_exporter.jar porridge@beczulka:~$ ``` We actually bump to 2.3.4 since there were other problems with 2.3.2 and 2.3.3.

viivek46 and others added 10 commits October 17, 2019 22:43

wip

ad99b97

Update params.yaml

091ef35

wip

0e9721a

Update params.yaml

6a723a4

Merge branch 'metrics' of https://github.com/mesosphere/kudo-cassandr…

b4f9168

…a-operator into metrics

wip

98da462

wip

84f1dc4

Implement Prometheus Metrics

573adbb

cleanup and added conditions

3be20ce

Update stateful-set.yaml

9e8b6b3

viivek46 changed the title ~~Metrics~~ Implement Prometheus Metrics Oct 21, 2019

viivek46 requested review from samvantran and mpereira October 21, 2019 21:21

samvantran suggested changes Oct 21, 2019

View reviewed changes

viivek46 and others added 2 commits October 21, 2019 22:06

Apply suggestions from code review

5bc636c

Co-Authored-By: Sam Tran <[email protected]>

updated param description for metrics's cpu/memory

8bc3cba

mpereira suggested changes Oct 22, 2019

View reviewed changes

viivek46 and others added 2 commits October 22, 2019 19:26

changed Param for metrics and added monitoring docs

5de2cee

revert matchNames to default

80f3599

viivek46 requested review from mpereira and samvantran October 22, 2019 23:19

mpereira reviewed Oct 23, 2019

View reviewed changes

images/Dockerfile Outdated Show resolved Hide resolved

wip

77e0d5e

mpereira suggested changes Oct 24, 2019

View reviewed changes

mpereira added the wip label Oct 24, 2019

changed image for prometheus + suggestions from review

faa378e

viivek46 force-pushed the metrics branch from 4c93f0f to faa378e Compare October 25, 2019 03:37

Update docs/README.md

43969e7

Co-Authored-By: Murilo Pereira <[email protected]>

viivek46 requested a review from mpereira October 28, 2019 15:55

viivek46 and others added 8 commits October 28, 2019 17:01

updated docekr file

be26e54

Merge remote-tracking branch 'upstream/master' into metrics

67b5db7

Merge branch 'master' into metrics

c5905fc

fix wrong conflic merge

b83db22

Update simple_install_uninstall_test.go

daa10a9

Update simple_install_uninstall_test.go

d3a2cb4

Upgrade KUDO to 0.7.5.

0abceba

viivek46 added ready for review and removed wip labels Oct 30, 2019

mpereira approved these changes Oct 30, 2019

View reviewed changes

mpereira merged commit e3ec1e2 into master Oct 30, 2019

mpereira deleted the metrics branch October 30, 2019 17:14

porridge reviewed Feb 5, 2020

View reviewed changes

porridge mentioned this pull request Feb 19, 2020

Update to latest release, drop the chown #56

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Prometheus Metrics #4

Implement Prometheus Metrics #4

viivek46 commented Oct 20, 2019 •

edited

Loading

viivek46 commented Oct 21, 2019

samvantran left a comment

samvantran Oct 21, 2019

mpereira Oct 22, 2019

samvantran Oct 22, 2019

mpereira Oct 23, 2019

mpereira Oct 24, 2019

viivek46 Oct 25, 2019 •

edited

Loading

mpereira Oct 22, 2019

mpereira commented Oct 22, 2019 •

edited

Loading

mpereira commented Oct 22, 2019

mpereira commented Oct 22, 2019

mpereira Oct 24, 2019

mpereira Oct 24, 2019

viivek46 Oct 25, 2019 •

edited

Loading

mpereira left a comment

porridge Feb 5, 2020

mpereira Feb 5, 2020

porridge Feb 5, 2020

porridge Feb 5, 2020

		@@ -0,0 +1,3 @@
		FROM criteord/cassandra_exporter:2.2.1

		RUN chown 999:999 /opt/cassandra_exporter/cassandra_exporter.jar

Implement Prometheus Metrics #4

Implement Prometheus Metrics #4

Conversation

viivek46 commented Oct 20, 2019 • edited Loading

What changes are proposed in this PR?

How were these changes tested?

viivek46 commented Oct 21, 2019

samvantran 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

viivek46 Oct 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpereira commented Oct 22, 2019 • edited Loading

mpereira commented Oct 22, 2019

mpereira commented Oct 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

viivek46 Oct 25, 2019 • edited Loading

Choose a reason for hiding this comment

mpereira 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

viivek46 commented Oct 20, 2019 •

edited

Loading

viivek46 Oct 25, 2019 •

edited

Loading

mpereira commented Oct 22, 2019 •

edited

Loading

viivek46 Oct 25, 2019 •

edited

Loading