[issue-464]Create a Prometheus ServiceMonitor object that can capture/collect metrics from deployed SonataFlow instances #540

jianrongzhang89 · 2024-09-23T19:21:33Z

The operator an track in the cluster if Prometheus Operator is installed. If this condition is true and the SonataFlowPlatform is marked with .spec.monitoring.enabled: true, and the workflow is deployed as an k8s deployment, then each workflow can have a ServiceMonitor object bound to it.

Note: for workflows deployed as Knative services, this PR does not create the ServiceMonitor objects.

Checklist

[X ] Add or Modify a unit test for your change
[X ] Have you verified that tall the tests are passing?

ricardozanini

Thanks @jianrongzhang89! I left a few comments, but I believe we are on the right path.

Makefile

controllers/profiles/common/monitoring.go

controllers/profiles/common/object_creators.go

controllers/profiles/preview/deployment_handler.go

controllers/profiles/preview/profile_preview.go

test/testdata/monitoring.yaml

utils/monitoring/monitoring.go

ricardozanini · 2024-10-01T15:45:39Z

Please add conditionals when registering the listeners in the controller:

2024-10-01T14:32:35.471896952Z stderr F E1001 14:32:35.471807       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:32:45.472140785Z stderr F E1001 14:32:45.472033       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:32:55.471966197Z stderr F E1001 14:32:55.471868       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:33:05.472046816Z stderr F E1001 14:33:05.471956       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:33:15.472269129Z stderr F E1001 14:33:15.472185       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:33:25.472286385Z stderr F E1001 14:33:25.472204       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:33:35.4720388Z stderr F E1001 14:33:35.471924       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:33:45.472208024Z stderr F E1001 14:33:45.472121       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:33:55.472168441Z stderr F E1001 14:33:55.472041       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"
2024-10-01T14:34:05.471752553Z stderr F E1001 14:34:05.471677       1 kind.go:71] "msg"="if kind is a CRD, it should be installed before calling Start" "error"="no matches for kind \"ServiceMonitor\" in version \"monitoring.coreos.com/v1\"" "kind"={"Group":"monitoring.coreos.com","Kind":"ServiceMonitor"} "logger"="sonataflow-manager.controller-runtime.source.EventHandler"

ricardozanini · 2024-10-01T18:05:37Z

The e2e tests have been fixed, please rebase your branch 🙏

jianrongzhang89 · 2024-10-02T14:42:07Z

Please add conditionals when registering the listeners in the controller:

Done!

jianrongzhang89 · 2024-10-02T14:42:23Z

The e2e tests have been fixed, please rebase your branch 🙏

Code has been rebased.

ricardozanini · 2024-10-02T16:49:36Z

Can you check the files generation? 😬

bundle/manifests/sonataflow.org_sonataflowclusterplatforms.yaml

ricardozanini · 2024-10-04T13:15:27Z

@jianrongzhang89 can you please rebase? Your Knative fix has been merged with the e2e refactoring.

ricardozanini

I will stop reviewing since we will rescope this feature.

api/v1alpha08/sonataflowplatform_types.go

internal/controller/profiles/common/mutate_visitors.go

internal/controller/profiles/common/object_creators.go

ricardozanini

Also, I think we can have a rather simple test for monitoring in the e2e. Do we really need all the supporting services and persistence?

api/v1alpha08/sonataflowplatform_types.go

internal/controller/profiles/dev/states_dev.go

internal/controller/profiles/monitoring/monitoring.go

internal/controller/profiles/preview/profile_preview.go

test/e2e/workflow_test.go

test/e2e/testdata/workflows/prometheus/k8s_deployment/02-sonataflow_greeting.yaml

test/e2e/workflow_test.go

wmedvede

Some minor comments from my side.
I'll give it a try.

internal/controller/profiles/common/object_creators.go

internal/controller/profiles/dev/states_dev.go

test/e2e/testdata/workflows/prometheus/k8s_deployment/01-sonataflow_platform.yaml

test/e2e/testdata/workflows/prometheus/k8s_deployment/02-sonataflow_greetings.yaml

test/e2e/testdata/workflows/prometheus/k8s_deployment/kustomization.yaml

test/testdata/grafana.yaml

test/testdata/prometheus.yaml

internal/controller/monitoring/monitoring.go

wmedvede · 2024-10-18T13:49:43Z

@ricardozanini @jianrongzhang89

My last comment on this PR guys.
Feel free to open a follow up issue if you believe makes sense.

case1:
I have brand new OpenShift Local installation.
I never installed the Prometheus operator
I create a SFP with spec.monitoring.enabled = true
I create a WF in namespace test
After the WF deployment I can see a ServiceMonitor monitor created for it, weird.
Only explanation is that OpenShift Local already has the operator installed the Prometheus resources ouf-of-the-box

In customer OpenShfift cluster installations different thant OpenShift Local installations IDK what will happen. Probably the operator is always there.

case2:
I install the Prometheus operator in namespace test1
I create a SFP, with spec.monitoring.enabled = true
I create, a WF, etc,
I can see the ServiceMonitior, etc.

I create a namespace test2, I don't install the Prometheus operator in that namespace.
I create a SFP, with spec.monitoring.enabled = true
I create, a WF, etc,
I can see that a ServiceMonitior, etc.

Sort of corolary o case1

wmedvede

left a last comment

ricardozanini · 2024-10-18T14:04:38Z

@wmedvede I think we can safely create the monitor always if the monitoring: true is enabled and Prometheus is available.

My only take is that we should report in events that monitoring: true and Prometheus is not available. This must be done in this PR. @jianrongzhang89 can you confirm?

jianrongzhang89 · 2024-10-18T15:08:52Z

@wmedvede I think we can safely create the monitor always if the monitoring: true is enabled and Prometheus is available.

My only take is that we should report in events that monitoring: true and Prometheus is not available. This must be done in this PR. @jianrongzhang89 can you confirm?

@ricardozanini yes, in this case, a message will be logged in the platform log:
I1018 15:05:40.135463 1 sonataflowplatform_controller.go:146] Monitoring is enabled in platform sonataflow-platform, but Prometheus is not installed

ricardozanini · 2024-10-18T15:12:01Z

@jianrongzhang89 Can you change to an event instead too?

jianrongzhang89 · 2024-10-18T16:22:19Z

@jianrongzhang89 Can you change to an event instead too?

Done

…e/collect metrics from deployed SonataFlow instances

…e/collect metrics from deployed SonataFlow instances (apache#540)

jianrongzhang89 requested a review from ricardozanini as a code owner September 23, 2024 19:21

ricardozanini requested review from jakubschwan and wmedvede September 24, 2024 21:32

ricardozanini reviewed Sep 24, 2024

View reviewed changes

jianrongzhang89 force-pushed the servicemonitor branch 3 times, most recently from b8091fc to 18b1063 Compare October 1, 2024 14:10

jianrongzhang89 force-pushed the servicemonitor branch 2 times, most recently from d1438cc to a0edf9d Compare October 2, 2024 14:31

ricardozanini reviewed Oct 2, 2024

View reviewed changes

bundle/manifests/sonataflow.org_sonataflowclusterplatforms.yaml Outdated Show resolved Hide resolved

jianrongzhang89 force-pushed the servicemonitor branch 4 times, most recently from 6231535 to 038527b Compare October 4, 2024 01:35

jianrongzhang89 force-pushed the servicemonitor branch from 5d12fc2 to 0eb16ee Compare October 8, 2024 02:35

ricardozanini reviewed Oct 10, 2024

View reviewed changes

api/v1alpha08/sonataflowplatform_types.go Outdated Show resolved Hide resolved

internal/controller/profiles/common/mutate_visitors.go Outdated Show resolved Hide resolved

internal/controller/profiles/common/object_creators.go Outdated Show resolved Hide resolved

jianrongzhang89 force-pushed the servicemonitor branch 2 times, most recently from 59f71df to 86a5975 Compare October 11, 2024 00:27

ricardozanini reviewed Oct 11, 2024

View reviewed changes

jianrongzhang89 force-pushed the servicemonitor branch 5 times, most recently from ab5b575 to 506d9f1 Compare October 15, 2024 14:53

jianrongzhang89 force-pushed the servicemonitor branch 2 times, most recently from a4d77e9 to 309827a Compare October 15, 2024 19:40

ricardozanini reviewed Oct 15, 2024

View reviewed changes

test/e2e/testdata/workflows/prometheus/k8s_deployment/02-sonataflow_greeting.yaml Outdated Show resolved Hide resolved

test/e2e/workflow_test.go Show resolved Hide resolved

jianrongzhang89 force-pushed the servicemonitor branch from 309827a to 5ce5c92 Compare October 16, 2024 15:03

ricardozanini approved these changes Oct 16, 2024

View reviewed changes

wmedvede reviewed Oct 17, 2024

View reviewed changes

jianrongzhang89 force-pushed the servicemonitor branch from 5ce5c92 to 0a24d5c Compare October 17, 2024 16:06

wmedvede approved these changes Oct 18, 2024

View reviewed changes

jianrongzhang89 force-pushed the servicemonitor branch from 0a24d5c to a7769e9 Compare October 18, 2024 16:16

jianrongzhang89 force-pushed the servicemonitor branch 3 times, most recently from fe75da8 to 215631d Compare October 18, 2024 21:39

[issue-464] Create a Prometheus ServiceMonitor object that can captur…

74d5d88

…e/collect metrics from deployed SonataFlow instances

jianrongzhang89 force-pushed the servicemonitor branch from 215631d to 74d5d88 Compare October 20, 2024 02:34

jakubschwan approved these changes Oct 23, 2024

View reviewed changes

ricardozanini merged commit 4f2ba09 into apache:main Oct 23, 2024
4 checks passed

rgdoliveira pushed a commit to rgdoliveira/kogito-serverless-operator that referenced this pull request Oct 24, 2024

[issue-464] Create a Prometheus ServiceMonitor object that can captur…

b99f958

…e/collect metrics from deployed SonataFlow instances (apache#540)

[issue-464]Create a Prometheus ServiceMonitor object that can capture/collect metrics from deployed SonataFlow instances #540

[issue-464]Create a Prometheus ServiceMonitor object that can capture/collect metrics from deployed SonataFlow instances #540

Uh oh!

Conversation

jianrongzhang89 commented Sep 23, 2024 • edited by ricardozanini Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ricardozanini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ricardozanini commented Oct 1, 2024

Uh oh!

ricardozanini commented Oct 1, 2024

Uh oh!

jianrongzhang89 commented Oct 2, 2024

Uh oh!

jianrongzhang89 commented Oct 2, 2024

Uh oh!

ricardozanini commented Oct 2, 2024

Uh oh!

Uh oh!

ricardozanini commented Oct 4, 2024

Uh oh!

ricardozanini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ricardozanini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wmedvede left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wmedvede commented Oct 18, 2024

Uh oh!

wmedvede left a comment

Choose a reason for hiding this comment

Uh oh!

ricardozanini commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jianrongzhang89 commented Oct 18, 2024

Uh oh!

ricardozanini commented Oct 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jianrongzhang89 commented Oct 18, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

jianrongzhang89 commented Sep 23, 2024 •

edited by ricardozanini

Loading

ricardozanini commented Oct 18, 2024 •

edited

Loading

ricardozanini commented Oct 18, 2024 •

edited

Loading