Skip to content

Commit 31a9534

Browse files
authored
fix(telemetry): bound compute driver categories
Signed-off-by: Drew Newberry <anewberry@nvidia.com>
1 parent 4201fb5 commit 31a9534

7 files changed

Lines changed: 113 additions & 147 deletions

File tree

architecture/build.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ OpenShell builds these main artifacts:
1010

1111
| Artifact | Source |
1212
|---|---|
13-
| Gateway binary | `crates/openshell-server` |
13+
| Gateway binary | `crates/openshell-gateway` |
1414
| CLI package and Python SDK | `python/openshell` plus Rust binaries where packaged |
1515
| TypeScript SDK package | `sdk/typescript` |
1616
| Gateway container image | `deploy/docker/Dockerfile.gateway` |

crates/openshell-core/src/telemetry.rs

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -159,20 +159,35 @@ impl SandboxTemplateSource {
159159
}
160160
}
161161

162-
#[derive(Debug, Clone, PartialEq, Eq)]
163-
pub struct TelemetryComputeDriver(String);
162+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
163+
pub struct TelemetryComputeDriver(&'static str);
164164

165165
impl TelemetryComputeDriver {
166166
#[must_use]
167-
pub fn as_str(&self) -> &str {
168-
&self.0
167+
pub const fn as_str(self) -> &'static str {
168+
self.0
169169
}
170170

171+
/// Classify an unregistered compute driver without exposing its configured
172+
/// name.
171173
#[must_use]
172-
pub fn from_raw(raw: &str) -> Self {
173-
let name = crate::config::normalize_compute_driver_name(raw)
174-
.unwrap_or_else(|_| "unknown".to_string());
175-
Self(name)
174+
pub const fn custom() -> Self {
175+
Self("custom")
176+
}
177+
178+
/// Define a bounded, anonymous category at a binary composition boundary.
179+
///
180+
/// The category must be a static operational label. Never construct it
181+
/// from user input, configuration, resource names, or other runtime data.
182+
#[must_use]
183+
pub const fn anonymous_category(category: &'static str) -> Self {
184+
Self(category)
185+
}
186+
}
187+
188+
impl Default for TelemetryComputeDriver {
189+
fn default() -> Self {
190+
Self::custom()
176191
}
177192
}
178193

@@ -657,21 +672,11 @@ mod tests {
657672
}
658673

659674
#[test]
660-
fn compute_driver_values_are_normalized_without_enumerating_backends() {
661-
assert_eq!(TelemetryComputeDriver::from_raw("alpha").as_str(), "alpha");
662-
assert_eq!(
663-
TelemetryComputeDriver::from_raw(" Alpha ").as_str(),
664-
"alpha"
665-
);
666-
assert_eq!(
667-
TelemetryComputeDriver::from_raw("CUSTOM_BACKEND").as_str(),
668-
"custom_backend"
669-
);
670-
assert_eq!(TelemetryComputeDriver::from_raw("beta").as_str(), "beta");
671-
assert_eq!(TelemetryComputeDriver::from_raw("gamma").as_str(), "gamma");
675+
fn compute_driver_values_are_bounded_by_the_composition_boundary() {
676+
assert_eq!(TelemetryComputeDriver::custom().as_str(), "custom");
672677
assert_eq!(
673-
TelemetryComputeDriver::from_raw("private-driver").as_str(),
674-
"private-driver"
678+
TelemetryComputeDriver::anonymous_category("first_party").as_str(),
679+
"first_party"
675680
);
676681
}
677682

@@ -760,7 +765,7 @@ mod disabled_tests {
760765
1,
761766
false,
762767
SandboxTemplateSource::Default,
763-
TelemetryComputeDriver::from_raw("test-driver"),
768+
TelemetryComputeDriver::custom(),
764769
);
765770
emit_policy_decision(
766771
PolicyDecisionOperation::Approve,

crates/openshell-gateway/BUILD.bazel

Lines changed: 0 additions & 64 deletions
This file was deleted.

crates/openshell-gateway/src/lib.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))]
1010
mod vm;
1111

12+
#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))]
13+
use openshell_core::telemetry::TelemetryComputeDriver;
1214
#[cfg(all(not(target_os = "windows"), feature = "in-tree-compute-drivers"))]
1315
use openshell_server::ComputeDriverRegistration;
1416
use openshell_server::ComputeDriverRegistry;
@@ -34,6 +36,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) {
3436
)
3537
.map(|registration| {
3638
registration
39+
.with_telemetry_category(TelemetryComputeDriver::anonymous_category("kubernetes"))
3740
.without_mtls_user_auth()
3841
.with_inherited_config_keys(&[
3942
"namespace",
@@ -54,6 +57,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) {
5457
)
5558
.map(|registration| {
5659
registration
60+
.with_telemetry_category(TelemetryComputeDriver::anonymous_category("podman"))
5761
.with_local_singleplayer()
5862
.with_tracing_setup(podman_tracing_setup)
5963
.with_inherited_config_keys(&[
@@ -73,6 +77,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) {
7377
)
7478
.map(|registration| {
7579
registration
80+
.with_telemetry_category(TelemetryComputeDriver::anonymous_category("docker"))
7681
.with_local_singleplayer()
7782
.with_inherited_config_keys(&[
7883
"sandbox_namespace",
@@ -86,6 +91,7 @@ fn install_in_tree_compute_drivers(registry: &mut ComputeDriverRegistry) {
8691
}),
8792
ComputeDriverRegistration::new("vm", u16::MAX, None, VmFactory).map(|registration| {
8893
registration
94+
.with_telemetry_category(TelemetryComputeDriver::anonymous_category("vm"))
8995
.with_local_singleplayer()
9096
.with_inherited_config_keys(&[
9197
"default_image",
@@ -108,18 +114,13 @@ fn podman_tracing_setup(
108114
let (provider, error) = openshell_driver_podman::otel_tracing::provider_for(otlp_endpoint);
109115
let layer = provider.as_ref().map(|provider| {
110116
let layer: openshell_server::ComputeDriverTracingLayer = Box::new(
111-
openshell_driver_podman::otel_tracing::in_process_layer(
112-
provider,
113-
),
117+
openshell_driver_podman::otel_tracing::in_process_layer(provider),
114118
);
115119
layer
116120
});
117121
let shutdown = provider.map(|provider| {
118-
let shutdown: openshell_server::ComputeDriverTracingShutdown = Box::new(move || {
119-
provider
120-
.shutdown()
121-
.map_err(|error| error.to_string())
122-
});
122+
let shutdown: openshell_server::ComputeDriverTracingShutdown =
123+
Box::new(move || provider.shutdown().map_err(|error| error.to_string()));
123124
shutdown
124125
});
125126
openshell_server::ComputeDriverTracingSetup::new(

crates/openshell-server/src/compute/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ use openshell_core::proto::{
3636
PlatformEvent, Sandbox, SandboxCondition, SandboxPhase, SandboxSpec, SandboxStatus,
3737
SandboxTemplate, ServiceEndpoint, SshSession,
3838
};
39+
use openshell_core::telemetry::TelemetryComputeDriver;
3940
use openshell_core::{ObjectLabels, ObjectWorkspace};
4041
use prost::Message;
4142
use std::collections::HashMap;
@@ -535,6 +536,7 @@ impl ComputeDriver for RemoteComputeDriver {
535536
pub struct ComputeRuntime {
536537
driver: TracedDriver,
537538
driver_info: ComputeDriverInfoSnapshot,
539+
telemetry_compute_driver: TelemetryComputeDriver,
538540
driver_process: Option<Arc<ManagedDriverProcess>>,
539541
default_image: String,
540542
store: Arc<Store>,
@@ -651,6 +653,7 @@ impl ComputeRuntime {
651653
Ok(Self {
652654
driver: TracedDriver::new(driver, driver_name),
653655
driver_info,
656+
telemetry_compute_driver: TelemetryComputeDriver::custom(),
654657
driver_process,
655658
default_image,
656659
store,
@@ -727,6 +730,20 @@ impl ComputeRuntime {
727730
&self.driver_info.name
728731
}
729732

733+
#[must_use]
734+
pub(crate) fn telemetry_compute_driver(&self) -> TelemetryComputeDriver {
735+
self.telemetry_compute_driver
736+
}
737+
738+
#[must_use]
739+
pub(crate) fn with_telemetry_compute_driver(
740+
mut self,
741+
telemetry_compute_driver: TelemetryComputeDriver,
742+
) -> Self {
743+
self.telemetry_compute_driver = telemetry_compute_driver;
744+
self
745+
}
746+
730747
#[must_use]
731748
pub(crate) fn gateway_listener_requirements(&self) -> &[GatewayListenerRequirement] {
732749
&self.gateway_listener_requirements
@@ -3995,6 +4012,7 @@ pub async fn new_test_runtime_with_driver(
39954012
driver_version: "test".to_string(),
39964013
gateway_manages_lifecycle: false,
39974014
},
4015+
telemetry_compute_driver: TelemetryComputeDriver::custom(),
39984016
driver_process: None,
39994017
default_image: "openshell/sandbox:test".to_string(),
40004018
store,
@@ -4679,6 +4697,7 @@ mod tests {
46794697
driver_version: "test".to_string(),
46804698
gateway_manages_lifecycle: false,
46814699
},
4700+
telemetry_compute_driver: TelemetryComputeDriver::custom(),
46824701
driver_process: None,
46834702
default_image: "openshell/sandbox:test".to_string(),
46844703
store,

crates/openshell-server/src/grpc/sandbox.rs

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ use openshell_core::proto::{
2929
};
3030
use openshell_core::proto::{Sandbox, SandboxPhase, SandboxTemplate, SshSession};
3131
use openshell_core::telemetry::{
32-
LifecycleOperation, LifecycleResource, SandboxTemplateSource, TelemetryComputeDriver,
33-
TelemetryOutcome,
32+
LifecycleOperation, LifecycleResource, SandboxTemplateSource, TelemetryOutcome,
3433
};
3534
use openshell_core::{ObjectId, ObjectName, ObjectWorkspace};
3635
use prost::Message;
@@ -171,7 +170,7 @@ fn emit_sandbox_create_telemetry(
171170
request: &CreateSandboxRequest,
172171
outcome: TelemetryOutcome,
173172
) {
174-
let compute_driver = telemetry_compute_driver(state.compute.configured_driver_name());
173+
let compute_driver = state.compute.telemetry_compute_driver();
175174
let Some(spec) = request.spec.as_ref() else {
176175
openshell_core::telemetry::emit_sandbox_create(
177176
outcome,
@@ -204,10 +203,6 @@ fn emit_sandbox_create_telemetry(
204203
);
205204
}
206205

207-
fn telemetry_compute_driver(driver_name: &str) -> TelemetryComputeDriver {
208-
TelemetryComputeDriver::from_raw(driver_name)
209-
}
210-
211206
async fn handle_create_sandbox_inner(
212207
state: &Arc<ServerState>,
213208
request: Request<CreateSandboxRequest>,
@@ -2407,30 +2402,6 @@ mod tests {
24072402

24082403
// ---- shell_escape ----
24092404

2410-
#[test]
2411-
fn telemetry_compute_driver_uses_resolved_driver_kind() {
2412-
assert_eq!(
2413-
telemetry_compute_driver("docker"),
2414-
TelemetryComputeDriver::from_raw("docker")
2415-
);
2416-
assert_eq!(
2417-
telemetry_compute_driver("kubernetes"),
2418-
TelemetryComputeDriver::from_raw("kubernetes")
2419-
);
2420-
assert_eq!(
2421-
telemetry_compute_driver("podman"),
2422-
TelemetryComputeDriver::from_raw("podman")
2423-
);
2424-
assert_eq!(
2425-
telemetry_compute_driver("vm"),
2426-
TelemetryComputeDriver::from_raw("vm")
2427-
);
2428-
assert_eq!(
2429-
telemetry_compute_driver(""),
2430-
TelemetryComputeDriver::from_raw("")
2431-
);
2432-
}
2433-
24342405
#[test]
24352406
fn shell_escape_safe_chars_pass_through() {
24362407
assert_eq!(shell_escape("ls").unwrap(), "ls");

0 commit comments

Comments
 (0)