Monitor an application within a tenant #86

wjro changed title from wip: added an implementation of CRDalertmanagerconfigs that can be used to add a discord webhook receiver, currently the namespace is hard coded and there are a bunch of todos!() that need to be cleaned up, and flags need to be added so that alertmanag… to added an implementation of CRDalertmanagerconfigs that can be used to add a discord webhook receiver, currently the namespace is hard coded and there are a bunch of todos!() that need to be cleaned up, and flags need to be added so that alertmanag…

2025-07-14 18:36:10 +00:00

taha reviewed 2025-07-14 19:59:05 +00:00

examples/rust/src/main.rs Outdated

						
				@@ -34,2 +49,3 @@

				            Box::new(Monitoring {

				            Box::new(PrometheusApplicationMonitoring {

				                application: application.clone(),

				                alert_receiver: vec![Box::new(discord_receiver), Box::new(webhook_receiver)],

taha commented

2025-07-14 19:55:34 +00:00

As JG mentioned, we want to keep this as simple as possible, as opinionated as possible.

If we keep it named Monitoring we can just replace the inner implementation later.

As JG mentioned, we want to keep this as simple as possible, as opinionated as possible. If we keep it named `Monitoring` we can just replace the inner implementation later.

wjro added 1 commit 2025-07-16 19:56:21 +00:00

wip:added impl for prometheuses, alertmanagers, prometheusrules, servicemonitors, and some default rules that are deployed for application monitor

Run Check Script / check (pull_request) Successful in 2m19s

Details

114219385f

working on implementing grafana crds via grafana operator
need to link prometheus rules and alert managers in prometheus, testing it shows that prometheus isnt detecting them automatically

wjro added 1 commit 2025-07-21 22:00:38 +00:00

Xwip: uses a helm chart to deploy a prometheus operator if crd are ont present in cluster, and deploys a grafana operator.

Run Check Script / check (pull_request) Successful in -32s

Details

1d8b503bd2

added a sample dashboard and prometheus data source to grafana

wjro added 1 commit 2025-07-22 19:56:42 +00:00

feat: added default prometheus rules and grafana dashboard for application monitoring

Run Check Script / check (pull_request) Successful in -32s

Details

b9e208f4cf

wjro added 1 commit 2025-07-28 19:15:45 +00:00

fix: prometheus operator and grafana operator deploy application namespace on local k3d

Run Check Script / check (pull_request) Failing after -1m5s

Details

b56a30de3c

if kube-prometheus-operator is present installation of prometheus-operator will skip
outside of local k3d installation installation of operator is skipped

wjro added 1 commit 2025-07-28 19:19:09 +00:00

fix: cargo fmt

Run Check Script / check (pull_request) Successful in -37s

Details

d7bce37b69

wjro added 1 commit 2025-07-28 19:22:52 +00:00

Merge remote-tracking branch 'origin/master' into feat/crd-alertmanager-configs

Run Check Script / check (pull_request) Successful in -37s

Details

0b965b6570

wjro added 1 commit 2025-07-31 20:18:00 +00:00

wip: need to convert the generic type AlertReceiver<CRDPrometheus> to CRDAlertManagerReceiver in k8sAnywhereTopology which extends AlertReceiver<CRDPrometheus> in order to be able to configure and install the receiver and its associated crd-alertmanagerconfigs to the cluster a5deda647b

letian reviewed 2025-08-01 21:38:03 +00:00

harmony/src/modules/prometheus/k3d_prometheus_alerting_score.rs Outdated

						
				@@ -0,0 +42,4 @@

				use super::prometheus::PrometheusMonitoring;

				#[derive(Clone, Debug, Serialize)]

				pub struct K3dPrometheusCRDAlertingScore {

letian commented

2025-08-01 21:38:03 +00:00

This file is almost identical to k8s_prometheus_alerting_score, the only difference being that this one has a namespace attribute whereas the k8s one has a sender that is used to retrieve the namespace.

Is there a reason for these 2 almost identical files?

Also, this file is actually unused.

This file is almost identical to `k8s_prometheus_alerting_score`, the only difference being that this one has a `namespace` attribute whereas the `k8s` one has a `sender` that is used to retrieve the namespace. Is there a reason for these 2 almost identical files? Also, this file is actually unused.

letian reviewed 2025-08-01 21:46:51 +00:00

harmony/src/modules/prometheus/prometheus.rs

						
				@@ -0,0 +13,4 @@

				}

				#[async_trait]

				pub trait PrometheusApplicationMonitoring<S: AlertSender>: PrometheusMonitoring<S> {

letian commented

2025-08-01 21:46:51 +00:00

This trait shouldn't be extending PrometheusMonitoring as they're not really related to each other. It's 2 different capabilities.

And actually if you dig a bit more, you see that ensure_prometheus_operator is used only internally by K8sAnywhere itself. So it should be a private method there instead of a separate capability.

This trait shouldn't be extending `PrometheusMonitoring` as they're not really related to each other. It's 2 different capabilities. And actually if you dig a bit more, you see that `ensure_prometheus_operator` is used only internally by `K8sAnywhere` itself. So it should be a private method there instead of a separate capability.

letian marked this conversation as resolved

letian added 3 commits 2025-08-02 03:09:27 +00:00

cleanup unused k3d prometheus monitoring score & simplify design 961a300154

remove unnecessary configure_receivers method from trait 4b6bebcaf1

merge configure_receiver with AlertReceiver::install & cleanup unused stuff

Run Check Script / check (pull_request) Successful in -39s

Details

c6b255d0bd

letian force-pushed feat/crd-alertmanager-configs from 8d4e06ced1 to 056152a1e5

2025-08-02 03:52:39 +00:00

Compare

letian reviewed 2025-08-02 03:54:15 +00:00

harmony/src/domain/topology/k8s.rs

						
				@@ -25,6 +25,15 @@ pub struct K8sClient {

				    client: Client,

				}

				impl Serialize for K8sClient {

letian commented

2025-08-02 03:54:15 +00:00

I'm not sure it's really a good thing that we try to Serialize the K8sClient 🤔 maybe we should find a way to avoid needing to store the client into the CrdPrometheus sender

I'm not sure it's really a good thing that we try to `Serialize` the `K8sClient` 🤔 maybe we should find a way to avoid needing to store the `client` into the `CrdPrometheus` sender

letian reviewed 2025-08-02 04:09:37 +00:00

harmony/src/modules/prometheus/prometheus.rs

						
				@@ -0,0 +8,4 @@

				#[async_trait]

				pub trait PrometheusApplicationMonitoring<S: AlertSender> {

				    async fn install_prometheus(

letian commented

2025-08-02 04:09:37 +00:00

install_prometheus seems a bit inaccurate: we're not installing prometheus here, we're installing the receivers to run on prometheus

maybe should be renamed to install_receivers or install_monitoring or something similar

`install_prometheus` seems a bit inaccurate: we're not installing prometheus here, we're installing the receivers to run on prometheus maybe should be renamed to `install_receivers` or `install_monitoring` or something similar

letian reviewed 2025-08-02 04:17:48 +00:00

harmony/src/modules/monitoring/alert_channel/discord_alert_channel.rs

						
				@@ -23,2 +108,4 @@

				}

				#[async_trait]

				impl AlertReceiver<Prometheus> for DiscordWebhook {

letian commented

2025-08-02 04:17:48 +00:00

this Prometheus sender doesn't seem to be used, meaning the AlertReceiver<Prometheus> implementation doesn't seem to be used either (and same goes for the webhook alert channel)

this `Prometheus` sender doesn't seem to be used, meaning the `AlertReceiver<Prometheus>` implementation doesn't seem to be used either (and same goes for the webhook alert channel)

letian reviewed 2025-08-02 14:08:23 +00:00

harmony/src/modules/prometheus/k8s_prometheus_alerting_score.rs Outdated

						
				@@ -0,0 +243,4 @@

				        Ok(Outcome::success(format!(

				            "installed grafana operator in ns {}",

				            self.sender.namespace.clone().clone()

letian commented

2025-08-02 14:08:23 +00:00

is there a reason why these double clone().clone()? there's a lot of them in this file

is there a reason why these double `clone().clone()`? there's a lot of them in this file

letian added 2 commits 2025-08-02 15:11:03 +00:00

fix check to ensure prometheus operator is installed 9403581be5

quick cleanup

Run Check Script / check (pull_request) Successful in -42s

Details

064f6d88ba

letian added 1 commit 2025-08-02 15:58:50 +00:00

simpler check to see if a crd exists + cleanup

Run Check Script / check (pull_request) Successful in -41s

Details

a8394cda47

letian reviewed 2025-08-04 19:26:01 +00:00

harmony/src/domain/topology/tenant/k8s.rs

						
				@@ -234,2 +238,3 @@

				                      //network

				                "ipBlock": {

				                    "cidr": "172.23.0.0/16",

				                    "cidr": "172.24.0.0/16",

letian commented

2025-08-04 19:26:01 +00:00

Selon le contexte (e.g. K3dLocal), introduire une nouvelle implementation K3d/sTenantManager qui permet d'aller chercher dans docker l'adresse ip a definir ici.

Selon le contexte (e.g. K3dLocal), introduire une nouvelle implementation `K3d/sTenantManager` qui permet d'aller chercher dans docker l'adresse ip a definir ici.

letian added 1 commit 2025-08-04 19:28:16 +00:00

quick cleanup

Run Check Script / check (pull_request) Successful in -44s

Details

e078f5c062

letian reviewed 2025-08-04 20:41:06 +00:00

examples/application_monitoring_with_tenant/src/main.rs

						
				@@ -0,0 +28,4 @@

				    //TODO there is a bug where the application is deployed into the namespace matching the

				    //application name and the tenant is created in the namesapce matching the tenant name

				    //in order for the application to be deployed in the tenant namespace the application.name and

				    //the TenantConfig.name must match

letian commented

2025-08-04 20:41:06 +00:00

Personal opinion, but I'm not even sure the end user should have to worry about providing names for the tenant, the application, or any other scores. It would be simpler if there is one "global" name for the whole configuration.

Something like this:

    let tenant = TenantScore {
        config: TenantConfig {
            id: Id::from_str("test-tenant-id"),
            ..Default::default()
        },
    };

    let application = Arc::new(RustWebapp {
        domain: Url::Url(url::Url::parse("https://rustapp.harmony.example.com").unwrap()),
        project_root: PathBuf::from("./examples/rust/webapp"),
        framework: Some(RustWebFramework::Leptos),
    });

    let webhook_receiver = WebhookReceiver {
        url: Url::Url(url::Url::parse("https://webhook-doesnt-exist.com").unwrap()),
    };

    let app = ApplicationScore {
        features: vec![Box::new(Monitoring {
            alert_receiver: vec![Box::new(webhook_receiver)],
            application: application.clone(),
        })],
        application,
    };

    harmony_cli::run(
        "example-monitoring",
        vec![Box::new(tenant), Box::new(app)],
        None
    ).await.unwrap();

That would simplify the setup and we would be able to enforce that the tenant name is the same as the application name behind the scene.

Personal opinion, but I'm not even sure the end user should have to worry about providing names for the tenant, the application, or any other scores. It would be simpler if there is one "global" name for the whole configuration. Something like this: ```rs let tenant = TenantScore { config: TenantConfig { id: Id::from_str("test-tenant-id"), ..Default::default() }, }; let application = Arc::new(RustWebapp { domain: Url::Url(url::Url::parse("https://rustapp.harmony.example.com").unwrap()), project_root: PathBuf::from("./examples/rust/webapp"), framework: Some(RustWebFramework::Leptos), }); let webhook_receiver = WebhookReceiver { url: Url::Url(url::Url::parse("https://webhook-doesnt-exist.com").unwrap()), }; let app = ApplicationScore { features: vec![Box::new(Monitoring { alert_receiver: vec![Box::new(webhook_receiver)], application: application.clone(), })], application, }; harmony_cli::run( "example-monitoring", vec![Box::new(tenant), Box::new(app)], None ).await.unwrap(); ``` That would simplify the setup and we would be able to enforce that the tenant name is the same as the application name behind the scene.

letian added 2 commits 2025-08-04 21:22:26 +00:00

Merge branch 'master' into feat/crd-alertmanager-configs 569839bf66

use new harmony_cli::run

Run Check Script / check (pull_request) Successful in -44s

Details

5cc93d3107

letian changed title from added an implementation of CRDalertmanagerconfigs that can be used to add a discord webhook receiver, currently the namespace is hard coded and there are a bunch of todos!() that need to be cleaned up, and flags need to be added so that alertmanag… to Monitor an application within a tenant

2025-08-04 21:26:38 +00:00

letian merged commit 024084859e into master

2025-08-04 21:42:05 +00:00

letian referenced this issue from a commit

2025-08-04 21:42:07 +00:00

Monitor an application within a tenant (#86)

Sign in to join this conversation.

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: NationTech/harmony#86