NationTech/harmony
6
0
Fork 1
Code Issues 32 Pull Requests 7 Actions 7 Packages Projects Releases 1 Wiki Activity

Monitor an application within a tenant #86

Merged
letian merged 22 commits from feat/crd-alertmanager-configs into master 2025-08-04 21:42:05 +00:00
Conversation 0 Commits 22 Files Changed 43 +2292 -177
wjro commented 2025-07-11 20:03:36 +00:00
Owner
Copy Link

WIP: added implementation to deploy crd-alertmanagerconfigs

WIP: added implementation to deploy crd-alertmanagerconfigs
wjro added 1 commit 2025-07-11 20:03:40 +00:00
wjro added 1 commit 2025-07-14 17:07:30 +00:00
wjro added 1 commit 2025-07-14 17:42:10 +00:00
feat: added impl for webhook receiver for crd alertmanagerconfigs
Some checks failed
Run Check Script / check (pull_request) Failing after 49s
Details
7b91088828
wjro added 1 commit 2025-07-14 18:18:57 +00:00
fix: added updated Cargo
All checks were successful
Run Check Script / check (pull_request) Successful in 1m52s
Details
55a4e79ec4
wjro added 1 commit 2025-07-14 18:35:15 +00:00
fix: git conflict
All checks were successful
Run Check Script / check (pull_request) Successful in -19s
Details
1525ac2226
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
Collaborator
Copy Link

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 
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 
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
All checks were successful
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
Some checks failed
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
All checks were successful
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
All checks were successful
Run Check Script / check (pull_request) Successful in -37s
Details
0b965b6570
wjro added 1 commit 2025-07-31 20:18:00 +00:00
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
Owner
Copy Link

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
Owner
Copy Link

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
merge configure_receiver with AlertReceiver::install & cleanup unused stuff
All checks were successful
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
Owner
Copy Link

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
Owner
Copy Link

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
Owner
Copy Link

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
Owner
Copy Link

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
quick cleanup
All checks were successful
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
All checks were successful
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
Owner
Copy Link

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
All checks were successful
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
Owner
Copy Link

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
use new harmony_cli::run
All checks were successful
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
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
Architecture Decision Record

   
Good first issue

   
RFC
No Label
Architecture Decision Record
Good first issue
RFC
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: NationTech/harmony#86
No description provided.
Delete Branch "feat/crd-alertmanager-configs"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?