Remove unused base_route field from oximeter producer info (#5769)

- Remove from DB schema, add migration to drop the column
- Remove all references, except in the timeseries schema reporting
oximeter self-statistics. That needs to wait for functionality to update
timeseries schema.
- Fixes #5658

clients/nexus-client/src/lib.rs

@@ -232,7 +232,6 @@ impl From<&omicron_common::api::internal::nexus::ProducerEndpoint>
     ) -> Self {
         Self {
             address: s.address.to_string(),
-            base_route: s.base_route.clone(),
             id: s.id,
             kind: s.kind.into(),
             interval: s.interval.into(),
@@ -415,7 +414,6 @@ impl TryFrom<types::ProducerEndpoint>
             id: ep.id,
             kind: ep.kind.into(),
             address,
-            base_route: ep.base_route,
             interval: ep.interval.into(),
         })
     }

clients/oximeter-client/src/lib.rs

@@ -41,7 +41,6 @@ impl From<&omicron_common::api::internal::nexus::ProducerEndpoint>
     ) -> Self {
         Self {
             address: s.address.to_string(),
-            base_route: s.base_route.clone(),
             id: s.id,
             kind: s.kind.into(),
             interval: s.interval.into(),

common/src/api/internal/nexus.rs

@@ -110,20 +110,10 @@ pub struct ProducerEndpoint {
     /// The IP address and port at which `oximeter` can collect metrics from the
     /// producer.
     pub address: SocketAddr,
-    /// NOTE: This field is deprecated, and will be ignored. It will be removed
-    /// in future releases.
-    pub base_route: String,
     /// The interval on which `oximeter` should collect metrics.
     pub interval: Duration,
 }
 
-impl ProducerEndpoint {
-    /// Return the route that can be used to request metric data.
-    pub fn collection_route(&self) -> String {
-        format!("{}/{}", &self.base_route, &self.id)
-    }
-}
-
 /// Response to a successful producer registration.
 #[derive(Clone, Debug, Deserialize, JsonSchema, PartialEq, Serialize)]
 pub struct ProducerRegistrationResponse {

dev-tools/omdb/src/bin/omdb/oximeter.rs

@@ -90,7 +90,6 @@ impl OximeterArgs {
 struct Producer {
     id: Uuid,
     address: SocketAddr,
-    base_route: String,
     interval: String,
 }
 
@@ -100,7 +99,6 @@ impl From<ProducerEndpoint> for Producer {
         Self {
             id: p.id,
             address: p.address.parse().unwrap(),
-            base_route: p.base_route,
             interval: humantime::format_duration(interval).to_string(),
         }
     }

nexus/db-model/src/producer_endpoint.rs

@@ -53,7 +53,6 @@ impl From<ProducerEndpoint> for internal::nexus::ProducerEndpoint {
             id: ep.id(),
             kind: ep.kind.into(),
             address: SocketAddr::new(ep.ip.ip(), *ep.port),
-            base_route: ep.base_route.clone(),
             interval: Duration::from_secs_f64(ep.interval),
         }
     }
@@ -71,7 +70,6 @@ pub struct ProducerEndpoint {
     pub ip: ipnetwork::IpNetwork,
     pub port: SqlU16,
     pub interval: f64,
-    pub base_route: String,
     pub oximeter_id: Uuid,
 }
 
@@ -87,14 +85,8 @@ impl ProducerEndpoint {
             kind: endpoint.kind.into(),
             ip: endpoint.address.ip().into(),
             port: endpoint.address.port().into(),
-            base_route: endpoint.base_route.clone(),
             interval: endpoint.interval.as_secs_f64(),
             oximeter_id,
         }
     }
-
-    /// Return the route that can be used to request metric data.
-    pub fn collection_route(&self) -> String {
-        format!("{}/{}", &self.base_route, self.id())
-    }
 }

nexus/db-model/src/schema.rs

@@ -463,7 +463,6 @@ table! {
         ip -> Inet,
         port -> Int4,
         interval -> Float8,
-        base_route -> Text,
         oximeter_id -> Uuid,
     }
 }

nexus/db-model/src/schema_versions.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(62, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(63, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(63, "remove-producer-base-route-column"),
         KnownVersion::new(62, "allocate-subnet-decommissioned-sleds"),
         KnownVersion::new(61, "blueprint-add-sled-state"),
         KnownVersion::new(60, "add-lookup-vmm-by-sled-id-index"),

nexus/db-queries/src/db/datastore/oximeter.rs

@@ -109,7 +109,6 @@ impl DataStore {
                 dsl::ip.eq(producer.ip),
                 dsl::port.eq(producer.port),
                 dsl::interval.eq(producer.interval),
-                dsl::base_route.eq(producer.base_route.clone()),
             ))
             .execute_async(&*self.pool_connection_authorized(opctx).await?)
             .await
@@ -297,7 +296,6 @@ mod tests {
                 id: Uuid::new_v4(),
                 kind: nexus::ProducerKind::Service,
                 address: "[::1]:0".parse().unwrap(), // unused
-                base_route: "/".to_string(),         // unused
                 interval: Duration::from_secs(0),    // unused
             },
             collector_info.id,

nexus/metrics-producer-gc/src/lib.rs

@@ -244,7 +244,6 @@ mod tests {
                 id: Uuid::new_v4(),
                 kind: nexus::ProducerKind::Service,
                 address: "[::1]:0".parse().unwrap(), // unused
-                base_route: "/".to_string(),         // unused
                 interval: Duration::from_secs(0),    // unused
             },
             collector_info.id,
@@ -327,7 +326,6 @@ mod tests {
                 id: Uuid::new_v4(),
                 kind: nexus::ProducerKind::Service,
                 address: "[::1]:0".parse().unwrap(), // unused
-                base_route: "/".to_string(),         // unused
                 interval: Duration::from_secs(0),    // unused
             },
             collector_info.id,

nexus/src/app/background/metrics_producer_gc.rs

@@ -190,7 +190,6 @@ mod tests {
                 id: Uuid::new_v4(),
                 kind: nexus::ProducerKind::Service,
                 address: "[::1]:0".parse().unwrap(), // unused
-                base_route: "/".to_string(),         // unused
                 interval: Duration::from_secs(0),    // unused
             },
             collector_info.id,

nexus/src/lib.rs

@@ -468,8 +468,6 @@ fn start_producer_server(
             id: registry.producer_id(),
             kind: ProducerKind::Service,
             address,
-            // NOTE: This is now unused, and will be removed in the future.
-            base_route: String::new(),
             interval: std::time::Duration::from_secs(10),
         },
         // Some(_) here prevents DNS resolution, using our own address to

nexus/test-utils/src/lib.rs

@@ -1468,7 +1468,6 @@ pub fn start_producer_server(
         id,
         kind: ProducerKind::Service,
         address: producer_address,
-        base_route: String::new(), // Unused, will be removed.
         interval: Duration::from_secs(1),
     };
     let config = oximeter_producer::Config {

openapi/nexus-internal.json

@@ -3894,10 +3894,6 @@
             "description": "The IP address and port at which `oximeter` can collect metrics from the producer.",
             "type": "string"
           },
-          "base_route": {
-            "description": "NOTE: This field is deprecated, and will be ignored. It will be removed in future releases.",
-            "type": "string"
-          },
           "id": {
             "description": "A unique ID for this producer.",
             "type": "string",
@@ -3922,7 +3918,6 @@
         },
         "required": [
           "address",
-          "base_route",
           "id",
           "interval",
           "kind"

openapi/oximeter.json

@@ -200,10 +200,6 @@
             "description": "The IP address and port at which `oximeter` can collect metrics from the producer.",
             "type": "string"
           },
-          "base_route": {
-            "description": "NOTE: This field is deprecated, and will be ignored. It will be removed in future releases.",
-            "type": "string"
-          },
           "id": {
             "description": "A unique ID for this producer.",
             "type": "string",
@@ -228,7 +224,6 @@
         },
         "required": [
           "address",
-          "base_route",
           "id",
           "interval",
           "kind"

oximeter/collector/src/agent.rs

@@ -80,11 +80,7 @@ async fn perform_collection(
 ) {
     debug!(log, "collecting from producer");
     let res = client
-        .get(format!(
-            "http://{}{}",
-            producer.address,
-            producer.collection_route()
-        ))
+        .get(format!("http://{}/{}", producer.address, producer.id,))
         .send()
         .await;
     match res {
@@ -155,10 +151,7 @@ async fn collection_task(
     mut inbox: mpsc::Receiver<CollectionMessage>,
     outbox: mpsc::Sender<(Option<CollectionToken>, ProducerResults)>,
 ) {
-    let mut log = orig_log.new(o!(
-        "route" => producer.collection_route(),
-        "address" => producer.address,
-    ));
+    let mut log = orig_log.new(o!("address" => producer.address));
     let client = reqwest::Client::new();
     let mut collection_timer = interval(producer.interval);
     collection_timer.tick().await; // completes immediately
@@ -199,10 +192,7 @@ async fn collection_task(
                         );
 
                         // Update the logger with the new information as well.
-                        log = orig_log.new(o!(
-                            "route" => producer.collection_route(),
-                            "address" => producer.address,
-                        ));
+                        log = orig_log.new(o!("address" => producer.address));
                         collection_timer = interval(producer.interval);
                         collection_timer.tick().await; // completes immediately
                     }
@@ -940,7 +930,6 @@ mod tests {
             id: Uuid::new_v4(),
             kind: ProducerKind::Service,
             address,
-            base_route: String::from("/"),
             interval: COLLECTION_INTERVAL,
         };
         collector
@@ -1009,7 +998,6 @@ mod tests {
                 0,
                 0,
             )),
-            base_route: String::from("/"),
             interval: COLLECTION_INTERVAL,
         };
         collector
@@ -1089,7 +1077,6 @@ mod tests {
             id: Uuid::new_v4(),
             kind: ProducerKind::Service,
             address,
-            base_route: String::from("/"),
             interval: COLLECTION_INTERVAL,
         };
         collector

oximeter/collector/src/self_stats.rs

@@ -46,6 +46,10 @@ pub struct Collections {
     /// The base route in the producer server used to collect metrics.
     ///
     /// The full route is `{base_route}/{producer_id}`.
+    ///
+    // TODO-cleanup: This is no longer relevant, but removing it entirely
+    // relies on nonexistent functionality for updating timeseries schema. When
+    // that lands, we should remove this.
     pub base_route: String,
     pub datum: Cumulative<u64>,
 }
@@ -98,6 +102,10 @@ pub struct FailedCollections {
     /// The base route in the producer server used to collect metrics.
     ///
     /// The full route is `{base_route}/{producer_id}`.
+    ///
+    // TODO-cleanup: This is no longer relevant, but removing it entirely
+    // relies on nonexistent functionality for updating timeseries schema. When
+    // that lands, we should remove this.
     pub base_route: String,
     /// The reason we could not collect.
     //
@@ -125,7 +133,7 @@ impl CollectionTaskStats {
                 producer_id: producer.id,
                 producer_ip: producer.address.ip(),
                 producer_port: producer.address.port(),
-                base_route: producer.base_route.clone(),
+                base_route: String::new(),
                 datum: Cumulative::new(0),
             },
             failed_collections: BTreeMap::new(),
@@ -203,7 +211,7 @@ mod tests {
             producer_id: uuid::uuid!("718452ab-7cca-42f6-b8b1-1aaaa1b09104"),
             producer_ip: IpAddr::V6(Ipv6Addr::LOCALHOST),
             producer_port: 12345,
-            base_route: String::from("/"),
+            base_route: String::new(),
             datum: Cumulative::new(0),
         }
     }
@@ -213,7 +221,7 @@ mod tests {
             producer_id: uuid::uuid!("718452ab-7cca-42f6-b8b1-1aaaa1b09104"),
             producer_ip: IpAddr::V6(Ipv6Addr::LOCALHOST),
             producer_port: 12345,
-            base_route: String::from("/"),
+            base_route: String::new(),
             reason: FailureReason::Unreachable.as_string(),
             datum: Cumulative::new(0),
         }

oximeter/producer/examples/producer.rs

@@ -120,7 +120,6 @@ async fn main() -> anyhow::Result<()> {
         id: registry.producer_id(),
         kind: ProducerKind::Service,
         address: args.address,
-        base_route: "/collect".to_string(),
         interval: Duration::from_secs(10),
     };
     let config = Config {

oximeter/producer/src/lib.rs

@@ -210,14 +210,6 @@ impl Server {
             return Err(Error::UuidMismatch);
         }
 
-        // Overwrite any provided base_route.
-        //
-        // This will be removed in future releases, as users no longer have or
-        // need any control over the route the producer server exposes.
-        //
-        // TODO-cleanup: Remove this field entirely.
-        server_info.base_route = String::from("/");
-
         // Build the logger / server.
         let log = Self::build_logger(log)?;
         let dropshot = ConfigDropshot {
@@ -284,7 +276,6 @@ impl Server {
         info!(
             log,
             "starting oximeter metric producer server";
-            "route" => server_info.collection_route(),
             "producer_id" => ?registry.producer_id(),
             "address" => server.local_addr(),
             "interval" => ?server_info.interval,
@@ -542,7 +533,6 @@ mod tests {
                 id: Uuid::new_v4(),
                 kind: ProducerKind::Service,
                 address,
-                base_route: String::new(),
                 interval: Duration::from_secs(10),
             },
             registration_address: Some(fake_nexus.local_addr()),

schema/crdb/dbinit.sql

@@ -1294,8 +1294,6 @@ CREATE TABLE IF NOT EXISTS omicron.public.metric_producer (
     ip INET NOT NULL,
     port INT4 CHECK (port BETWEEN 0 AND 65535) NOT NULL,
     interval FLOAT NOT NULL,
-    /* TODO: Is this length appropriate? */
-    base_route STRING(512) NOT NULL,
     /* Oximeter collector instance to which this metric producer is assigned. */
     oximeter_id UUID NOT NULL
 );
@@ -3861,7 +3859,7 @@ INSERT INTO omicron.public.db_metadata (
     version,
     target_version
 ) VALUES
-    (TRUE, NOW(), NOW(), '62.0.0', NULL)
+    (TRUE, NOW(), NOW(), '63.0.0', NULL)
 ON CONFLICT DO NOTHING;
 
 COMMIT;

schema/crdb/remove-producer-base-route-column/up.sql

@@ -0,0 +1 @@
+ALTER TABLE omicron.public.metric_producer DROP COLUMN IF EXISTS base_route;

sled-agent/src/metrics.rs

@@ -169,7 +169,6 @@ fn start_producer_server(
             id: registry.producer_id(),
             kind: ProducerKind::SledAgent,
             address,
-            base_route: String::new(), // Unused, will be removed.
             interval: METRIC_COLLECTION_INTERVAL,
         },
         registration_address,

sled-agent/src/sim/disk.rs

@@ -170,7 +170,6 @@ impl SimDisk {
             id,
             kind: ProducerKind::SledAgent,
             address: producer_address,
-            base_route: String::new(), // Unused, will be removed.
             interval: Duration::from_millis(200),
         };
         let config = oximeter_producer::Config {