feat(flags++): added quota limiting to the new flags service and in doing so refactored out a bunch of common stuff

[dmarticus]

Problem

TODO – wanna see if this passes CI first.

[greptile-apps]

PR Summary

This PR refactors the codebase to implement quota limiting in the feature flags service by extracting common functionality into shared modules.

• Moved Redis client implementation from service-specific modules to rust/common/redis for consistent usage across services
• Relocated rate limiting functionality from capture service to rust/common/limiters with three submodules: overflow, redis, and token_dropper
• Added support for feature flag request limiting by introducing a FeatureFlags variant to the QuotaResource enum
• Updated import paths across multiple files to use the new common modules instead of service-specific implementations
• Integrated the shared RedisLimiter into the feature flags service router and server components for quota management

_{30 file(s) reviewed, 8 comment(s)
Edit PR Review Bot Settings | Greptile}

[greptile-apps]

style: There's an extra space after rand = { workspace = true} between 'true' and the closing brace. This should be rand = { workspace = true } for consistency with other workspace dependencies.

Suggested change

	rand = { workspace = true}
	rand = { workspace = true }

[greptile-apps]

style: The serde-pickle dependency is missing a space between the version and closing brace, unlike the other dependencies. This is a minor inconsistency in formatting.

[greptile-apps]

style: The file is missing a newline at the end, which is typically recommended for text files in Unix-like systems.

Suggested change

	tokio = { workspace = true }
	tokio = { workspace = true }

[greptile-apps]

logic: The return type for zrangebyscore is inconsistent with other methods - it uses anyhow::Result while other methods use Result<T, CustomRedisError>. This inconsistency could lead to confusion and different error handling patterns.

Suggested change

	async fn zrangebyscore(&self, k: String, min: String, max: String) -> Result<Vec<String>>;
	async fn zrangebyscore(&self, k: String, min: String, max: String) -> Result<Vec<String>, CustomRedisError>;

[greptile-apps]

style: This error handling logic for empty values is only applied to the get method, but not to other methods like hget. Consider applying consistent error handling for empty values across all methods.

[greptile-apps]

logic: The mock implementation of zrangebyscore returns anyhow::Error with a string message, while the real implementation returns various Redis errors. This inconsistency could cause tests to pass that would fail in production.

Suggested change

	async fn zrangebyscore(&self, key: String, _min: String, _max: String) -> Result<Vec<String>> {
	match self.zrangebyscore_ret.get(&key) {
	Some(val) => Ok(val.clone()),
	None => Err(anyhow::anyhow!("Not found")),
	}
	async fn zrangebyscore(&self, key: String, _min: String, _max: String) -> Result<Vec<String>> {
	match self.zrangebyscore_ret.get(&key) {
	Some(val) => Ok(val.clone()),
	None => Err(CustomRedisError::NotFound.into()),
	}

[greptile-apps]

style: The mock set implementation ignores the value parameter, which means tests won't catch bugs related to serialization of specific values.

[greptile-apps]

logic: Using .expect() here will cause the service to panic if Redis is unavailable. Consider handling this error more gracefully, similar to how other client initialization errors are handled above.

Suggested change

	let billing_limiter = RedisLimiter::new(
	Duration::seconds(5),
	redis_client.clone(),
	QUOTA_LIMITER_CACHE_KEY.to_string(),
	None,
	QuotaResource::FeatureFlags,
	)
	.expect("failed to create billing limiter");
	let billing_limiter = match RedisLimiter::new(
	Duration::seconds(5),
	redis_client.clone(),
	QUOTA_LIMITER_CACHE_KEY.to_string(),
	None,
	QuotaResource::FeatureFlags,
	) {
	Ok(limiter) => limiter,
	Err(e) => {
	tracing::error!("Failed to create billing limiter: {}", e);
	return;
	}
	};