revert: rate limit based on peer address #1351

common/src/models/error.rs

@@ -60,7 +60,6 @@ pub enum ErrorKind {
     NotReady,
     ServiceUnavailable,
     DeleteProjectFailed,
-    RateLimited(String),
 }
 
 impl From<ErrorKind> for ApiError {
@@ -127,12 +126,6 @@ impl From<ErrorKind> for ApiError {
             ErrorKind::Forbidden => (StatusCode::FORBIDDEN, "Forbidden"),
             ErrorKind::NotReady => (StatusCode::INTERNAL_SERVER_ERROR, "Service not ready"),
             ErrorKind::DeleteProjectFailed => (StatusCode::INTERNAL_SERVER_ERROR, "Deleting project failed"),
-            ErrorKind::RateLimited(message) => {
-                return Self {
-                    message,
-                    status_code: StatusCode::TOO_MANY_REQUESTS.as_u16(),
-                }
-            },
         };
         Self {
             message: error_message.to_string(),

deployer/src/handlers/error.rs

@@ -26,8 +26,6 @@ pub enum Error {
     Internal(#[from] anyhow::Error),
     #[error("Missing header: {0}")]
     MissingHeader(String),
-    #[error("{0}. Retry the request in a few minutes")]
-    RateLimited(String),
 }
 
 impl Serialize for Error {
@@ -49,7 +47,6 @@ impl IntoResponse for Error {
 
         let code = match self {
             Error::NotFound(_) => StatusCode::NOT_FOUND,
-            Error::RateLimited(_) => StatusCode::TOO_MANY_REQUESTS,
             _ => StatusCode::INTERNAL_SERVER_ERROR,
         };
 

deployer/src/handlers/mod.rs

@@ -33,7 +33,7 @@ use shuttle_common::{
     claims::{Claim, Scope},
     models::{
         deployment::{DeploymentRequest, CREATE_SERVICE_BODY_LIMIT, GIT_STRINGS_MAX_LENGTH},
-        error::{axum::CustomErrorPath, ApiError, ErrorKind},
+        error::axum::CustomErrorPath,
         project::ProjectName,
     },
     request_span, LogItem,
@@ -663,16 +663,8 @@ pub async fn get_logs(
                 .collect(),
         )),
         Err(error) => {
-            if error.code() == tonic::Code::Unavailable
-                && error.metadata().get("x-ratelimit-limit").is_some()
-            {
-                Err(Error::RateLimited(
-                    "your application is producing too many logs. Interactions with the shuttle logger service will be rate limited"
-                        .to_string(),
-                ))
-            } else {
-                Err(anyhow!("failed to retrieve logs for deployment").into())
-            }
+            error!(error = %error, "failed to retrieve logs for deployment");
+            Err(anyhow!("failed to retrieve logs for deployment").into())
         }
     }
 }
@@ -723,24 +715,10 @@ async fn logs_websocket_handler(
                 "failed to get backlog of logs"
             );
 
-            if error.code() == tonic::Code::Unavailable
-                && error.metadata().get("x-ratelimit-limit").is_some()
-            {
-                let message = serde_json::to_string(
-                    &ApiError::from(
-                        ErrorKind::RateLimited(
-                            "your application is producing too many logs. Interactions with the shuttle logger service will be rate limited"
-                            .to_string()
-                        )
-                    ))
-                    .expect("to convert error to json");
+            let _ = s
+                .send(ws::Message::Text("failed to get logs".to_string()))
+                .await;
 
-                let _ = s.send(ws::Message::Text(message)).await;
-            } else {
-                let _ = s
-                    .send(ws::Message::Text("failed to get logs".to_string()))
-                    .await;
-            }
             let _ = s.close().await;
             return;
         }

logger/Cargo.toml

@@ -12,7 +12,6 @@ shuttle-proto = { workspace = true, features = ["logger"] }
 async-trait = { workspace = true }
 chrono = { workspace = true }
 clap = { workspace = true }
-http = { workspace = true }
 prost-types = { workspace = true }
 serde_json = { workspace = true }
 sqlx = { workspace = true, features = [
@@ -25,8 +24,6 @@ thiserror = { workspace = true }
 tokio = { workspace = true, features = ["rt-multi-thread"] }
 tokio-stream = { workspace = true }
 tonic = { workspace = true }
-tower = { workspace = true }
-tower_governor = { version= "0.1.0", features = ["tracing"] }
 tracing = { workspace = true }
 tracing-subscriber = { workspace = true, features = ["default"] }
 
@@ -38,5 +35,3 @@ serde_json = { workspace = true }
 shuttle-common-tests = { workspace = true }
 uuid = { workspace = true }
 ctor = { workspace = true }
-futures = { workspace = true }
-tower = { workspace = true, features = ["util"] }

logger/src/lib.rs

@@ -15,7 +15,6 @@ use tracing::{debug, error, field, Span};
 
 pub mod args;
 mod dal;
-pub mod rate_limiting;
 
 pub use dal::Postgres;
 

logger/src/main.rs

@@ -8,15 +8,9 @@ use shuttle_common::{
     },
     log::Backend,
 };
-use shuttle_logger::{
-    args::Args,
-    rate_limiting::{tonic_error, TonicPeerIpKeyExtractor, BURST_SIZE, REFRESH_INTERVAL},
-    Postgres, Service,
-};
+use shuttle_logger::{args::Args, Postgres, Service};
 use shuttle_proto::logger::logger_server::LoggerServer;
 use tonic::transport::Server;
-use tower::ServiceBuilder;
-use tower_governor::{governor::GovernorConfigBuilder, GovernorLayer};
 use tracing::trace;
 
 #[tokio::main]
@@ -27,33 +21,12 @@ async fn main() {
 
     trace!(args = ?args, "parsed args");
 
-    let governor_config = GovernorConfigBuilder::default()
-        // Regenerate capacity at a rate of 2 requests per second, meaning the maximum capacity
-        // for sustained traffic is 2 RPS per peer address.
-        .per_millisecond(REFRESH_INTERVAL)
-        // Allow bursts of up to 6 requests, when any burst capacity is used, it will regenerate
-        // one element at a time at the rate set above.
-        .burst_size(BURST_SIZE)
-        .use_headers()
-        .key_extractor(TonicPeerIpKeyExtractor)
-        .finish()
-        .unwrap();
-
     let mut server_builder = Server::builder()
         .http2_keepalive_interval(Some(Duration::from_secs(60)))
         .layer(JwtAuthenticationLayer::new(AuthPublicKey::new(
             args.auth_uri,
         )))
-        .layer(ExtractPropagationLayer)
-        .layer(
-            ServiceBuilder::new()
-                // This middleware goes above `GovernorLayer` because it will receive errors returned by
-                // `GovernorLayer`.
-                .map_err(tonic_error)
-                .layer(GovernorLayer {
-                    config: &governor_config,
-                }),
-        );
+        .layer(ExtractPropagationLayer);
 
     let postgres = Postgres::new(&args.db_connection_uri).await;