From afd8dc67b082fe84c3f4f5f5edcbb99d3141207c Mon Sep 17 00:00:00 2001 From: Jan Michael Auer Date: Thu, 17 Jun 2021 10:24:20 +0200 Subject: [PATCH] ref(server): Remove metered actix_web client connector (#1021) Since we no longer use the actix_web HTTP client, we can remove the metered client connector. --- CHANGELOG.md | 4 ++ relay-server/src/actors/connector.rs | 85 ---------------------------- relay-server/src/actors/mod.rs | 1 - relay-server/src/metrics.rs | 27 --------- relay-server/src/service.rs | 5 -- 5 files changed, 4 insertions(+), 118 deletions(-) delete mode 100644 relay-server/src/actors/connector.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e6094d4e2..84b00b7ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ - All fields in breakdown config should be camelCase, and rename the breakdown key name in project options. ([#1020](https://github.com/getsentry/relay/pull/1020)) +**Bug Fixes**: + +- Remove connection metrics reported under `connector.*`. They have been fully disabled since version `21.3.0`. ([#1021](https://github.com/getsentry/relay/pull/1021)) + ## 21.6.1 - No documented changes. diff --git a/relay-server/src/actors/connector.rs b/relay-server/src/actors/connector.rs deleted file mode 100644 index 22a03e8695..0000000000 --- a/relay-server/src/actors/connector.rs +++ /dev/null @@ -1,85 +0,0 @@ -//! Provides a metered client connector for upstream connections. -//! -//! See the [`MeteredConnector`] struct for more information. - -use std::sync::Arc; - -use actix::prelude::*; -use actix_web::client::{ClientConnector, ClientConnectorStats}; - -use relay_common::metric; -use relay_config::Config; - -use crate::metrics::{RelayCounters, RelayHistograms}; - -/// Actor that reports connection metrics. -/// -/// This actor implements a receiver for `ClientConnectorStats`, which provide periodic updates on -/// the client connector used to establish upstream connections. The numbers are reported into the -/// `connector.*` counter and historgram metrics. -/// -/// # Example -/// -/// To start the connector, use [`MeteredConnector::start`]. This also starts the actual client -/// connector and registers it as default system service. After that, each client request uses this -/// connector by default. -/// -/// ```ignore -/// use std::sync::Arc; -/// use relay_config::Config; -/// -/// let config = Arc::new(Config::default()); -/// MeteredConnector::start(config); -/// -/// let request = actix_web::client::get("http://example.org") -/// .finish(); -/// ``` -#[derive(Debug)] -pub struct MeteredConnector; - -impl MeteredConnector { - pub fn start(config: Arc) -> Addr { - let metered_connector = Self.start(); - - let connector = ClientConnector::default() - .limit(config.max_concurrent_requests()) - .stats(metered_connector.clone().recipient()) - .start(); - - // Register as default system service. - System::current().registry().set(connector); - - metered_connector - } -} - -impl Default for MeteredConnector { - fn default() -> Self { - Self - } -} - -impl Actor for MeteredConnector { - type Context = Context; - - fn started(&mut self, _context: &mut Self::Context) { - relay_log::info!("metered connector started"); - } - - fn stopped(&mut self, _context: &mut Self::Context) { - relay_log::info!("metered connector stopped"); - } -} - -impl Handler for MeteredConnector { - type Result = (); - - fn handle(&mut self, message: ClientConnectorStats, _context: &mut Self::Context) { - metric!(histogram(RelayHistograms::ConnectorWaitQueue) = message.wait_queue as u64); - metric!(counter(RelayCounters::ConnectorReused) += message.reused as i64); - metric!(counter(RelayCounters::ConnectorOpened) += message.opened as i64); - metric!(counter(RelayCounters::ConnectorClosed) += message.closed as i64); - metric!(counter(RelayCounters::ConnectorErrors) += message.errors as i64); - metric!(counter(RelayCounters::ConnectorTimeouts) += message.timeouts as i64); - } -} diff --git a/relay-server/src/actors/mod.rs b/relay-server/src/actors/mod.rs index e771420416..603ba940ce 100644 --- a/relay-server/src/actors/mod.rs +++ b/relay-server/src/actors/mod.rs @@ -32,7 +32,6 @@ //! .expect("failed to start relay"); //! ``` -pub mod connector; pub mod controller; pub mod envelopes; pub mod healthcheck; diff --git a/relay-server/src/metrics.rs b/relay-server/src/metrics.rs index a9e43ab650..2737e9d3c9 100644 --- a/relay-server/src/metrics.rs +++ b/relay-server/src/metrics.rs @@ -101,11 +101,6 @@ pub enum RelayHistograms { /// /// There is no limit to the number of cached projects. ProjectStateCacheSize, - /// The number of upstream requests queued up for a connection in the connection pool. - /// - /// Relay uses explicit queueing for most requests. This wait queue should almost always be - /// empty, and a large number of queued requests indicates a severe bug. - ConnectorWaitQueue, /// The number of upstream requests queued up for sending. /// /// Relay employs connection keep-alive whenever possible. Connections are kept open for _15_ @@ -147,7 +142,6 @@ impl HistogramMetric for RelayHistograms { RelayHistograms::ProjectStateRequestBatchSize => "project_state.request.batch_size", RelayHistograms::ProjectStateReceived => "project_state.received", RelayHistograms::ProjectStateCacheSize => "project_cache.size", - RelayHistograms::ConnectorWaitQueue => "connector.wait_queue", RelayHistograms::UpstreamMessageQueueSize => "http_queue.size", RelayHistograms::UpstreamRetries => "upstream.retries", } @@ -437,22 +431,6 @@ pub enum RelayCounters { /// be used to ingest events. Once the grace period expires, the cache is evicted and new /// requests wait for an update. EvictingStaleProjectCaches, - /// Number of requests that reused an already open upstream connection. - /// - /// Relay employs connection keep-alive whenever possible. Connections are kept open for _15_ - /// seconds of inactivity or _75_ seconds of activity. - ConnectorReused, - /// Number of upstream connections opened. - ConnectorOpened, - /// Number of upstream connections closed due to connection timeouts. - /// - /// Relay employs connection keep-alive whenever possible. Connections are kept open for _15_ - /// seconds of inactivity or _75_ seconds of activity. - ConnectorClosed, - /// Number of upstream connections that experienced errors. - ConnectorErrors, - /// Number of upstream connections that experienced a timeout. - ConnectorTimeouts, /// An event has been produced to Kafka for one of the configured "internal" projects. #[cfg(feature = "processing")] InternalCapturedEventStoreActor, @@ -484,11 +462,6 @@ impl CounterMetric for RelayCounters { RelayCounters::Requests => "requests", RelayCounters::ResponsesStatusCodes => "responses.status_codes", RelayCounters::EvictingStaleProjectCaches => "project_cache.eviction", - RelayCounters::ConnectorReused => "connector.reused", - RelayCounters::ConnectorOpened => "connector.opened", - RelayCounters::ConnectorClosed => "connector.closed", - RelayCounters::ConnectorErrors => "connector.errors", - RelayCounters::ConnectorTimeouts => "connector.timeouts", #[cfg(feature = "processing")] RelayCounters::InternalCapturedEventStoreActor => "internal.captured.event.store_actor", RelayCounters::InternalCapturedEventEndpoint => "internal.captured.event.endpoint", diff --git a/relay-server/src/service.rs b/relay-server/src/service.rs index e7c46716ba..9206fb80b2 100644 --- a/relay-server/src/service.rs +++ b/relay-server/src/service.rs @@ -11,7 +11,6 @@ use relay_common::clone; use relay_config::Config; use relay_redis::RedisPool; -use crate::actors::connector::MeteredConnector; use crate::actors::controller::{Configure, Controller}; use crate::actors::envelopes::EnvelopeManager; use crate::actors::healthcheck::Healthcheck; @@ -304,10 +303,6 @@ pub fn start(config: Config) -> Result, ServerErro shutdown_timeout: config.shutdown_timeout(), }); - // Start the connector before creating the ServiceState. The service state will spawn Arbiters - // that immediately start the authentication process. The connector must be available before. - MeteredConnector::start(config.clone()); - let state = ServiceState::start(config.clone())?; let mut server = server::new(move || make_app(state.clone())); server = server