Add simple cache to gha_logs endpoint

Urgau · Urgau · commit fe64f974b534 · 2025-06-29T14:51:31.000+02:00
diff --git a/src/gha_logs.rs b/src/gha_logs.rs
@@ -3,11 +3,56 @@ use crate::handlers::Context;
 use anyhow::Context as _;
 use hyper::header::{CONTENT_SECURITY_POLICY, CONTENT_TYPE};
 use hyper::{Body, Response, StatusCode};
+use std::collections::VecDeque;
 use std::str::FromStr;
 use std::sync::Arc;
 use uuid::Uuid;
 
 const ANSI_UP_URL: &str = "https://cdn.jsdelivr.net/npm/ansi_up@6.0.6/+esm";
+const MAX_CACHE_CAPACITY_BYTES: u64 = 50 * 1024 * 1024; // 50 Mb
+
+#[derive(Default)]
+pub struct GitHubActionLogsCache {
+    capacity: u64,
+    entries: VecDeque<(String, Arc<String>)>,
+}
+
+impl GitHubActionLogsCache {
+    pub fn get(&mut self, key: &String) -> Option<Arc<String>> {
+        if let Some(pos) = self.entries.iter().position(|(k, _)| k == key) {
+            // Move previously cached entry to the front
+            let entry = self.entries.remove(pos).unwrap();
+            self.entries.push_front(entry.clone());
+            Some(entry.1)
+        } else {
+            None
+        }
+    }
+
+    pub fn put(&mut self, key: String, value: Arc<String>) -> Arc<String> {
+        if value.len() as u64 > MAX_CACHE_CAPACITY_BYTES {
+            // Entry is too large, don't cache, return as is
+            return value;
+        }
+
+        // Remove duplicate or last entry when necessary
+        let removed = if let Some(pos) = self.entries.iter().position(|(k, _)| k == &key) {
+            self.entries.remove(pos)
+        } else if self.capacity + value.len() as u64 >= MAX_CACHE_CAPACITY_BYTES {
+            self.entries.pop_back()
+        } else {
+            None
+        };
+        if let Some(removed) = removed {
+            self.capacity -= removed.1.len() as u64;
+        }
+
+        // Add entry the front of the list ane return it
+        self.capacity += value.len() as u64;
+        self.entries.push_front((key, value.clone()));
+        value
+    }
+}
 
 pub async fn gha_logs(
     ctx: Arc<Context>,
@@ -52,27 +97,42 @@ async fn process_logs(
         anyhow::bail!("Repository `{repo}` is not part of team repos");
     }
 
-    let logs = ctx
-        .github
-        .raw_job_logs(
-            &github::IssueRepository {
-                organization: owner.to_string(),
-                repository: repo.to_string(),
-            },
-            log_id,
-        )
-        .await
-        .context("unable to get the raw logs")?;
+    let log_uuid = format!("{owner}/{repo}${log_id}");
+
+    let logs = 'logs: {
+        if let Some(logs) = ctx.gha_logs.write().await.get(&log_uuid) {
+            tracing::info!("gha_logs: cache hit for {log_uuid}");
+            break 'logs logs;
+        }
 
-    let json_logs = serde_json::to_string(&logs).context("unable to JSON-ify the raw logs")?;
+        tracing::info!("gha_logs: cache miss for {log_uuid}");
+        let logs = ctx
+            .github
+            .raw_job_logs(
+                &github::IssueRepository {
+                    organization: owner.to_string(),
+                    repository: repo.to_string(),
+                },
+                log_id,
+            )
+            .await
+            .context("unable to get the raw logs")?;
+
+        let json_logs = serde_json::to_string(&*logs).context("unable to JSON-ify the raw logs")?;
+
+        ctx.gha_logs
+            .write()
+            .await
+            .put(log_uuid.clone(), json_logs.into())
+    };
 
     let nonce = Uuid::new_v4().to_hyphenated().to_string();
 
     let html = format!(
         r#"<!DOCTYPE html>
 <html>
 <head>
-    <title>{owner}/{repo}${log_id} - triagebot</title>
+    <title>{log_uuid} - triagebot</title>
     <meta charset="UTF-8">
     <meta name="viewport" content="width=device-width, initial-scale=1.0">
     <link rel="icon" sizes="32x32" type="image/png" href="https://rust-lang.org/static/images/favicon-32x32.png">    
@@ -87,7 +147,7 @@ async fn process_logs(
     <script type="module" nonce="{nonce}">
         import {{ AnsiUp }} from '{ANSI_UP_URL}'
 
-        var logs = {json_logs};
+        var logs = {logs};
         var ansi_up = new AnsiUp();
 
         var html = ansi_up.ansi_to_html(logs);
@@ -101,7 +161,7 @@ async fn process_logs(
 </html>"#,
     );
 
-    tracing::info!("gha_logs: serving logs for {owner}/{repo}#{log_id}");
+    tracing::info!("gha_logs: serving logs for {log_uuid}");
 
     return Ok(Response::builder()
         .status(StatusCode::OK)
diff --git a/src/handlers.rs b/src/handlers.rs
@@ -1,4 +1,5 @@
 use crate::config::{self, Config, ConfigurationError};
+use crate::gha_logs::GitHubActionLogsCache;
 use crate::github::{Event, GithubClient, IssueCommentAction, IssuesAction, IssuesEvent};
 use crate::handlers::pr_tracking::ReviewerWorkqueue;
 use crate::team_data::TeamClient;
@@ -380,4 +381,5 @@ pub struct Context {
     /// Represents the workqueue (assigned open PRs) of individual reviewers.
     /// tokio's RwLock is used to avoid deadlocks, since we run on a single-threaded tokio runtime.
     pub workqueue: Arc<tokio::sync::RwLock<ReviewerWorkqueue>>,
+    pub gha_logs: Arc<tokio::sync::RwLock<GitHubActionLogsCache>>,
 }
diff --git a/src/main.rs b/src/main.rs
@@ -12,6 +12,7 @@ use tokio::{task, time};
 use tower::{Service, ServiceExt};
 use tracing as log;
 use tracing::Instrument;
+use triagebot::gha_logs::GitHubActionLogsCache;
 use triagebot::handlers::pr_tracking::ReviewerWorkqueue;
 use triagebot::handlers::pr_tracking::load_workqueue;
 use triagebot::jobs::{
@@ -318,6 +319,7 @@ async fn run_server(addr: SocketAddr) -> anyhow::Result<()> {
         team: team_api,
         octocrab: oc,
         workqueue: Arc::new(RwLock::new(workqueue)),
+        gha_logs: Arc::new(RwLock::new(GitHubActionLogsCache::default())),
         zulip,
     });
 
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
@@ -82,6 +82,7 @@ impl TestContext {
             username: "triagebot-test".to_string(),
             octocrab,
             workqueue: Arc::new(RwLock::new(Default::default())),
+            gha_logs: Arc::new(RwLock::new(Default::default())),
         };
 
         Self {
