More wip

Cargo.toml

@@ -301,9 +301,15 @@ prettyplease = "0.2.16"
 proc-macro2 = "1.0"
 progenitor = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
 progenitor-client = { git = "https://github.com/oxidecomputer/progenitor", branch = "main" }
-bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "1e25649e8c2ac274bd04adfe0513dd14a482058c" }
-propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "1e25649e8c2ac274bd04adfe0513dd14a482058c" }
-propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "1e25649e8c2ac274bd04adfe0513dd14a482058c" }
+
+# TODO(ben): Put back to main before merge.
+#bhyve_api = { git = "https://github.com/oxidecomputer/propolis", rev = "1e25649e8c2ac274bd04adfe0513dd14a482058c" }
+#propolis-client = { git = "https://github.com/oxidecomputer/propolis", rev = "1e25649e8c2ac274bd04adfe0513dd14a482058c" }
+#propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", rev = "1e25649e8c2ac274bd04adfe0513dd14a482058c" }
+bhyve_api = { git = "https://github.com/oxidecomputer/propolis", branch = "vcpu-usage-stats" }
+propolis-client = { git = "https://github.com/oxidecomputer/propolis", branch = "vcpu-usage-stats" }
+propolis-mock-server = { git = "https://github.com/oxidecomputer/propolis", branch = "vcpu-usage-stats" }
+
 proptest = "1.4.0"
 quote = "1.0"
 rand = "0.8.5"

oximeter/instruments/src/kstat/virtual_machine.rs

@@ -25,6 +25,8 @@ use oximeter::Sample;
 use oximeter::Target;
 use uuid::Uuid;
 
+// TODO(ben): Add tests for changes to the kstat-based schema.
+
 /// A single virtual machine instance.
 #[derive(Clone, Debug)]
 pub struct VirtualMachine {

oximeter/producer/src/lib.rs

@@ -38,8 +38,8 @@ pub enum Error {
     #[error("Error running producer HTTP server: {0}")]
     Server(String),
 
-    #[error("Error registering as metric producer: {0}")]
-    RegistrationError(String),
+    #[error("Error registering as metric producer: {msg}")]
+    RegistrationError { retryable: bool, msg: String },
 
     #[error("Producer registry and config UUIDs do not match")]
     UuidMismatch,
@@ -251,11 +251,19 @@ pub async fn register(
 ) -> Result<(), Error> {
     let client =
         nexus_client::Client::new(&format!("http://{}", address), log.clone());
-    client
-        .cpapi_producers_post(&server_info.into())
-        .await
-        .map(|_| ())
-        .map_err(|msg| Error::RegistrationError(msg.to_string()))
+    client.cpapi_producers_post(&server_info.into()).await.map(|_| ()).map_err(
+        |err| {
+            let retryable = match &err {
+                nexus_client::Error::CommunicationError(..) => true,
+                nexus_client::Error::ErrorResponse(resp) => {
+                    resp.status().is_server_error()
+                }
+                _ => false,
+            };
+            let msg = err.to_string();
+            Error::RegistrationError { retryable, msg }
+        },
+    )
 }
 
 /// Handle a request to pull available metric data from a [`ProducerRegistry`].

sled-agent/src/instance.rs

@@ -9,8 +9,6 @@ use crate::common::instance::{
     PublishedVmmState,
 };
 use crate::instance_manager::{InstanceManagerServices, InstanceTicket};
-use crate::metrics::MetricsManager;
-use crate::metrics::INSTANCE_SAMPLE_INTERVAL;
 use crate::nexus::NexusClientWithResolver;
 use crate::params::ZoneBundleCause;
 use crate::params::ZoneBundleMetadata;
@@ -235,15 +233,8 @@ struct InstanceInner {
     // Object used to collect zone bundles from this instance when terminated.
     zone_bundler: ZoneBundler,
 
-    // Data used to start / stop collection of instance-related metrics.
-    tracking_instance_metrics: bool,
-    metrics_manager: MetricsManager,
-
     // Object representing membership in the "instance manager".
     instance_ticket: InstanceTicket,
-
-    // Metadata used to track statistics for this instance.
-    metadata: InstanceMetadata,
 }
 
 impl InstanceInner {
@@ -390,38 +381,7 @@ impl InstanceInner {
                 self.terminate().await?;
                 Ok(Reaction::Terminate)
             }
-            None => {
-                // TODO-robustness: If we fail to register the actual kstats for
-                // this instance, we should produce missing samples, rather than
-                // nothing at all.
-                //
-                // See https://github.com/oxidecomputer/omicron/issues/4863.
-                //
-                // TODO(ben) -- need to do this once, or ignore duplicate target
-                // error, not every time we get a state update from propolis.
-                if !self.tracking_instance_metrics {
-                    if let Err(e) = self
-                        .metrics_manager
-                        .track_instance(
-                            &self.id(),
-                            &self.metadata,
-                            self.properties.vcpus.into(),
-                            INSTANCE_SAMPLE_INTERVAL,
-                        )
-                        .await
-                    {
-                        error!(
-                            self.log,
-                            "failed to track instance metrics, \
-                            no samples will be produced";
-                            "error" => ?e,
-                        );
-                    } else {
-                        self.tracking_instance_metrics = true;
-                    }
-                }
-                Ok(Reaction::Continue)
-            }
+            None => Ok(Reaction::Continue),
         }
     }
 
@@ -583,18 +543,6 @@ impl InstanceInner {
             );
         }
 
-        // Stop tracking instance-related metrics.
-        if let Err(e) =
-            self.metrics_manager.stop_tracking_instance(self.id()).await
-        {
-            error!(
-                self.log,
-                "Failed to stop tracking instance metrics";
-                "instance_id" => %self.id(),
-                "error" => ?e,
-            );
-        }
-
         // Ensure that no zone exists. This succeeds even if no zone was ever
         // created.
         // NOTE: we call`Zones::halt_and_remove_logged` directly instead of
@@ -674,7 +622,6 @@ impl Instance {
             port_manager,
             storage,
             zone_bundler,
-            metrics_manager,
             zone_builder_factory,
         } = services;
 
@@ -716,6 +663,9 @@ impl Instance {
                 id,
                 name: hardware.properties.hostname.clone(),
                 description: "Test description".to_string(),
+                metadata: propolis_client::types::InstanceMetadata::from(
+                    metadata.clone(),
+                ),
                 image_id: Uuid::nil(),
                 bootrom_id: Uuid::nil(),
                 // TODO: Align the byte type w/propolis.
@@ -746,10 +696,7 @@ impl Instance {
             storage,
             zone_builder_factory,
             zone_bundler,
-            tracking_instance_metrics: false,
-            metrics_manager,
             instance_ticket: ticket,
-            metadata,
         };
 
         let inner = Arc::new(Mutex::new(instance));

sled-agent/src/instance_manager.rs

@@ -6,7 +6,6 @@
 
 use crate::instance::propolis_zone_name;
 use crate::instance::Instance;
-use crate::metrics::MetricsManager;
 use crate::nexus::NexusClientWithResolver;
 use crate::params::InstanceMetadata;
 use crate::params::ZoneBundleMetadata;
@@ -79,7 +78,6 @@ struct InstanceManagerInternal {
     port_manager: PortManager,
     storage: StorageHandle,
     zone_bundler: ZoneBundler,
-    metrics_manager: MetricsManager,
     zone_builder_factory: ZoneBuilderFactory,
 }
 
@@ -89,7 +87,6 @@ pub(crate) struct InstanceManagerServices {
     pub port_manager: PortManager,
     pub storage: StorageHandle,
     pub zone_bundler: ZoneBundler,
-    pub metrics_manager: MetricsManager,
     pub zone_builder_factory: ZoneBuilderFactory,
 }
 
@@ -100,15 +97,13 @@ pub struct InstanceManager {
 
 impl InstanceManager {
     /// Initializes a new [`InstanceManager`] object.
-    #[allow(clippy::too_many_arguments)]
     pub fn new(
         log: Logger,
         nexus_client: NexusClientWithResolver,
         etherstub: Etherstub,
         port_manager: PortManager,
         storage: StorageHandle,
         zone_bundler: ZoneBundler,
-        metrics_manager: MetricsManager,
         zone_builder_factory: ZoneBuilderFactory,
     ) -> Result<InstanceManager, Error> {
         Ok(InstanceManager {
@@ -123,7 +118,6 @@ impl InstanceManager {
                 port_manager,
                 storage,
                 zone_bundler,
-                metrics_manager,
                 zone_builder_factory,
             }),
         })
@@ -281,7 +275,6 @@ impl InstanceManager {
                     port_manager: self.inner.port_manager.clone(),
                     storage: self.inner.storage.clone(),
                     zone_bundler: self.inner.zone_bundler.clone(),
-                    metrics_manager: self.inner.metrics_manager.clone(),
                     zone_builder_factory: self
                         .inner
                         .zone_builder_factory

sled-agent/src/params.rs

@@ -88,6 +88,15 @@ pub struct InstanceMetadata {
     pub project_id: Uuid,
 }
 
+impl From<InstanceMetadata> for propolis_client::types::InstanceMetadata {
+    fn from(md: InstanceMetadata) -> propolis_client::types::InstanceMetadata {
+        propolis_client::types::InstanceMetadata {
+            silo_id: md.silo_id,
+            project_id: md.project_id,
+        }
+    }
+}
+
 /// The body of a request to ensure that a instance and VMM are known to a sled
 /// agent.
 #[derive(Serialize, Deserialize, JsonSchema)]

sled-agent/src/sim/sled_agent.rs

@@ -237,7 +237,7 @@ impl SledAgent {
         hardware: InstanceHardware,
         instance_runtime: InstanceRuntimeState,
         vmm_runtime: VmmRuntimeState,
-        _metadata: InstanceMetadata,
+        metadata: InstanceMetadata,
     ) -> Result<SledInstanceState, Error> {
         // respond with a fake 500 level failure if asked to ensure an instance
         // with more than 16 CPUs.
@@ -287,6 +287,7 @@ impl SledAgent {
                     id: propolis_id,
                     name: hardware.properties.hostname.clone(),
                     description: "sled-agent-sim created instance".to_string(),
+                    metadata: metadata.into(),
                     image_id: Uuid::default(),
                     bootrom_id: Uuid::default(),
                     memory: hardware.properties.memory.to_whole_mebibytes(),

sled-agent/src/sled_agent.rs

@@ -466,7 +466,6 @@ impl SledAgent {
             port_manager.clone(),
             storage_manager.clone(),
             long_running_task_handles.zone_bundler.clone(),
-            metrics_manager.clone(),
             ZoneBuilderFactory::default(),
         )?;