Observability #6

Merged
qpismont merged 6 commits from 1.1 into main 2026-07-26 21:55:42 +02:00
10 changed files with 75 additions and 154 deletions
Showing only changes of commit 743b6b33c9 - Show all commits
+5 -1
View File
@@ -12,8 +12,12 @@
"containerEnv": { "containerEnv": {
"SHELL": "/bin/bash" "SHELL": "/bin/bash"
}, },
"customizations": {
"vscode": {
"extensions": ["fill-labs.dependi", "rust-lang.rust-analyzer", "tamasfe.even-better-toml"]
}
},
"workspaceMount": "source=${localWorkspaceFolder},target=/workspaces/herald,type=bind", "workspaceMount": "source=${localWorkspaceFolder},target=/workspaces/herald,type=bind",
"workspaceFolder": "/workspaces/herald", "workspaceFolder": "/workspaces/herald",
"runArgs": ["--userns=keep-id", "--security-opt", "label=disable"],
"appPort": [3000] "appPort": [3000]
} }
+3
View File
@@ -0,0 +1,3 @@
{
"rust-analyzer.check.command": "clippy"
}
-11
View File
@@ -1,11 +0,0 @@
{
"lsp": {
"rust-analyzer": {
"initialization_options": {
"check": {
"command": "clippy"
}
}
}
}
}
Generated
+1 -98
View File
@@ -280,15 +280,6 @@ version = "2.11.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3" checksum = "c4512299f36f043ab09a583e57bceb5a5aab7a73db1805848e8fef3c9e8c78b3"
[[package]]
name = "block-buffer"
version = "0.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cdd35008169921d80bc60d3d0ab416eecb028c4cd653352907921d95084790be"
dependencies = [
"hybrid-array",
]
[[package]] [[package]]
name = "block2" name = "block2"
version = "0.6.2" version = "0.6.2"
@@ -341,18 +332,6 @@ version = "0.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724" checksum = "613afe47fcd5fac7ccf1db93babcb082c5994d996f20b8b159f2ad1658eb5724"
[[package]]
name = "cmov"
version = "0.5.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "0c9ea0ac24bc397ab3c98583a3c9ba74fa56b09a4449bbe172b9b1ddb016027a"
[[package]]
name = "const-oid"
version = "0.10.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a6ef517f0926dd24a1582492c791b6a4818a4d94e789a334894aa15b0d12f55c"
[[package]] [[package]]
name = "convert_case" name = "convert_case"
version = "0.10.0" version = "0.10.0"
@@ -378,33 +357,6 @@ version = "0.8.7"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b"
[[package]]
name = "cpufeatures"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8b2a41393f66f16b0823bb79094d54ac5fbd34ab292ddafb9a0456ac9f87d201"
dependencies = [
"libc",
]
[[package]]
name = "crypto-common"
version = "0.2.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453"
dependencies = [
"hybrid-array",
]
[[package]]
name = "ctutils"
version = "0.4.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7d5515a3834141de9eafb9717ad39eea8247b5674e6066c404e8c4b365d2a29e"
dependencies = [
"cmov",
]
[[package]] [[package]]
name = "darling" name = "darling"
version = "0.20.11" version = "0.20.11"
@@ -523,18 +475,6 @@ dependencies = [
"unicode-xid", "unicode-xid",
] ]
[[package]]
name = "digest"
version = "0.11.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f1dd6dbb5841937940781866fa1281a1ff7bd3bf827091440879f9994983d5c2"
dependencies = [
"block-buffer",
"const-oid",
"crypto-common",
"ctutils",
]
[[package]] [[package]]
name = "dispatch2" name = "dispatch2"
version = "0.3.1" version = "0.3.1"
@@ -794,15 +734,13 @@ dependencies = [
"dotenvy", "dotenvy",
"futures-util", "futures-util",
"hex", "hex",
"hmac",
"openrouter-rs", "openrouter-rs",
"reqwest 0.12.28", "reqwest 0.12.28",
"ring",
"sentry", "sentry",
"sentry-anyhow", "sentry-anyhow",
"serde", "serde",
"serde_json", "serde_json",
"sha2",
"subtle",
"thiserror 2.0.18", "thiserror 2.0.18",
"tokio", "tokio",
"tokio-stream", "tokio-stream",
@@ -819,15 +757,6 @@ version = "0.4.3"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70"
[[package]]
name = "hmac"
version = "0.13.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "6303bc9732ae41b04cb554b844a762b4115a61bfaa81e3e83050991eeb56863f"
dependencies = [
"digest",
]
[[package]] [[package]]
name = "hostname" name = "hostname"
version = "0.4.2" version = "0.4.2"
@@ -895,15 +824,6 @@ version = "1.0.3"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9"
[[package]]
name = "hybrid-array"
version = "0.4.12"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9155a582abd142abc056962c29e3ce5ff2ad5469f4246b537ed42c5deba857da"
dependencies = [
"typenum",
]
[[package]] [[package]]
name = "hyper" name = "hyper"
version = "1.9.0" version = "1.9.0"
@@ -2268,17 +2188,6 @@ dependencies = [
"serde", "serde",
] ]
[[package]]
name = "sha2"
version = "0.11.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "446ba717509524cb3f22f17ecc096f10f4822d76ab5c0b9822c5f9c284e825f4"
dependencies = [
"cfg-if",
"cpufeatures",
"digest",
]
[[package]] [[package]]
name = "sharded-slab" name = "sharded-slab"
version = "0.1.7" version = "0.1.7"
@@ -2701,12 +2610,6 @@ version = "0.2.5"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b"
[[package]]
name = "typenum"
version = "1.20.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20"
[[package]] [[package]]
name = "uname" name = "uname"
version = "0.1.1" version = "0.1.1"
+2 -4
View File
@@ -25,8 +25,6 @@ tracing-subscriber = { version = "0.3", features=["env-filter"] }
axum = "0.8" axum = "0.8"
anyhow = { version = "1.0", features = ["backtrace"] } anyhow = { version = "1.0", features = ["backtrace"] }
thiserror = "2.0" thiserror = "2.0"
hmac = "0.13" ring = "0.17"
sha2 = "0.11"
hex = "0.4" hex = "0.4"
subtle = "2.6" bytes = "1.1"
bytes = "1.11"
+4 -11
View File
@@ -4,12 +4,10 @@ use axum::http::Request;
use axum::response::IntoResponse; use axum::response::IntoResponse;
use axum::routing::{get, post}; use axum::routing::{get, post};
use axum::{Json, Router}; use axum::{Json, Router};
use hmac::{Hmac, KeyInit, Mac};
use reqwest::StatusCode; use reqwest::StatusCode;
use ring::hmac;
use sentry::integrations::tower::{NewSentryLayer, SentryHttpLayer}; use sentry::integrations::tower::{NewSentryLayer, SentryHttpLayer};
use serde_json::Value; use serde_json::Value;
use sha2::Sha256;
use subtle::ConstantTimeEq;
use tower::ServiceBuilder; use tower::ServiceBuilder;
use tower_http::trace::TraceLayer; use tower_http::trace::TraceLayer;
use tracing::{info, instrument}; use tracing::{info, instrument};
@@ -105,7 +103,7 @@ where
}); });
}); });
let webhook = parse_webhook(&type_header, &app_state.config.bot_name, &body_bytes)?; let webhook = parse_webhook(&type_header, &app_state.bot.name(), &body_bytes)?;
Ok(WebhookExtract(webhook)) Ok(WebhookExtract(webhook))
} }
} }
@@ -137,12 +135,7 @@ fn parse_webhook(header: &str, bot_name: &str, body_bytes: &[u8]) -> Result<Webh
fn verify_signature(secret_key: &[u8], sig_header: &str, body: &[u8]) -> Result<(), AppError> { fn verify_signature(secret_key: &[u8], sig_header: &str, body: &[u8]) -> Result<(), AppError> {
let sig_header_decoded = let sig_header_decoded =
hex::decode(sig_header).map_err(|_| AppError::WebHookSigHeaderInvalidErr)?; hex::decode(sig_header).map_err(|_| AppError::WebHookSigHeaderInvalidErr)?;
let mut mac = Hmac::<Sha256>::new_from_slice(secret_key).map_err(anyhow::Error::from)?; let key = hmac::Key::new(hmac::HMAC_SHA256, secret_key);
mac.update(body); hmac::verify(&key, body, &sig_header_decoded).map_err(|_| AppError::WebHookSigHeaderInvalidErr)
let generated_hmac = mac.finalize().into_bytes();
bool::from(generated_hmac.ct_eq(&sig_header_decoded))
.then_some(())
.ok_or(AppError::WebHookSigHeaderInvalidErr)
} }
+24 -21
View File
@@ -1,10 +1,9 @@
use crate::{ use crate::{
env::EnvConfig,
gitea::{GiteaAPI, WebhookType}, gitea::{GiteaAPI, WebhookType},
open_router::OpenRouterClient, open_router::OpenRouterClient,
}; };
use serde::Deserialize; use serde::Deserialize;
use std::{collections::HashSet, sync::Arc, time::Duration}; use std::{collections::HashSet, sync::Arc};
use tokio::sync::Mutex; use tokio::sync::Mutex;
use tokio_util::sync::CancellationToken; use tokio_util::sync::CancellationToken;
use tracing::{error, info, instrument}; use tracing::{error, info, instrument};
@@ -26,33 +25,37 @@ pub struct ReviewItem {
#[derive(Clone)] #[derive(Clone)]
pub struct Bot { pub struct Bot {
config: EnvConfig, bot_name: String,
gitea_api: GiteaAPI, gitea_api: GiteaAPI,
open_router_client: OpenRouterClient, open_router_client: OpenRouterClient,
http_client: reqwest::Client, http_client: reqwest::Client,
max_concurrent: usize, max_concurrent: usize,
open_router_model: String,
actions_handled: Arc<Mutex<HashSet<u64>>>, actions_handled: Arc<Mutex<HashSet<u64>>>,
} }
impl Bot { impl Bot {
pub fn new(config: EnvConfig) -> anyhow::Result<Self> { pub fn new(
let gitea_timeout = config.gitea_timeout; bot_name: String,
let open_router_timeout = config.open_router_timeout; gitea_api: GiteaAPI,
open_router_client: OpenRouterClient,
Ok(Self { http_client: reqwest::Client,
gitea_api: GiteaAPI::new(&config.gitea_url, &config.gitea_token, gitea_timeout)?, max_concurrent: usize,
open_router_client: OpenRouterClient::new( open_router_model: String,
&config.open_router_api_key, ) -> Self {
&config.open_router_model, Self {
open_router_timeout, bot_name,
)?, gitea_api,
max_concurrent: config.bot_max_concurrent, open_router_client,
config, http_client,
max_concurrent,
open_router_model,
actions_handled: Arc::new(Mutex::new(HashSet::new())), actions_handled: Arc::new(Mutex::new(HashSet::new())),
http_client: reqwest::Client::builder() }
.timeout(Duration::from_secs(gitea_timeout)) }
.build()?,
qpismont marked this conversation as resolved
Review

La méthode name() retourne une String en clonant self.bot_name. Cela peut être inefficace si appelée fréquemment. Envisagez de retourner un &str pour éviter une allocation, ou changez le champ en Arc<str> si un partage est nécessaire.

La méthode `name()` retourne une `String` en clonant `self.bot_name`. Cela peut être inefficace si appelée fréquemment. Envisagez de retourner un `&str` pour éviter une allocation, ou changez le champ en `Arc<str>` si un partage est nécessaire.
}) pub fn name(&self) -> String {
self.bot_name.clone()
} }
pub async fn start( pub async fn start(
@@ -111,7 +114,7 @@ impl Bot {
&self.gitea_api, &self.gitea_api,
&self.open_router_client, &self.open_router_client,
&self.http_client, &self.http_client,
&self.config.open_router_model, &self.open_router_model,
review_payload, review_payload,
), ),
} }
-3
View File
@@ -7,7 +7,6 @@ pub struct EnvConfig {
pub open_router_api_key: String, pub open_router_api_key: String,
pub open_router_model: String, pub open_router_model: String,
pub open_router_timeout: u64, pub open_router_timeout: u64,
pub bot_name: String,
pub bot_max_concurrent: usize, pub bot_max_concurrent: usize,
pub gitea_url: String, pub gitea_url: String,
pub gitea_token: String, pub gitea_token: String,
@@ -16,7 +15,6 @@ pub struct EnvConfig {
pub fn load_config() -> anyhow::Result<EnvConfig> { pub fn load_config() -> anyhow::Result<EnvConfig> {
let http_port = try_get_env("HTTP_PORT")?.parse()?; let http_port = try_get_env("HTTP_PORT")?.parse()?;
let bot_name = try_get_env("BOT_NAME")?;
let webhook_secret = try_get_env("WEBHOOK_SIG_HEADER_SECRET")?; let webhook_secret = try_get_env("WEBHOOK_SIG_HEADER_SECRET")?;
let open_router_api_key = try_get_env("OPEN_ROUTER_API_KEY")?; let open_router_api_key = try_get_env("OPEN_ROUTER_API_KEY")?;
let open_router_model = try_get_env("OPEN_ROUTER_MODEL")?; let open_router_model = try_get_env("OPEN_ROUTER_MODEL")?;
@@ -29,7 +27,6 @@ pub fn load_config() -> anyhow::Result<EnvConfig> {
Ok(EnvConfig { Ok(EnvConfig {
http_port, http_port,
webhook_secret, webhook_secret,
bot_name,
open_router_api_key, open_router_api_key,
open_router_model, open_router_model,
open_router_timeout, open_router_timeout,
+16
View File
@@ -29,6 +29,21 @@ impl GiteaAPI {
}) })
} }
#[instrument(skip(self))]
pub async fn get_authorized_user(&self) -> anyhow::Result<User> {
let url = format!("{}/api/v1/user", self.base_url);
let res = self.client.get(url).send().await?;
if !res.status().is_success() {
return Err(anyhow::anyhow!(
"Failed to get authorized user: {}",
res.status()
));
}
res.json::<User>().await.map_err(anyhow::Error::from)
}
#[instrument(skip(self))] #[instrument(skip(self))]
pub async fn comment( pub async fn comment(
&self, &self,
@@ -192,6 +207,7 @@ pub struct Comment {
#[derive(Deserialize, Debug)] #[derive(Deserialize, Debug)]
pub struct User { pub struct User {
pub id: u64, pub id: u64,
pub login: String,
} }
#[derive(Deserialize, Debug)] #[derive(Deserialize, Debug)]
+20 -5
View File
@@ -1,6 +1,4 @@
use std::sync::Arc; use crate::{bot::Bot, gitea::{GiteaAPI, WebhookType}, open_router::OpenRouterClient, state::AppState};
use crate::{bot::Bot, gitea::WebhookType, state::AppState};
use dotenvy::dotenv; use dotenvy::dotenv;
use tokio::signal::unix::{SignalKind, signal}; use tokio::signal::unix::{SignalKind, signal};
@@ -51,17 +49,34 @@ fn main() -> anyhow::Result<()> {
async fn run() -> anyhow::Result<()> { async fn run() -> anyhow::Result<()> {
let config = env::load_config()?; let config = env::load_config()?;
let gitea_api = GiteaAPI::new(&config.gitea_url, &config.gitea_token, config.gitea_timeout)?;
let gitea_user = gitea_api.get_authorized_user().await?;
qpismont marked this conversation as resolved
Review

Le démarrage conditionnel de l'exportateur Prometheus est une bonne pratique. Toutefois, si l'installation échoue, l'erreur est propagée via ?, ce qui met fin à l'application. Cela peut être acceptable, mais un message d'erreur plus explicite (ex: avec tracing::error) aiderait au débogage.

Le démarrage conditionnel de l'exportateur Prometheus est une bonne pratique. Toutefois, si l'installation échoue, l'erreur est propagée via `?`, ce qui met fin à l'application. Cela peut être acceptable, mais un message d'erreur plus explicite (ex: avec `tracing::error`) aiderait au débogage.
info!( info!(
port = config.http_port, port = config.http_port,
model = %config.open_router_model, model = %config.open_router_model,
gitea_url = %config.gitea_url, gitea_url = %config.gitea_url,
bot_name = %config.bot_name, bot_name = %gitea_user.login,
"Starting Herald" "Starting Herald"
); );
let open_router_client = OpenRouterClient::new(
&config.open_router_api_key,
&config.open_router_model,
config.open_router_timeout,
)?;
let shutdown = CancellationToken::new(); let shutdown = CancellationToken::new();
let bot = Bot::new(config.clone())?; let bot = Bot::new(
gitea_user.login,
gitea_api,
open_router_client,
reqwest::Client::new(),
config.bot_max_concurrent,
config.open_router_model.clone(),
);
let (tx, rx) = tokio::sync::mpsc::channel::<WebhookType>(config.bot_max_concurrent * 2); let (tx, rx) = tokio::sync::mpsc::channel::<WebhookType>(config.bot_max_concurrent * 2);
let app_state = AppState { let app_state = AppState {
bot_tx: tx, bot_tx: tx,