Merge branch 'master' into doc/intra-doc-links

* master:
  ref(server): Remove legacy id fallback (#800)

Author: jan-auer

CHANGELOG.md

@@ -17,6 +17,7 @@
 **Internal**:
 
 - Project states are now cached separately per DSN public key instead of per project ID. This means that there will be multiple separate cache entries for projects with more than one DSN. ([#778](https://github.com/getsentry/relay/pull/778))
+- Relay no longer uses the Sentry endpoint to resolve project IDs by public key. Ingestion for the legacy store endpoint has been refactored to rely on key-based caches only. As a result, the legacy endpoint is supported only on managed Relays. ([#800](https://github.com/getsentry/relay/pull/800))
 
 ## 20.9.0
 

relay-common/src/project.rs

@@ -1,5 +1,6 @@
 use std::borrow::Cow;
 use std::fmt;
+use std::str::FromStr;
 
 use serde::{Deserialize, Serialize};
 
@@ -75,3 +76,11 @@ impl fmt::Display for ProjectKey {
         self.as_str().fmt(f)
     }
 }
+
+impl FromStr for ProjectKey {
+    type Err = ParseProjectKeyError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Self::parse(s)
+    }
+}

relay-server/src/actors/events.rs

@@ -12,7 +12,7 @@ use futures::prelude::*;
 use parking_lot::RwLock;
 use serde_json::Value as SerdeValue;
 
-use relay_common::{clone, metric, LogError};
+use relay_common::{clone, metric, LogError, ProjectId};
 use relay_config::{Config, HttpEncoding, RelayMode};
 use relay_general::pii::{PiiAttachmentsProcessor, PiiProcessor};
 use relay_general::processor::{process_value, ProcessingState};
@@ -92,6 +92,9 @@ enum ProcessingError {
     #[fail(display = "failed to resolve project information")]
     ProjectFailed(#[cause] ProjectError),
 
+    #[fail(display = "missing project id in DSN")]
+    MissingProjectId,
+
     #[fail(display = "invalid security report type")]
     InvalidSecurityType,
 
@@ -154,7 +157,8 @@ impl ProcessingError {
             | Self::ScheduleFailed(_)
             | Self::ProjectFailed(_)
             | Self::Timeout
-            | Self::ProcessingFailed(_) => Some(Outcome::Invalid(DiscardReason::Internal)),
+            | Self::ProcessingFailed(_)
+            | Self::MissingProjectId => Some(Outcome::Invalid(DiscardReason::Internal)),
             #[cfg(feature = "processing")]
             Self::StoreFailed(_) | Self::QuotasFailed(_) => {
                 Some(Outcome::Invalid(DiscardReason::Internal))
@@ -201,6 +205,13 @@ struct ProcessEnvelopeState {
     /// The state of the project that this envelope belongs to.
     project_state: Arc<ProjectState>,
 
+    /// The id of the project that this envelope is ingested into.
+    ///
+    /// This identifier can differ from the one stated in the Envelope's DSN if the key was moved to
+    /// a new project or on the legacy endpoint. In that case, normalization will update the project
+    /// ID.
+    project_id: ProjectId,
+
     /// UTC date time converted from the `start_time` instant.
     received_at: DateTime<Utc>,
 }
@@ -283,8 +294,8 @@ impl EventProcessor {
     fn process_sessions(&self, state: &mut ProcessEnvelopeState) -> Result<(), ProcessingError> {
         let envelope = &mut state.envelope;
         let received = state.received_at;
+        let project_id = state.project_id;
 
-        let project_id = envelope.meta().project_id().value();
         let clock_drift_processor =
             ClockDriftProcessor::new(envelope.sent_at(), received).at_least(MINIMUM_CLOCK_DRIFT);
         let client = envelope.meta().client().map(str::to_owned);
@@ -383,12 +394,30 @@ impl EventProcessor {
             envelope.set_retention(retention);
         }
 
+        // Prefer the project's project ID, and fall back to the stated project id from the
+        // envelope. The project ID is available in all modes, other than in proxy mode, where
+        // events for unknown projects are forwarded blindly.
+        //
+        // Neither ID can be available in proxy mode on the /store/ endpoint. This is not supported,
+        // since we cannot process an event without project ID, so drop it.
+        let project_id = project_state
+            .project_id
+            .or_else(|| envelope.meta().project_id())
+            .ok_or(ProcessingError::MissingProjectId)?;
+
+        // Ensure the project ID is updated to the stored instance for this project cache. This can
+        // differ in two cases:
+        //  1. The envelope was sent to the legacy `/store/` endpoint without a project ID.
+        //  2. The DSN was moved and the envelope sent to the old project ID.
+        envelope.meta_mut().set_project_id(project_id);
+
         Ok(ProcessEnvelopeState {
             envelope,
             event: Annotated::empty(),
             metrics: Metrics::default(),
             rate_limits: RateLimits::new(),
             project_state,
+            project_id,
             received_at: relay_common::instant_to_date_time(start_time),
         })
     }
@@ -885,7 +914,7 @@ impl EventProcessor {
         }
 
         let store_config = StoreConfig {
-            project_id: Some(envelope.meta().project_id().value()),
+            project_id: Some(state.project_id.value()),
             client_ip: envelope.meta().client_addr().map(IpAddr::from),
             client: envelope.meta().client().map(str::to_owned),
             key_id,
@@ -1361,7 +1390,6 @@ impl Handler<HandleEnvelope> for EventManager {
         } = message;
 
         let event_id = envelope.event_id();
-        let project_id = envelope.meta().project_id();
         let remote_addr = envelope.meta().client_addr();
 
         // Compute whether this envelope contains an event. This is used in error handling to
@@ -1371,13 +1399,15 @@ impl Handler<HandleEnvelope> for EventManager {
 
         let scoping = Rc::new(RefCell::new(envelope.meta().get_partial_scoping()));
 
-        metric!(set(RelaySets::UniqueProjects) = project_id.value() as i64);
-
         let future = project
             .send(CheckEnvelope::fetched(envelope))
             .map_err(ProcessingError::ScheduleFailed)
             .and_then(|result| result.map_err(ProcessingError::ProjectFailed))
             .and_then(clone!(scoping, |response| {
+                // Use the project id from the loaded project state to account for redirects.
+                let project_id = response.scoping.project_id.value();
+                metric!(set(RelaySets::UniqueProjects) = project_id as i64);
+
                 scoping.replace(response.scoping);
 
                 let checked = response.result.map_err(ProcessingError::EventRejected)?;
@@ -1452,6 +1482,7 @@ impl Handler<HandleEnvelope> for EventManager {
                 }
 
                 log::trace!("sending event to sentry endpoint");
+                let project_id = scoping.borrow().project_id;
                 let request = SendRequest::post(format!("/api/{}/envelope/", project_id)).build(
                     move |builder| {
                         // Override the `sent_at` timestamp. Since the event went through basic

relay-server/src/actors/mod.rs

@@ -46,7 +46,6 @@ pub mod healthcheck;
 pub mod outcome;
 pub mod project;
 pub mod project_cache;
-pub mod project_keys;
 pub mod project_local;
 pub mod project_upstream;
 pub mod relays;

relay-server/src/actors/project.rs

@@ -87,18 +87,12 @@ pub struct LimitedProjectConfig {
     pub datascrubbing_settings: DataScrubbingConfig,
 }
 
-/// Returns an invalid placeholder project identifier.
-fn default_project_id() -> ProjectId {
-    ProjectId::new(0)
-}
-
 /// The project state is a cached server state of a project.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ProjectState {
     /// Unique identifier of this project.
-    #[serde(default = "default_project_id")]
-    pub project_id: ProjectId,
+    pub project_id: Option<ProjectId>,
     /// The timestamp of when the state was last changed.
     ///
     /// This might be `None` in some rare cases like where states
@@ -137,7 +131,7 @@ pub struct ProjectState {
 #[derive(Debug, Serialize)]
 #[serde(rename_all = "camelCase", remote = "ProjectState")]
 pub struct LimitedProjectState {
-    pub project_id: ProjectId,
+    pub project_id: Option<ProjectId>,
     pub last_change: Option<DateTime<Utc>>,
     pub disabled: bool,
     #[serde(with = "limited_public_key_comfigs")]
@@ -152,7 +146,7 @@ impl ProjectState {
     /// Project state for a missing project.
     pub fn missing() -> Self {
         ProjectState {
-            project_id: ProjectId::new(0),
+            project_id: None,
             last_change: None,
             disabled: true,
             public_keys: SmallVec::new(),
@@ -200,9 +194,9 @@ impl ProjectState {
 
     /// Returns whether this state is outdated and needs to be refetched.
     pub fn outdated(&self, config: &Config) -> Outdated {
-        let expiry = match self.project_id.value() {
-            0 => config.cache_miss_expiry(),
-            _ => config.project_cache_expiry(),
+        let expiry = match self.project_id {
+            None => config.cache_miss_expiry(),
+            Some(_) => config.project_cache_expiry(),
         };
 
         let elapsed = self.last_fetch.elapsed();
@@ -223,9 +217,13 @@ impl ProjectState {
     /// Returns `true` if the given project ID matches this project.
     ///
     /// If the project state has not been loaded, this check is skipped because the project
-    /// identifier is not yet known.
-    pub fn is_valid_project_id(&self, project_id: ProjectId) -> bool {
-        self.project_id.value() == 0 || self.project_id == project_id
+    /// identifier is not yet known. Likewise, this check is skipped for the legacy store endpoint
+    /// which comes without a project ID. The id is later overwritten in `check_envelope`.
+    pub fn is_valid_project_id(&self, stated_id: Option<ProjectId>) -> bool {
+        match (self.project_id, stated_id) {
+            (Some(actual_id), Some(stated_id)) => actual_id == stated_id,
+            _ => true,
+        }
     }
 
     /// Checks if this origin is allowed for this project.
@@ -260,7 +258,7 @@ impl ProjectState {
             key_config.public_key == public_key
         } else {
             // Loaded states must have a key config, but ignore missing and invalid states.
-            self.project_id.value() == 0
+            self.project_id.is_none()
         }
     }
 
@@ -282,8 +280,8 @@ impl ProjectState {
         // The original project identifier is part of the DSN. If the DSN was moved to another
         // project, the actual project identifier is different and can be obtained from project
         // states. This is only possible when the project state has been loaded.
-        if self.project_id.value() != 0 {
-            scoping.project_id = self.project_id;
+        if let Some(project_id) = self.project_id {
+            scoping.project_id = project_id;
         }
 
         // This is a hack covering three cases:

relay-server/src/actors/project_cache.rs

@@ -173,6 +173,7 @@ pub struct FetchProjectState {
     pub public_key: ProjectKey,
 }
 
+#[derive(Debug)]
 pub struct ProjectStateResponse {
     pub state: Arc<ProjectState>,
     pub is_local: bool,