ref: Replace organization id type with newtype

CHANGELOG.md

@@ -27,6 +27,7 @@
 - Add a field to suggest consumers to ingest spans in EAP. ([#4206](https://github.com/getsentry/relay/pull/4206))
 - Run internal worker threads with a lower priority. ([#4222](https://github.com/getsentry/relay/pull/4222))
 - Add additional fields to the `Event` `Getter`. ([#4238](https://github.com/getsentry/relay/pull/4238))
+- Replace u64 with `OrganizationId` new-type struct for organization id. ([#4159](https://github.com/getsentry/relay/pull/4159))
 
 ## 24.10.0
 

relay-base-schema/src/lib.rs

@@ -9,5 +9,6 @@
 pub mod data_category;
 pub mod events;
 pub mod metrics;
+pub mod organization;
 pub mod project;
 pub mod spans;

relay-base-schema/src/organization.rs

@@ -0,0 +1,47 @@
+//! Contains [`OrganizationId`] which is the ID of a Sentry organization and is currently a
+//! a wrapper over `u64`.
+
+use serde::{Deserialize, Serialize};
+
+/// The unique identifier of a Sentry organization.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Hash, Serialize, Deserialize)]
+pub struct OrganizationId(u64);
+
+impl OrganizationId {
+    /// Creates a new organization ID from its numeric value
+    #[inline]
+    pub fn new(id: u64) -> Self {
+        Self(id)
+    }
+
+    /// Returns the numeric value of the organization ID.
+    pub fn value(self) -> u64 {
+        self.0
+    }
+}
+
+impl std::fmt::Display for OrganizationId {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.value())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_deserialize() {
+        let json = r#"[42]"#;
+        let ids: Vec<OrganizationId> =
+            serde_json::from_str(json).expect("deserialize organization ids");
+        assert_eq!(ids, vec![OrganizationId::new(42)]);
+    }
+
+    #[test]
+    fn test_serialize() {
+        let ids = vec![OrganizationId::new(42)];
+        let json = serde_json::to_string(&ids).expect("serialize organization ids");
+        assert_eq!(json, r#"[42]"#);
+    }
+}

relay-cardinality/benches/redis_impl.rs

@@ -6,6 +6,7 @@ use std::{
 use criterion::{BatchSize, BenchmarkId, Criterion};
 use relay_base_schema::{
     metrics::{MetricName, MetricNamespace},
+    organization::OrganizationId,
     project::ProjectId,
 };
 use relay_cardinality::{
@@ -83,7 +84,7 @@ impl Params {
                 namespace: None,
             }],
             scoping: Scoping {
-                organization_id: 1,
+                organization_id: OrganizationId::new(1),
                 project_id: ProjectId::new(100),
             },
             rounds,

relay-cardinality/src/lib.rs

@@ -26,6 +26,3 @@ pub use self::window::SlidingWindow;
 /// Redis Set based cardinality limiter.
 #[cfg(feature = "redis")]
 pub type CardinalityLimiter = self::limiter::CardinalityLimiter<RedisSetLimiter>;
-
-/// Internal alias for better readability.
-type OrganizationId = u64;

relay-cardinality/src/limiter.rs

@@ -5,12 +5,13 @@ use std::collections::BTreeMap;
 
 use hashbrown::{HashMap, HashSet};
 use relay_base_schema::metrics::{MetricName, MetricNamespace, MetricType};
+use relay_base_schema::organization::OrganizationId;
 use relay_base_schema::project::ProjectId;
 use relay_common::time::UnixTimestamp;
 use relay_statsd::metric;
 
 use crate::statsd::CardinalityLimiterTimers;
-use crate::{CardinalityLimit, Error, OrganizationId, Result};
+use crate::{CardinalityLimit, Error, Result};
 
 /// Data scoping information.
 ///
@@ -394,7 +395,7 @@ mod tests {
 
     fn build_scoping() -> Scoping {
         Scoping {
-            organization_id: 1,
+            organization_id: OrganizationId::new(1),
             project_id: ProjectId::new(1),
         }
     }

relay-cardinality/src/redis/cache.rs

@@ -197,11 +197,12 @@ impl ScopedCache {
 #[cfg(test)]
 mod tests {
     use relay_base_schema::metrics::{MetricName, MetricNamespace};
+    use relay_base_schema::organization::OrganizationId;
     use relay_base_schema::project::ProjectId;
 
     use crate::limiter::{Entry, EntryId};
     use crate::redis::quota::PartialQuotaScoping;
-    use crate::{CardinalityLimit, CardinalityScope, OrganizationId, Scoping, SlidingWindow};
+    use crate::{CardinalityLimit, CardinalityScope, Scoping, SlidingWindow};
 
     use super::*;
 
@@ -238,7 +239,7 @@ mod tests {
             window_seconds: 100,
             granularity_seconds: 10,
         };
-        let scope = build_scoping(1, window);
+        let scope = build_scoping(OrganizationId::new(1), window);
         let now = UnixTimestamp::now();
         let future = now + Duration::from_secs(window.granularity_seconds + 1);
 
@@ -298,8 +299,8 @@ mod tests {
             window_seconds: 100,
             granularity_seconds: 10,
         };
-        let scope1 = build_scoping(1, window);
-        let scope2 = build_scoping(2, window);
+        let scope1 = build_scoping(OrganizationId::new(1), window);
+        let scope2 = build_scoping(OrganizationId::new(2), window);
 
         let now = UnixTimestamp::now();
 
@@ -336,8 +337,8 @@ mod tests {
             window_seconds: vacuum_interval.as_secs() * 10,
             granularity_seconds: vacuum_interval.as_secs() * 2,
         };
-        let scope1 = build_scoping(1, window);
-        let scope2 = build_scoping(2, window);
+        let scope1 = build_scoping(OrganizationId::new(1), window);
+        let scope2 = build_scoping(OrganizationId::new(2), window);
 
         let now = UnixTimestamp::now();
         let in_interval = now + Duration::from_secs(vacuum_interval.as_secs() - 1);

relay-cardinality/src/redis/limiter.rs

@@ -262,6 +262,7 @@ mod tests {
     use std::sync::atomic::AtomicU64;
 
     use relay_base_schema::metrics::{MetricName, MetricNamespace::*, MetricType};
+    use relay_base_schema::organization::OrganizationId;
     use relay_base_schema::project::ProjectId;
     use relay_redis::{redis, RedisConfigOptions};
 
@@ -293,7 +294,9 @@ mod tests {
         static ORGS: AtomicU64 = AtomicU64::new(100);
 
         let scoping = Scoping {
-            organization_id: ORGS.fetch_add(1, std::sync::atomic::Ordering::SeqCst),
+            organization_id: OrganizationId::new(
+                ORGS.fetch_add(1, std::sync::atomic::Ordering::SeqCst),
+            ),
             project_id: ProjectId::new(1),
         };
 
@@ -593,12 +596,12 @@ mod tests {
         let granularity_seconds = 10_000;
 
         let scoping1 = Scoping {
-            organization_id: granularity_seconds,
+            organization_id: OrganizationId::new(granularity_seconds),
             project_id: ProjectId::new(1),
         };
         let scoping2 = Scoping {
             // Time shift relative to `scoping1` should be half the granularity.
-            organization_id: granularity_seconds / 2,
+            organization_id: OrganizationId::new(granularity_seconds / 2),
             project_id: ProjectId::new(1),
         };
 

relay-cardinality/src/redis/quota.rs

@@ -3,13 +3,14 @@ use std::fmt::{self, Write};
 use std::hash::Hash;
 
 use relay_base_schema::metrics::{MetricName, MetricNamespace, MetricType};
+use relay_base_schema::organization::OrganizationId;
 use relay_base_schema::project::ProjectId;
 use relay_common::time::UnixTimestamp;
 
 use crate::limiter::Entry;
 use crate::redis::{KEY_PREFIX, KEY_VERSION};
 use crate::window::Slot;
-use crate::{CardinalityLimit, CardinalityScope, OrganizationId, Scoping, SlidingWindow};
+use crate::{CardinalityLimit, CardinalityScope, Scoping, SlidingWindow};
 
 /// A quota scoping extracted from a [`CardinalityLimit`] and a [`Scoping`].
 ///
@@ -70,7 +71,7 @@ impl PartialQuotaScoping {
     fn shifted(&self, timestamp: UnixTimestamp) -> UnixTimestamp {
         let shift = self
             .organization_id
-            .map(|o| o % self.window.granularity_seconds)
+            .map(|o| o.value() % self.window.granularity_seconds)
             .unwrap_or(0);
 
         UnixTimestamp::from_secs(timestamp.as_secs() + shift)
@@ -119,7 +120,7 @@ impl QuotaScoping {
 
     /// Turns the scoping into a Redis key for the passed slot.
     pub fn to_redis_key(&self, slot: Slot) -> String {
-        let organization_id = self.organization_id.unwrap_or(0);
+        let organization_id = self.organization_id.unwrap_or(OrganizationId::new(0));
         let project_id = self.project_id.map(|p| p.value()).unwrap_or(0);
         let namespace = self.namespace.map(|ns| ns.as_str()).unwrap_or("");
         let metric_type = DisplayOptMinus(self.metric_type);

relay-cardinality/src/redis/state.rs

@@ -162,10 +162,10 @@ impl<'a> Drop for LimitState<'a> {
             passive = passive,
         );
 
-        let organization_id = self.scoping.organization_id as i64;
+        let organization_id = self.scoping.organization_id;
         let status = if self.rejections > 0 { "limited" } else { "ok" };
         metric!(
-            set(CardinalityLimiterSets::Organizations) = organization_id,
+            set(CardinalityLimiterSets::Organizations) = organization_id.value() as i64,
             id = &self.cardinality_limit.id,
             passive = passive,
             status = status,

relay-quotas/src/global.rs

@@ -245,6 +245,7 @@ mod tests {
     use super::*;
 
     use relay_base_schema::data_category::DataCategory;
+    use relay_base_schema::organization::OrganizationId;
     use relay_base_schema::project::{ProjectId, ProjectKey};
     use relay_common::time::UnixTimestamp;
     use relay_redis::{RedisConfigOptions, RedisPool};
@@ -273,7 +274,7 @@ mod tests {
 
     fn build_scoping() -> Scoping {
         Scoping {
-            organization_id: 69420,
+            organization_id: OrganizationId::new(69420),
             project_id: ProjectId::new(42),
             project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
             key_id: Some(4711),

relay-quotas/src/quota.rs

@@ -2,6 +2,7 @@ use std::fmt;
 use std::str::FromStr;
 
 use relay_base_schema::metrics::MetricNamespace;
+use relay_base_schema::organization::OrganizationId;
 use relay_base_schema::project::{ProjectId, ProjectKey};
 use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
@@ -15,7 +16,7 @@ pub use relay_base_schema::data_category::DataCategory;
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct Scoping {
     /// The organization id.
-    pub organization_id: u64,
+    pub organization_id: OrganizationId,
 
     /// The project id.
     pub project_id: ProjectId,
@@ -129,7 +130,7 @@ impl ItemScoping<'_> {
     pub fn scope_id(&self, scope: QuotaScope) -> Option<u64> {
         match scope {
             QuotaScope::Global => None,
-            QuotaScope::Organization => Some(self.organization_id),
+            QuotaScope::Organization => Some(self.organization_id.value()),
             QuotaScope::Project => Some(self.project_id.value()),
             QuotaScope::Key => self.key_id,
             QuotaScope::Unknown => None,
@@ -726,7 +727,7 @@ mod tests {
         assert!(quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -751,7 +752,7 @@ mod tests {
         assert!(!quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -776,7 +777,7 @@ mod tests {
         assert!(quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -787,7 +788,7 @@ mod tests {
         assert!(!quota.matches(ItemScoping {
             category: DataCategory::Transaction,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -812,7 +813,7 @@ mod tests {
         assert!(!quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -837,7 +838,7 @@ mod tests {
         assert!(quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -848,7 +849,7 @@ mod tests {
         assert!(!quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 0,
+                organization_id: OrganizationId::new(0),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -873,7 +874,7 @@ mod tests {
         assert!(quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -884,7 +885,7 @@ mod tests {
         assert!(!quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(0),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -909,7 +910,7 @@ mod tests {
         assert!(quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(17),
@@ -920,7 +921,7 @@ mod tests {
         assert!(!quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: Some(0),
@@ -931,7 +932,7 @@ mod tests {
         assert!(!quota.matches(ItemScoping {
             category: DataCategory::Error,
             scoping: &Scoping {
-                organization_id: 42,
+                organization_id: OrganizationId::new(42),
                 project_id: ProjectId::new(21),
                 project_key: ProjectKey::parse("a94ae32be2584e0bbd7a4cbb95971fee").unwrap(),
                 key_id: None,