disable project on error, tune config, drop v2 project config endpoint

relay-config/src/config.rs

@@ -1017,7 +1017,9 @@ struct Cache {
     /// The cache timeout for project configurations in seconds.
     project_expiry: u32,
     /// Continue using project state this many seconds after cache expiry while a new state is
-    /// being fetched. This is added on top of `project_expiry` and `miss_expiry`. Default is 0.
+    /// being fetched. This is added on top of `project_expiry`.
+    ///
+    /// Default is 2 minutes.
     project_grace_period: u32,
     /// The cache timeout for downstream relay info (public keys) in seconds.
     relay_expiry: u32,
@@ -1053,17 +1055,17 @@ impl Default for Cache {
     fn default() -> Self {
         Cache {
             project_request_full_config: false,
-            project_expiry: 300, // 5 minutes
-            project_grace_period: 0,
-            relay_expiry: 3600,   // 1 hour
-            envelope_expiry: 600, // 10 minutes
+            project_expiry: 300,       // 5 minutes
+            project_grace_period: 120, // 2 minutes
+            relay_expiry: 3600,        // 1 hour
+            envelope_expiry: 600,      // 10 minutes
             envelope_buffer_size: 1000,
             miss_expiry: 60,                       // 1 minute
             batch_interval: 100,                   // 100ms
             downstream_relays_batch_interval: 100, // 100ms
             batch_size: 500,
             file_interval: 10,                // 10 seconds
-            eviction_interval: 60,            // 60 seconds
+            eviction_interval: 15,            // 15 seconds
             global_config_fetch_interval: 10, // 10 seconds
         }
     }

relay-server/src/endpoints/project_configs.rs

@@ -3,6 +3,7 @@ use std::sync::Arc;
 
 use axum::extract::{Query, Request};
 use axum::handler::Handler;
+use axum::http::StatusCode;
 use axum::response::{IntoResponse, Result};
 use axum::{Json, RequestExt};
 use relay_base_schema::project::ProjectKey;
@@ -17,11 +18,7 @@ use crate::services::global_config::{self, StatusResponse};
 use crate::services::projects::project::{
     LimitedParsedProjectState, ParsedProjectState, ProjectState, Revision,
 };
-
-/// V2 version of this endpoint.
-///
-/// The request is a list of [`ProjectKey`]s and the response a list of [`ProjectStateWrapper`]s
-const ENDPOINT_V2: u16 = 2;
+use crate::utils::ApiErrorResponse;
 
 /// V3 version of this endpoint.
 ///
@@ -31,6 +28,16 @@ const ENDPOINT_V2: u16 = 2;
 /// returned, or a further poll ensues.
 const ENDPOINT_V3: u16 = 3;
 
+#[derive(Debug, Clone, Copy, thiserror::Error)]
+#[error("This API version is no longer supported, upgrade your Relay or Client")]
+struct VersionOutdatedError;
+
+impl IntoResponse for VersionOutdatedError {
+    fn into_response(self) -> axum::response::Response {
+        (StatusCode::BAD_REQUEST, ApiErrorResponse::from_error(&self)).into_response()
+    }
+}
+
 /// Helper to deserialize the `version` query parameter.
 #[derive(Clone, Copy, Debug, Deserialize)]
 struct VersionQuery {
@@ -216,9 +223,14 @@ async fn inner(
     }))
 }
 
+/// Returns `true` for all `?version` query parameters that are no longer supported by Relay and Sentry.
+fn is_outdated(Query(query): Query<VersionQuery>) -> bool {
+    query.version < ENDPOINT_V3
+}
+
 /// Returns `true` if the `?version` query parameter is compatible with this implementation.
 fn is_compatible(Query(query): Query<VersionQuery>) -> bool {
-    query.version >= ENDPOINT_V2 && query.version <= ENDPOINT_V3
+    query.version >= ENDPOINT_V3 && query.version <= ENDPOINT_V3
 }
 
 /// Endpoint handler for the project configs endpoint.
@@ -234,9 +246,11 @@ fn is_compatible(Query(query): Query<VersionQuery>) -> bool {
 /// support old Relay versions.
 pub async fn handle(state: ServiceState, mut req: Request) -> Result<impl IntoResponse> {
     let data = req.extract_parts().await?;
-    Ok(if is_compatible(data) {
-        inner.call(req, state).await
+    if is_outdated(data) {
+        Err(VersionOutdatedError.into())
+    } else if is_compatible(data) {
+        Ok(inner.call(req, state).await)
     } else {
-        forward::forward(state, req).await
-    })
+        Ok(forward::forward(state, req).await)
+    }
 }

relay-server/src/services/projects/cache/service.rs

@@ -119,11 +119,14 @@ impl ProjectCacheService {
                 Ok(result) => result,
                 Err(err) => {
                     relay_log::error!(
+                        tags.project_key = fetch.project_key().as_str(),
                         error = &err as &dyn std::error::Error,
-                        "failed to fetch project state for {fetch:?}"
+                        "failed to fetch project from source: {fetch:?}"
                     );
-                    // Fallback to a pending project on error, we will retry.
-                    ProjectState::Pending.into()
+
+                    // TODO: change this to ProjectState::Pending once we consider it safe to do so.
+                    // see https://github.com/getsentry/relay/pull/4140.
+                    ProjectState::Disabled.into()
                 }
             };
 

relay-server/src/services/projects/cache/state.rs

@@ -96,12 +96,10 @@ impl ProjectStore {
                 _removed.is_some(),
                 "an expired project must exist in the shared state"
             );
+            relay_log::trace!(tags.project_key = project_key.as_str(), "project evicted");
 
             evicted += 1;
 
-            // TODO: garbage disposal? Do we still need that, we shouldn't have a problem with
-            // timings anymore.
-
             on_evict(project_key);
         }
         drop(shared);
@@ -169,10 +167,6 @@ impl Shared {
     /// The caller must ensure that the project cache is instructed to
     /// [`super::ProjectCache::Fetch`] the retrieved project.
     pub fn get_or_create(&self, project_key: ProjectKey) -> SharedProject {
-        // TODO: do we need to check for expiry here?
-        // TODO: if yes, we need to include the timestamp in the shared project state.
-        // TODO: grace periods?
-
         // The fast path, we expect the project to exist.
         let projects = self.projects.pin();
         if let Some(project) = projects.get(&project_key) {
@@ -235,7 +229,8 @@ impl ProjectRef<'_> {
     }
 
     fn complete_fetch(&mut self, fetch: CompletedFetch) {
-        self.private.complete_fetch(&fetch);
+        let now = Instant::now();
+        self.private.complete_fetch(&fetch, now);
 
         // Keep the old state around if the current fetch is pending.
         // It may still be useful to callers.
@@ -427,7 +422,7 @@ impl PrivateProjectState {
         let when = match &self.state {
             FetchState::InProgress {} => {
                 relay_log::trace!(
-                    project_key = self.project_key.as_str(),
+                    tags.project_key = self.project_key.as_str(),
                     "project fetch skipped, fetch in progress"
                 );
                 return None;
@@ -440,7 +435,7 @@ impl PrivateProjectState {
                 if last_fetch.check_expiry(now, config).is_updated() {
                     // The current state is up to date, no need to start another fetch.
                     relay_log::trace!(
-                        project_key = self.project_key.as_str(),
+                        tags.project_key = self.project_key.as_str(),
                         "project fetch skipped, already up to date"
                     );
                     return None;
@@ -452,8 +447,8 @@ impl PrivateProjectState {
         // Mark a current fetch in progress.
         self.state = FetchState::InProgress {};
 
-        relay_log::debug!(
-            project_key = &self.project_key.as_str(),
+        relay_log::trace!(
+            tags.project_key = &self.project_key.as_str(),
             attempts = self.backoff.attempt() + 1,
             "project state fetch scheduled in {:?}",
             when.saturating_duration_since(Instant::now()),
@@ -466,20 +461,28 @@ impl PrivateProjectState {
         })
     }
 
-    fn complete_fetch(&mut self, fetch: &CompletedFetch) {
+    fn complete_fetch(&mut self, fetch: &CompletedFetch, now: Instant) {
         debug_assert!(
             matches!(self.state, FetchState::InProgress),
             "fetch completed while there was no current fetch registered"
         );
 
         if fetch.is_pending() {
             self.state = FetchState::Pending {
-                next_fetch_attempt: Instant::now().checked_add(self.backoff.next_backoff()),
+                next_fetch_attempt: now.checked_add(self.backoff.next_backoff()),
             };
+            relay_log::trace!(
+                tags.project_key = &self.project_key.as_str(),
+                "project state fetch completed but still pending"
+            );
         } else {
+            relay_log::trace!(
+                tags.project_key = &self.project_key.as_str(),
+                "project state fetch completed with non-pending config"
+            );
             self.backoff.reset();
             self.state = FetchState::NonPending {
-                last_fetch: LastFetch(Instant::now()),
+                last_fetch: LastFetch(now),
             };
         }
     }