added x-metrics pod monitor #8

humoflife · 2023-10-31T04:38:42Z

Description of your changes

Added x-metrics pod monitor. Please review.

Fixes #

I have:

Read and followed Upbound's contribution process.
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR, as appropriate.

How has this code been tested

… namespace selectors for crossplane-system and upbound-system Signed-off-by: Christopher Haar <[email protected]>

haarchri · 2023-11-01T12:14:11Z

@humoflife you missing the usage part
https://github.com/upbound/configuration-observability-oss/blob/main/apis/oss/composition.yaml#L177-L193

feat(podmonitors): change pod monitors

moved patch for deterministic dashboard creation

xmetrics dashboard checkpoint checkin

humoflife · 2023-11-07T04:28:31Z

Adding the usage did not let prometheus display the target. Still exploring other possible causes for the x-metrics podMonitor not showing up.

…ility-oss into x-metrics

humoflife · 2023-11-08T00:02:05Z

/test-examples

humoflife · 2023-11-08T00:13:55Z

/test-examples

humoflife · 2023-11-08T00:57:16Z

/test-examples

humoflife · 2023-11-08T01:14:57Z

/test-examples

haarchri · 2023-11-08T03:56:05Z

Can you rebase this branch ?

…ility-oss into x-metrics

humoflife · 2023-11-14T00:41:18Z

/test-examples

candonov · 2023-11-14T20:35:42Z

apis/oss/composition.yaml

+    - name: podMonitorXmetrics
+      base:
+        apiVersion: kubernetes.crossplane.io/v1alpha1
+        kind: Object
+        metadata:
+          labels: 
+            type: podMonitorXmetrics


X-metrics installs with a service, when a service exists, it is better to use a service monitor for Prometheus. The helm chart already supports service monitor, use helm values to enable it.

candonov · 2023-11-15T00:11:38Z

README.md

+**Warning**
+## Disclaimer: Management Cluster Resource Use
+Prometheus and Grafana may require significant cluster resources in relation
+to the amount of metrics scraped, processed and visualized. This may impact
+cluster operations. Consult the respective Prometheus Operator and
+Grafana documentation for set up guidance prior to using this configuration
+on mission critical Crossplane management clusters.
+
+**Warning**
+## Disclaimer: Metric Stability
+Crossplane has no concept of metric stability. This implies
+that metrics used in this configuration may be absent in future versions
+of Crossplane and / or its providers.
+


These warning messages do not provide much direction and the language has negative connotation. Consider rewriting them. What is the goal of having them here?

The goal is to guide users to keep their mission critical management clusters happily operational through proper Prometheus tuning. I'll come up with more positive wording.

candonov · 2023-11-15T00:12:49Z

README.md

@@ -20,37 +34,32 @@ at the conclusion of the tests by default.

 Apply the resource claim as follows to re-create
 the namespace, Prometheus, Grafana and dependencies for further


why is the claim 're-creating' the ns, Prometheus, and Grafana?

How about we offer a make cluster so that the workflow does not duplicate the installation after Uptest cleaned up its test setup? Users who want to run make e2e could do it purposefully as a separate step with or without pre-existing cluster.

Added a make cluster option.

candonov · 2023-11-15T00:13:44Z

README.md

-exploration.
+exploration. Note that the xmetrics configuration examples
+rely on the CRDs to be installed through the oss composition
+first.
 ```
 kubectl apply -f examples/oss.yaml


Is this claim installing x-metrics? If so, change the name. OSS is very generic, I would avoid using it.

Would you recommend to not use oss.yaml for the claim name, even though x-metrics, prometheus and grafana are open source?

If so, which name would resonate with you?

candonov · 2023-11-15T00:17:47Z

README.md

 To load dashboards that are part of this configuration repository,
 please apply the following dashboard resource claims.


Suggested change

To load dashboards that are part of this configuration repository,

please apply the following dashboard resource claims.

The following command will apply all the dashboard resource claims located in the `dashboards` folder. Each claim will create a respective Grafana dashboard.

candonov · 2023-11-15T00:23:00Z

README.md

-    tail +2|\
-    grep prometheus-0)
-kubectl -n operators port-forward ${PROMETHEUS_POD_NAME} 9090
+kubectl -n operators port-forward prometheus-oss-kube-prometheus-stack-prometheus-0 9090


Suggested change

kubectl -n operators port-forward prometheus-oss-kube-prometheus-stack-prometheus-0 9090

kubectl -n operators port-forward svc/kube-prometheus-stack-prometheus 9090:9090

candonov · 2023-11-15T00:25:01Z

README.md

-    tail +2|\
-    grep prometheus-0)
-kubectl -n operators port-forward ${PROMETHEUS_POD_NAME} 9090
+kubectl -n operators port-forward prometheus-oss-kube-prometheus-stack-prometheus-0 9090
 ```

 Use the following to forward localhost:3000 to the Grafana pod.


Suggested change

Use the following to forward localhost:3000 to the Grafana pod.

To access the Grafana dashboard, port-forward to the Grafana service on port 80.

candonov · 2023-11-15T00:28:53Z

examples/xmetrics.yaml

+metadata:
+  name: upbound-metric-sample
+spec:
+  matchName: ".upbound.io"


This creates a lot of custom metrics, can you provide an example with a list of common families instead?

Added more narrow scoped matches, e.g. ".s3.aws.upbound.io" or ".ec2.aws.upbound.io", etc.

candonov · 2023-11-15T00:29:54Z

examples/xmetrics.yaml

+---
+apiVersion: metrics.crossplane.io/v1
+kind: ClusterMetric
+metadata:
+  name: clustermetric-sample
+spec:
+  matchName: ".crossplane.io"


Is this for the community provider?

candonov · 2023-11-15T00:32:04Z

README.md

+Crossplane has no concept of metric stability. This implies
+that metrics used in this configuration may be absent in future versions
+of Crossplane and / or its providers.
+
 ## Purpose
 The goal for configuration-observability-oss is to complement
 other configurations such as configuration-caas. See the


What is configuration-caas?

configuration-caas is a plug and play configuration for cluster as a service.

humoflife and others added 2 commits October 30, 2023 21:37

added x-metrics pod monitor

e79781a

feat(podmonitors): change pod monitors to operators namespace and add…

97c6a6e

… namespace selectors for crossplane-system and upbound-system Signed-off-by: Christopher Haar <[email protected]>

humoflife and others added 12 commits November 1, 2023 09:17

Merge pull request #9 from upbound/feature/streamline-podmonitors

ab39fc9

feat(podmonitors): change pod monitors

xmetrics dashboard checkpoint checkin

e75584e

variable setup for top level dashboard

80c294a

dashboard updates

798f785

added dashboard panels

91b73ff

added new line to yaml

96db29f

moved patch for deterministic dashboard creation

ec5c873

Merge pull request #11 from upbound/fix/dashboard-creation

9c1e7ed

moved patch for deterministic dashboard creation

moved patch for deterministic dashboard creation

7be5310

dashboard update

aca5877

Merge pull request #10 from upbound/feature/x-metrics-dashboard

09c9e68

xmetrics dashboard checkpoint checkin

added x-metrics pod monitor

884713b

humoflife added 4 commits November 7, 2023 08:49

Merge branch 'x-metrics' of github.com:upbound/configuration-observab…

aa7502e

…ility-oss into x-metrics

added usage

dab3b01

x-metrics port name is metrics

86ed1a4

added x-metrics install

f16fb4e

updated Makefile

b68c396

updated README.md and moved dashboards

7d199f6

updated README.md

ac0453e

humoflife and others added 2 commits November 13, 2023 16:38

updated Makefile

9ba87a6

added usage

10b18ff

humoflife and others added 7 commits November 13, 2023 16:38

x-metrics port name is metrics

e980050

added x-metrics install

8be1e6a

updated Makefile

d235260

updated README.md and moved dashboards

1a9e312

updated README.md

554b2c6

updated Makefile

b692476

Merge branch 'x-metrics' of github.com:upbound/configuration-observab…

8fb5789

…ility-oss into x-metrics

candonov reviewed Nov 15, 2023

View reviewed changes

humoflife added 4 commits November 15, 2023 08:44

replaced generic metrics example with smaller scoped ones

658dde8

added make cluster option and updated README disclaimers

7cb4f68

updated README.md

a4dc317

replace disclaimer with note

f5b61c3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added x-metrics pod monitor #8

added x-metrics pod monitor #8

humoflife commented Oct 31, 2023

haarchri commented Nov 1, 2023

humoflife commented Nov 7, 2023

humoflife commented Nov 8, 2023

humoflife commented Nov 8, 2023

humoflife commented Nov 8, 2023

humoflife commented Nov 8, 2023

haarchri commented Nov 8, 2023

humoflife commented Nov 14, 2023

candonov Nov 14, 2023

candonov Nov 15, 2023

humoflife Nov 15, 2023

candonov Nov 15, 2023

humoflife Nov 15, 2023

humoflife Nov 15, 2023

candonov Nov 15, 2023

humoflife Nov 15, 2023

candonov Nov 15, 2023 •

edited

Loading

humoflife Nov 15, 2023

candonov Nov 15, 2023

candonov Nov 15, 2023

humoflife Nov 15, 2023

candonov Nov 15, 2023

humoflife Nov 15, 2023 •

edited

Loading

candonov Nov 15, 2023

candonov Nov 15, 2023

humoflife Nov 15, 2023

		@@ -20,37 +34,32 @@ at the conclusion of the tests by default.

		Apply the resource claim as follows to re-create
		the namespace, Prometheus, Grafana and dependencies for further

		To load dashboards that are part of this configuration repository,
		please apply the following dashboard resource claims.

	To load dashboards that are part of this configuration repository,
	please apply the following dashboard resource claims.
	The following command will apply all the dashboard resource claims located in the `dashboards` folder. Each claim will create a respective Grafana dashboard.

	kubectl -n operators port-forward prometheus-oss-kube-prometheus-stack-prometheus-0 9090
	kubectl -n operators port-forward svc/kube-prometheus-stack-prometheus 9090:9090

	Use the following to forward localhost:3000 to the Grafana pod.
	To access the Grafana dashboard, port-forward to the Grafana service on port 80.

added x-metrics pod monitor #8

Are you sure you want to change the base?

added x-metrics pod monitor #8

Conversation

humoflife commented Oct 31, 2023

Description of your changes

How has this code been tested

haarchri commented Nov 1, 2023

humoflife commented Nov 7, 2023

humoflife commented Nov 8, 2023

humoflife commented Nov 8, 2023

humoflife commented Nov 8, 2023

humoflife commented Nov 8, 2023

haarchri commented Nov 8, 2023

humoflife commented Nov 14, 2023

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

candonov Nov 15, 2023 • edited Loading

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

humoflife Nov 15, 2023 • edited Loading

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

candonov Nov 15, 2023 •

edited

Loading

humoflife Nov 15, 2023 •

edited

Loading