feat: harden community production foundation through story 1.5
This commit is contained in:
@@ -1,7 +1,5 @@
|
||||
use std::{
|
||||
collections::HashMap,
|
||||
env,
|
||||
num::ParseIntError,
|
||||
sync::Arc,
|
||||
time::{Duration, Instant},
|
||||
};
|
||||
@@ -21,7 +19,6 @@ use tokio::sync::RwLock;
|
||||
pub struct RuntimeCacheConfig {
|
||||
pub backend: CacheBackend,
|
||||
pub url: Option<String>,
|
||||
pub default_ttl_ms: Option<u64>,
|
||||
}
|
||||
|
||||
impl Default for RuntimeCacheConfig {
|
||||
@@ -29,26 +26,22 @@ impl Default for RuntimeCacheConfig {
|
||||
Self {
|
||||
backend: CacheBackend::Memory,
|
||||
url: None,
|
||||
default_ttl_ms: None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl RuntimeCacheConfig {
|
||||
pub fn from_env() -> Result<Self, RuntimeCacheConfigError> {
|
||||
let backend = parse_backend()?;
|
||||
let url = parse_optional_string("CRANK_CACHE_URL")?;
|
||||
let default_ttl_ms = parse_optional_u64("CRANK_CACHE_DEFAULT_TTL_MS")?;
|
||||
|
||||
pub fn try_new(
|
||||
backend: CacheBackend,
|
||||
url: Option<String>,
|
||||
) -> Result<Self, RuntimeCacheConfigError> {
|
||||
if backend.is_external() && url.is_none() {
|
||||
return Err(RuntimeCacheConfigError::MissingUrl { backend });
|
||||
}
|
||||
|
||||
Ok(Self {
|
||||
backend,
|
||||
url,
|
||||
default_ttl_ms,
|
||||
})
|
||||
if !backend.is_external() && url.is_some() {
|
||||
return Err(RuntimeCacheConfigError::UnexpectedUrl);
|
||||
}
|
||||
Ok(Self { backend, url })
|
||||
}
|
||||
}
|
||||
|
||||
@@ -137,17 +130,11 @@ impl RedisCacheStore {
|
||||
url: &str,
|
||||
) -> Result<Self, RuntimeCacheStoreInitError> {
|
||||
let client =
|
||||
Client::open(url).map_err(|source| RuntimeCacheStoreInitError::InvalidUrl {
|
||||
backend,
|
||||
url: url.to_owned(),
|
||||
details: source.to_string(),
|
||||
})?;
|
||||
let connection_manager = client.get_connection_manager().await.map_err(|source| {
|
||||
RuntimeCacheStoreInitError::ConnectFailed {
|
||||
backend,
|
||||
details: source.to_string(),
|
||||
}
|
||||
})?;
|
||||
Client::open(url).map_err(|_| RuntimeCacheStoreInitError::InvalidUrl { backend })?;
|
||||
let connection_manager = client
|
||||
.get_connection_manager()
|
||||
.await
|
||||
.map_err(|_| RuntimeCacheStoreInitError::ConnectFailed { backend })?;
|
||||
Ok(Self {
|
||||
backend,
|
||||
connection_manager,
|
||||
@@ -774,86 +761,20 @@ fn scoped_key(scope: CacheScope, key: &str) -> String {
|
||||
format!("{scope:?}:{key}")
|
||||
}
|
||||
|
||||
fn parse_backend() -> Result<CacheBackend, RuntimeCacheConfigError> {
|
||||
match env::var("CRANK_CACHE_BACKEND") {
|
||||
Ok(raw) => raw
|
||||
.parse::<CacheBackend>()
|
||||
.map_err(|source| RuntimeCacheConfigError::InvalidBackend { value: raw, source }),
|
||||
Err(env::VarError::NotPresent) => Ok(CacheBackend::Memory),
|
||||
Err(env::VarError::NotUnicode(_)) => Err(RuntimeCacheConfigError::InvalidUnicode {
|
||||
name: "CRANK_CACHE_BACKEND",
|
||||
}),
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_optional_string(name: &'static str) -> Result<Option<String>, RuntimeCacheConfigError> {
|
||||
match env::var(name) {
|
||||
Ok(raw) => {
|
||||
let trimmed = raw.trim();
|
||||
if trimmed.is_empty() {
|
||||
Ok(None)
|
||||
} else {
|
||||
Ok(Some(trimmed.to_owned()))
|
||||
}
|
||||
}
|
||||
Err(env::VarError::NotPresent) => Ok(None),
|
||||
Err(env::VarError::NotUnicode(_)) => Err(RuntimeCacheConfigError::InvalidUnicode { name }),
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_optional_u64(name: &'static str) -> Result<Option<u64>, RuntimeCacheConfigError> {
|
||||
match env::var(name) {
|
||||
Ok(raw) => {
|
||||
let trimmed = raw.trim();
|
||||
if trimmed.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
let value = trimmed
|
||||
.parse::<u64>()
|
||||
.map_err(|source| RuntimeCacheConfigError::InvalidTtl { value: raw, source })?;
|
||||
if value == 0 {
|
||||
return Err(RuntimeCacheConfigError::ZeroTtl { name });
|
||||
}
|
||||
Ok(Some(value))
|
||||
}
|
||||
Err(env::VarError::NotPresent) => Ok(None),
|
||||
Err(env::VarError::NotUnicode(_)) => Err(RuntimeCacheConfigError::InvalidUnicode { name }),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
pub enum RuntimeCacheConfigError {
|
||||
#[error("{name} must contain valid UTF-8")]
|
||||
InvalidUnicode { name: &'static str },
|
||||
#[error("CRANK_CACHE_BACKEND must be one of memory, valkey, redis, got {value}")]
|
||||
InvalidBackend {
|
||||
value: String,
|
||||
source: crank_core::ParseCacheBackendError,
|
||||
},
|
||||
#[error("CRANK_CACHE_DEFAULT_TTL_MS must be a positive integer, got {value}")]
|
||||
InvalidTtl {
|
||||
value: String,
|
||||
source: ParseIntError,
|
||||
},
|
||||
#[error("{name} must be greater than zero")]
|
||||
ZeroTtl { name: &'static str },
|
||||
#[error("{backend} backend requires CRANK_CACHE_URL")]
|
||||
MissingUrl { backend: CacheBackend },
|
||||
#[error("memory cache backend does not accept CRANK_CACHE_URL")]
|
||||
UnexpectedUrl,
|
||||
}
|
||||
|
||||
#[derive(Debug, Error, PartialEq, Eq)]
|
||||
pub enum RuntimeCacheStoreInitError {
|
||||
#[error("{backend} backend requires CRANK_CACHE_URL")]
|
||||
MissingUrl { backend: CacheBackend },
|
||||
#[error("invalid {backend} cache url {url}: {details}")]
|
||||
InvalidUrl {
|
||||
backend: CacheBackend,
|
||||
url: String,
|
||||
details: String,
|
||||
},
|
||||
#[error("failed to connect to {backend} cache backend: {details}")]
|
||||
ConnectFailed {
|
||||
backend: CacheBackend,
|
||||
details: String,
|
||||
},
|
||||
#[error("invalid {backend} cache url")]
|
||||
InvalidUrl { backend: CacheBackend },
|
||||
#[error("failed to connect to {backend} cache backend")]
|
||||
ConnectFailed { backend: CacheBackend },
|
||||
}
|
||||
|
||||
@@ -11,7 +11,9 @@ use crank_metrics::{
|
||||
ToolOutcome, record_cache_outcome, record_confirmation_outcome, record_idempotency_outcome,
|
||||
record_limit_rejection,
|
||||
};
|
||||
use crank_trace::{ErrorCategory, Stage, StageOutcome};
|
||||
use crank_trace::{
|
||||
ErrorCategory, Stage, StageOutcome, set_parent_from_trace_context, trace_context_for_span,
|
||||
};
|
||||
use serde_json::{Map, Value, json};
|
||||
use time::OffsetDateTime;
|
||||
use tokio::sync::{OwnedSemaphorePermit, Semaphore};
|
||||
@@ -242,11 +244,22 @@ impl RuntimeExecutor {
|
||||
&self,
|
||||
request: RuntimeExecutionRequest<'_>,
|
||||
) -> Result<Value, RuntimeError> {
|
||||
log_runtime_event("unary.execute", request.operation, request.request_context);
|
||||
let runtime_span = Stage::RuntimeExecute.span();
|
||||
if let Some(context) = request.request_context {
|
||||
set_parent_from_trace_context(&runtime_span, &context.trace_context);
|
||||
}
|
||||
let generated_context = request.request_context.is_none().then(|| {
|
||||
RuntimeRequestContext::new(
|
||||
crank_core::RequestId::generate(),
|
||||
trace_context_for_span(&runtime_span)
|
||||
.unwrap_or_else(crank_core::TraceContext::generate),
|
||||
)
|
||||
});
|
||||
let request_context = request.request_context.or(generated_context.as_ref());
|
||||
log_runtime_event("unary.execute", request.operation, request_context);
|
||||
let started_at = Instant::now();
|
||||
let invocation_metrics =
|
||||
ToolInvocationMetrics::start(metric_invocation_source(request.request_context));
|
||||
let runtime_span = Stage::RuntimeExecute.span();
|
||||
ToolInvocationMetrics::start(metric_invocation_source(request_context));
|
||||
let result = async {
|
||||
let _permit = self.acquire_unary_permit(request.operation)?;
|
||||
let _inflight = InFlightGuard::runtime();
|
||||
@@ -261,7 +274,7 @@ impl RuntimeExecutor {
|
||||
request.operation,
|
||||
request.input,
|
||||
prepared_request,
|
||||
request.request_context,
|
||||
request_context,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -274,13 +287,8 @@ impl RuntimeExecutor {
|
||||
Err(error) => (ToolOutcome::Error, runtime_error_kind(error)),
|
||||
};
|
||||
invocation_metrics.complete(outcome, error_kind);
|
||||
self.record_metering(
|
||||
request.operation,
|
||||
request.request_context,
|
||||
&result,
|
||||
started_at,
|
||||
)
|
||||
.await;
|
||||
self.record_metering(request.operation, request_context, &result, started_at)
|
||||
.await;
|
||||
result
|
||||
}
|
||||
|
||||
@@ -829,7 +837,7 @@ fn log_runtime_event(
|
||||
operation_id = operation.operation_id.as_str(),
|
||||
protocol,
|
||||
request_id = context.request_id.as_str(),
|
||||
correlation_id = context.correlation_id.as_str(),
|
||||
trace_id = context.trace_context.trace_id().as_str(),
|
||||
"runtime execution"
|
||||
);
|
||||
} else {
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
use std::sync::Arc;
|
||||
|
||||
use crank_adapter_rest::{OutboundHttpPolicy, RestAdapter, RestAdapterError};
|
||||
use crank_adapter_rest::{OutboundHttpPolicy, RestAdapter};
|
||||
use crank_core::{
|
||||
AdapterRegistry, CoordinationStateStore, MeteringSink, NoopMeteringSink, ResponseCacheStore,
|
||||
SharedMeteringSink, SharedProtocolAdapter,
|
||||
@@ -74,11 +74,6 @@ pub fn community_default() -> RuntimeExecutorBuilder {
|
||||
.register_adapter(Arc::new(RestAdapter::new()) as SharedProtocolAdapter)
|
||||
}
|
||||
|
||||
pub fn community_from_env() -> Result<RuntimeExecutorBuilder, RestAdapterError> {
|
||||
Ok(RuntimeExecutorBuilder::new()
|
||||
.register_adapter(Arc::new(RestAdapter::from_env()?) as SharedProtocolAdapter))
|
||||
}
|
||||
|
||||
pub fn community_with_outbound_policy(policy: OutboundHttpPolicy) -> RuntimeExecutorBuilder {
|
||||
RuntimeExecutorBuilder::new()
|
||||
.register_adapter(Arc::new(RestAdapter::with_policy(policy)) as SharedProtocolAdapter)
|
||||
|
||||
@@ -27,7 +27,7 @@ pub use crank_adapter_rest::OutboundHttpPolicy;
|
||||
pub use error::RuntimeError;
|
||||
pub use executor::{RuntimeExecutionRequest, RuntimeExecutor};
|
||||
pub use executor_builder::{
|
||||
RuntimeExecutorBuilder, community_default, community_from_env, community_with_outbound_policy,
|
||||
RuntimeExecutorBuilder, community_default, community_with_outbound_policy,
|
||||
};
|
||||
pub use limits::{RuntimeLimits, RuntimeLimitsConfigError};
|
||||
pub use model::{AdapterResponse, PreparedRequest, RuntimeOperation};
|
||||
|
||||
@@ -1,9 +1,8 @@
|
||||
use std::{env, num::ParseIntError};
|
||||
|
||||
use thiserror::Error;
|
||||
|
||||
const DEFAULT_MAX_CONCURRENT_UNARY: usize = 64;
|
||||
const DEFAULT_MAX_CONCURRENT_SESSIONS: usize = 16;
|
||||
const MAX_CONCURRENCY: usize = 65_535;
|
||||
|
||||
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
|
||||
pub struct RuntimeLimits {
|
||||
@@ -21,52 +20,31 @@ impl Default for RuntimeLimits {
|
||||
}
|
||||
|
||||
impl RuntimeLimits {
|
||||
pub fn from_env() -> Result<Self, RuntimeLimitsConfigError> {
|
||||
pub fn try_new(
|
||||
max_concurrent_unary: usize,
|
||||
max_concurrent_sessions: usize,
|
||||
) -> Result<Self, RuntimeLimitsConfigError> {
|
||||
if !(1..=MAX_CONCURRENCY).contains(&max_concurrent_unary) {
|
||||
return Err(RuntimeLimitsConfigError::OutOfRange {
|
||||
field: "runtime.max_concurrent_unary",
|
||||
});
|
||||
}
|
||||
if !(1..=MAX_CONCURRENCY).contains(&max_concurrent_sessions) {
|
||||
return Err(RuntimeLimitsConfigError::OutOfRange {
|
||||
field: "runtime.max_concurrent_sessions",
|
||||
});
|
||||
}
|
||||
Ok(Self {
|
||||
max_concurrent_unary: parse_limit(
|
||||
"CRANK_RUNTIME_MAX_CONCURRENT_UNARY",
|
||||
DEFAULT_MAX_CONCURRENT_UNARY,
|
||||
)?,
|
||||
max_concurrent_sessions: parse_limit(
|
||||
"CRANK_RUNTIME_MAX_CONCURRENT_SESSIONS",
|
||||
DEFAULT_MAX_CONCURRENT_SESSIONS,
|
||||
)?,
|
||||
max_concurrent_unary,
|
||||
max_concurrent_sessions,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_limit(name: &'static str, default: usize) -> Result<usize, RuntimeLimitsConfigError> {
|
||||
match env::var(name) {
|
||||
Ok(raw) => {
|
||||
let value =
|
||||
raw.parse::<usize>()
|
||||
.map_err(|source| RuntimeLimitsConfigError::InvalidValue {
|
||||
name,
|
||||
value: raw,
|
||||
source,
|
||||
})?;
|
||||
if value == 0 {
|
||||
return Err(RuntimeLimitsConfigError::ZeroValue { name });
|
||||
}
|
||||
Ok(value)
|
||||
}
|
||||
Err(env::VarError::NotPresent) => Ok(default),
|
||||
Err(env::VarError::NotUnicode(_)) => Err(RuntimeLimitsConfigError::InvalidUnicode { name }),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Error)]
|
||||
#[derive(Debug, Error, Eq, PartialEq)]
|
||||
pub enum RuntimeLimitsConfigError {
|
||||
#[error("{name} must contain valid UTF-8")]
|
||||
InvalidUnicode { name: &'static str },
|
||||
#[error("{name} must be a positive integer, got {value}")]
|
||||
InvalidValue {
|
||||
name: &'static str,
|
||||
value: String,
|
||||
source: ParseIntError,
|
||||
},
|
||||
#[error("{name} must be greater than zero")]
|
||||
ZeroValue { name: &'static str },
|
||||
#[error("runtime limit is outside its allowed bounds: {field}")]
|
||||
OutOfRange { field: &'static str },
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -76,28 +54,17 @@ mod tests {
|
||||
#[test]
|
||||
fn defaults_are_positive() {
|
||||
let limits = RuntimeLimits::default();
|
||||
|
||||
assert!(limits.max_concurrent_unary > 0);
|
||||
assert!(limits.max_concurrent_sessions > 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_zero_limit_values() {
|
||||
unsafe {
|
||||
std::env::set_var("CRANK_RUNTIME_MAX_CONCURRENT_UNARY", "0");
|
||||
}
|
||||
|
||||
let error = RuntimeLimits::from_env().unwrap_err();
|
||||
|
||||
assert!(matches!(
|
||||
error,
|
||||
RuntimeLimitsConfigError::ZeroValue {
|
||||
name: "CRANK_RUNTIME_MAX_CONCURRENT_UNARY"
|
||||
fn value_constructor_rejects_zero() {
|
||||
assert_eq!(
|
||||
RuntimeLimits::try_new(0, 1).unwrap_err(),
|
||||
RuntimeLimitsConfigError::OutOfRange {
|
||||
field: "runtime.max_concurrent_unary"
|
||||
}
|
||||
));
|
||||
|
||||
unsafe {
|
||||
std::env::remove_var("CRANK_RUNTIME_MAX_CONCURRENT_UNARY");
|
||||
}
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
use std::collections::BTreeMap;
|
||||
|
||||
use crank_core::{AgentId, InvocationSource, WorkspaceId};
|
||||
use crank_core::{
|
||||
AgentId, CorrelationContext, InvocationSource, RequestId, TraceContext, WorkspaceId,
|
||||
};
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct ResponseCacheScope {
|
||||
@@ -10,8 +12,8 @@ pub struct ResponseCacheScope {
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct RuntimeRequestContext {
|
||||
pub request_id: String,
|
||||
pub correlation_id: String,
|
||||
pub request_id: RequestId,
|
||||
pub trace_context: TraceContext,
|
||||
pub response_cache_scope: Option<ResponseCacheScope>,
|
||||
pub metering_context: Option<MeteringContext>,
|
||||
pub confirmation_token: Option<String>,
|
||||
@@ -26,10 +28,10 @@ pub struct MeteringContext {
|
||||
}
|
||||
|
||||
impl RuntimeRequestContext {
|
||||
pub fn new(request_id: impl Into<String>, correlation_id: impl Into<String>) -> Self {
|
||||
pub fn new(request_id: RequestId, trace_context: TraceContext) -> Self {
|
||||
Self {
|
||||
request_id: request_id.into(),
|
||||
correlation_id: correlation_id.into(),
|
||||
request_id,
|
||||
trace_context,
|
||||
response_cache_scope: None,
|
||||
metering_context: None,
|
||||
confirmation_token: None,
|
||||
@@ -39,13 +41,31 @@ impl RuntimeRequestContext {
|
||||
|
||||
pub fn from_request_id(request_id: impl Into<String>) -> Self {
|
||||
let request_id = request_id.into();
|
||||
Self::new(request_id.clone(), request_id)
|
||||
Self::new(
|
||||
RequestId::resolve(Some(&request_id)),
|
||||
TraceContext::generate(),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn from_correlation(context: &CorrelationContext) -> Self {
|
||||
Self::new(
|
||||
context.request_id().clone(),
|
||||
context.trace_context().clone(),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn outbound_headers(&self) -> BTreeMap<String, String> {
|
||||
BTreeMap::from([
|
||||
("x-request-id".to_owned(), self.request_id.clone()),
|
||||
("x-correlation-id".to_owned(), self.correlation_id.clone()),
|
||||
("x-request-id".to_owned(), self.request_id.to_string()),
|
||||
(
|
||||
"x-trace-id".to_owned(),
|
||||
self.trace_context.trace_id().to_string(),
|
||||
),
|
||||
(
|
||||
"traceparent".to_owned(),
|
||||
self.trace_context.traceparent().to_owned(),
|
||||
),
|
||||
("x-correlation-id".to_owned(), self.request_id.to_string()),
|
||||
])
|
||||
}
|
||||
|
||||
@@ -124,7 +144,7 @@ impl From<&RuntimeRequestContext> for crank_core::RuntimeRequestContext {
|
||||
fn from(value: &RuntimeRequestContext) -> Self {
|
||||
Self {
|
||||
request_id: value.request_id.clone(),
|
||||
correlation_id: value.correlation_id.clone(),
|
||||
trace_context: value.trace_context.clone(),
|
||||
response_cache_scope: value.response_cache_scope.as_ref().map(Into::into),
|
||||
metering_context: value.metering_context.as_ref().map(Into::into),
|
||||
}
|
||||
@@ -158,11 +178,11 @@ mod tests {
|
||||
use super::RuntimeRequestContext;
|
||||
|
||||
#[test]
|
||||
fn uses_request_id_for_default_correlation_id() {
|
||||
fn generates_a_separate_default_trace_identity() {
|
||||
let context = RuntimeRequestContext::from_request_id("req_123");
|
||||
|
||||
assert_eq!(context.request_id, "req_123");
|
||||
assert_eq!(context.correlation_id, "req_123");
|
||||
assert_eq!(context.request_id.as_str(), "req_123");
|
||||
assert_ne!(context.trace_context.trace_id().as_str(), "req_123");
|
||||
assert!(context.response_cache_scope.is_none());
|
||||
assert!(context.confirmation_token.is_none());
|
||||
assert!(!context.approval_granted());
|
||||
|
||||
Reference in New Issue
Block a user