Add background refresh statistics and monitoring coordination

- Add background_successful_refreshes and background_failed_refreshes
  counters to IssuerStats for tracking per-issuer background refresh results
- Add is_running() method to BackgroundRefreshManager to check thread state
- Track statistics in refresh_loop() when JWKS refresh succeeds or fails
- Add maybe_write_monitoring_file_from_verify() that skips file writes when
  background refresh thread is running (to avoid redundant writes)
- Write monitoring file from background thread at end of each refresh cycle
- Update get_json() to include new background refresh statistics
- Update integration test to verify background refresh via monitoring API
  using keycache_set_jwks() to force cache entry with short update interval

Author: bbockelm

src/scitokens_internal.cpp

@@ -110,19 +110,26 @@ void BackgroundRefreshManager::refresh_loop() {
 
             // If next update is within threshold, try to refresh
             if (time_until_update <= threshold) {
+                auto &stats =
+                    MonitoringStats::instance().get_issuer_stats(issuer);
                 try {
                     // Perform refresh (this will use the refresh_jwks method)
                     scitokens::Validator::refresh_jwks(issuer);
+                    stats.inc_background_successful_refresh();
                 } catch (std::exception &) {
+                    // Track failed refresh attempts
+                    stats.inc_background_failed_refresh();
                     // Silently ignore errors in background refresh to avoid
                     // disrupting the application. Background refresh is a
                     // best-effort optimization. If it fails, the next token
                     // verification will trigger a foreground refresh as usual.
-                    // TODO: In future work, track statistics (success/failure
-                    // counts) to monitor refresh health.
                 }
             }
         }
+
+        // Write monitoring file from background thread if configured
+        // This avoids writing from verify() when background thread is running
+        MonitoringStats::instance().maybe_write_monitoring_file();
     }
 }
 

src/scitokens_internal.h

@@ -122,6 +122,11 @@ class BackgroundRefreshManager {
     // Stop the background refresh thread (can be called multiple times)
     void stop();
 
+    // Check if the background refresh thread is running
+    bool is_running() const {
+        return m_running.load(std::memory_order_acquire);
+    }
+
   private:
     BackgroundRefreshManager() = default;
     ~BackgroundRefreshManager() { stop(); }
@@ -216,6 +221,10 @@ struct IssuerStats {
     std::atomic<uint64_t> failed_refreshes{0};
     std::atomic<uint64_t> stale_key_uses{0};
 
+    // Background refresh statistics (tracked by background thread)
+    std::atomic<uint64_t> background_successful_refreshes{0};
+    std::atomic<uint64_t> background_failed_refreshes{0};
+
     // Increment methods for atomic counters
     void inc_successful_validation() { successful_validations++; }
     void inc_unsuccessful_validation() { unsuccessful_validations++; }
@@ -227,6 +236,10 @@ struct IssuerStats {
     void inc_expired_key() { expired_keys++; }
     void inc_successful_key_lookup() { successful_key_lookups++; }
     void inc_failed_key_lookup() { failed_key_lookups++; }
+    void inc_background_successful_refresh() {
+        background_successful_refreshes++;
+    }
+    void inc_background_failed_refresh() { background_failed_refreshes++; }
 
     // Time setters that accept std::chrono::duration
     template <typename Rep, typename Period>
@@ -321,6 +334,13 @@ class MonitoringStats {
      */
     void maybe_write_monitoring_file() noexcept;
 
+    /**
+     * Same as maybe_write_monitoring_file(), but skips if background refresh
+     * thread is running. This should be called from verify() routines to
+     * avoid redundant writes when the background thread is handling them.
+     */
+    void maybe_write_monitoring_file_from_verify() noexcept;
+
   private:
     MonitoringStats() = default;
     ~MonitoringStats() = default;
@@ -674,8 +694,9 @@ class Validator {
 
     void verify(const SciToken &scitoken, time_t expiry_time) {
         // Check if monitoring file should be written (fast-path, relaxed
-        // atomic)
-        internal::MonitoringStats::instance().maybe_write_monitoring_file();
+        // atomic). Skip if background thread is running.
+        internal::MonitoringStats::instance()
+            .maybe_write_monitoring_file_from_verify();
 
         std::string issuer = "";
         auto start_time = std::chrono::steady_clock::now();
@@ -770,8 +791,9 @@ class Validator {
 
     void verify(const jwt::decoded_jwt<jwt::traits::kazuho_picojson> &jwt) {
         // Check if monitoring file should be written (fast-path, relaxed
-        // atomic)
-        internal::MonitoringStats::instance().maybe_write_monitoring_file();
+        // atomic). Skip if background thread is running.
+        internal::MonitoringStats::instance()
+            .maybe_write_monitoring_file_from_verify();
 
         std::string issuer = "";
         auto start_time = std::chrono::steady_clock::now();

src/scitokens_monitoring.cpp

@@ -121,6 +121,12 @@ std::string MonitoringStats::get_json() const {
         issuer_obj["stale_key_uses"] =
             picojson::value(static_cast<double>(stats.stale_key_uses.load()));
 
+        // Background refresh statistics
+        issuer_obj["background_successful_refreshes"] = picojson::value(
+            static_cast<double>(stats.background_successful_refreshes.load()));
+        issuer_obj["background_failed_refreshes"] = picojson::value(
+            static_cast<double>(stats.background_failed_refreshes.load()));
+
         std::string sanitized_issuer = sanitize_issuer_for_json(issuer);
         issuers_obj[sanitized_issuer] = picojson::value(issuer_obj);
     }
@@ -190,6 +196,15 @@ void MonitoringStats::maybe_write_monitoring_file() noexcept {
     }
 }
 
+void MonitoringStats::maybe_write_monitoring_file_from_verify() noexcept {
+    // If background refresh thread is running, it will handle the writes
+    // This avoids redundant writes and potential contention
+    if (BackgroundRefreshManager::get_instance().is_running()) {
+        return;
+    }
+    maybe_write_monitoring_file();
+}
+
 void MonitoringStats::write_monitoring_file_impl() noexcept {
     try {
         std::string monitoring_file =

test/integration_test.cpp

@@ -39,6 +39,9 @@ class MonitoringStats {
         uint64_t expired_keys{0};
         uint64_t failed_refreshes{0};
         uint64_t stale_key_uses{0};
+        // Background refresh statistics
+        uint64_t background_successful_refreshes{0};
+        uint64_t background_failed_refreshes{0};
     };
 
     struct FailedIssuerLookup {
@@ -157,6 +160,19 @@ class MonitoringStats {
                             static_cast<uint64_t>(it->second.get<double>());
                     }
 
+                    // Background refresh statistics
+                    it = stats_obj.find("background_successful_refreshes");
+                    if (it != stats_obj.end() && it->second.is<double>()) {
+                        stats.background_successful_refreshes =
+                            static_cast<uint64_t>(it->second.get<double>());
+                    }
+
+                    it = stats_obj.find("background_failed_refreshes");
+                    if (it != stats_obj.end() && it->second.is<double>()) {
+                        stats.background_failed_refreshes =
+                            static_cast<uint64_t>(it->second.get<double>());
+                    }
+
                     issuers_[issuer_entry.first] = stats;
                 }
             }
@@ -1113,6 +1129,13 @@ TEST_F(IntegrationTest, MonitoringFileOutput) {
 TEST_F(IntegrationTest, BackgroundRefreshTest) {
     char *err_msg = nullptr;
 
+    // Reset monitoring stats to get a clean baseline
+    scitoken_reset_monitoring_stats(&err_msg);
+    if (err_msg) {
+        free(err_msg);
+        err_msg = nullptr;
+    }
+
     // Set smaller intervals for testing (1 second refresh interval, 2 seconds
     // threshold)
     int rv =
@@ -1133,6 +1156,16 @@ TEST_F(IntegrationTest, BackgroundRefreshTest) {
         err_msg = nullptr;
     }
 
+    // Set update interval to 1 second BEFORE first verification so the
+    // cache entry will have next_update just 1 second in the future.
+    // This ensures the background thread can refresh within the test window.
+    rv = scitoken_config_set_int("keycache.update_interval_s", 1, &err_msg);
+    ASSERT_EQ(rv, 0);
+    if (err_msg) {
+        free(err_msg);
+        err_msg = nullptr;
+    }
+
     // Create a key and token
     std::unique_ptr<void, decltype(&scitoken_key_destroy)> key(
         scitoken_key_create("test-key-1", "ES256", public_key_.c_str(),
@@ -1203,22 +1236,37 @@ TEST_F(IntegrationTest, BackgroundRefreshTest) {
     ASSERT_EQ(rv, 0) << "Failed to get cached JWKS: "
                      << (err_msg ? err_msg : "unknown error");
     ASSERT_TRUE(jwks_before != nullptr);
-    std::unique_ptr<char, decltype(&free)> jwks_before_ptr(jwks_before, free);
     if (err_msg) {
         free(err_msg);
         err_msg = nullptr;
     }
 
     std::cout << "Initial JWKS fetched successfully" << std::endl;
 
-    // Set update interval to 1 second so keys will need refresh soon
-    rv = scitoken_config_set_int("keycache.update_interval_s", 1, &err_msg);
-    ASSERT_EQ(rv, 0);
+    // Re-set the JWKS to force a fresh cache entry with the current
+    // update_interval (1 second). This ensures next_update is just 1 second
+    // in the future so the background thread will refresh it.
+    rv = keycache_set_jwks(issuer_url_.c_str(), jwks_before, &err_msg);
+    ASSERT_EQ(rv, 0) << "Failed to set JWKS: "
+                     << (err_msg ? err_msg : "unknown error");
+    free(jwks_before);
     if (err_msg) {
         free(err_msg);
         err_msg = nullptr;
     }
 
+    std::cout << "JWKS re-set with 1-second update interval" << std::endl;
+
+    // Get monitoring stats before background refresh
+    auto before_stats = getCurrentMonitoringStats();
+    auto before_issuer_stats = before_stats.getIssuerStats(issuer_url_);
+    std::cout << "Before background refresh:" << std::endl;
+    std::cout << "  background_successful_refreshes: "
+              << before_issuer_stats.background_successful_refreshes
+              << std::endl;
+    std::cout << "  background_failed_refreshes: "
+              << before_issuer_stats.background_failed_refreshes << std::endl;
+
     // Enable background refresh
     rv = keycache_set_background_refresh(1, &err_msg);
     ASSERT_EQ(rv, 0) << "Failed to enable background refresh: "
@@ -1238,10 +1286,6 @@ TEST_F(IntegrationTest, BackgroundRefreshTest) {
     std::cout << "Waiting 4 seconds for background refresh..." << std::endl;
     sleep(4);
 
-    // The background refresh should have occurred
-    // We can't easily verify it refreshed without instrumenting the code more,
-    // but we can verify the thread is running and didn't crash
-
     // Stop background refresh
     rv = keycache_stop_background_refresh(&err_msg);
     ASSERT_EQ(rv, 0) << "Failed to stop background refresh: "
@@ -1265,6 +1309,30 @@ TEST_F(IntegrationTest, BackgroundRefreshTest) {
         err_msg = nullptr;
     }
 
+    // Verify that background refresh statistics increased for our issuer
+    auto after_stats = getCurrentMonitoringStats();
+    auto after_issuer_stats = after_stats.getIssuerStats(issuer_url_);
+
+    std::cout << "After background refresh:" << std::endl;
+    std::cout << "  background_successful_refreshes: "
+              << after_issuer_stats.background_successful_refreshes
+              << std::endl;
+    std::cout << "  background_failed_refreshes: "
+              << after_issuer_stats.background_failed_refreshes << std::endl;
+
+    // The background thread should have performed at least one refresh
+    // for our issuer (either successful or failed)
+    uint64_t total_background_refreshes =
+        after_issuer_stats.background_successful_refreshes +
+        after_issuer_stats.background_failed_refreshes;
+    uint64_t before_total =
+        before_issuer_stats.background_successful_refreshes +
+        before_issuer_stats.background_failed_refreshes;
+
+    EXPECT_GT(total_background_refreshes, before_total)
+        << "Background refresh thread should have performed at least one "
+           "refresh attempt for our issuer";
+
     std::cout << "Test completed successfully" << std::endl;
 }