Skip to content

Commit

Permalink
Define timeseries schema in TOML
Browse files Browse the repository at this point in the history
This is the first commit in a thread of work to support updates to
timeseries schema. In our first step, we're moving the definition of
timeseries from the Rust structs we use today, to TOML text files. Each
file describes one target and all the metrics associated with it, from
which we generate exactly the same Rust code that it replaces.

This opens the door to a lot of downstream work, including well-defined
updates to schema; detection of conflicting schema; improved metadata
for timeseries; and generation of documentation. As an example, this
moves exactly one (1) timeseries into the new format, the physical
datalink statistics tracked through illumos kstats.

- Move the `oximeter` crate into a private implementation crate, and
  re-export it at its original path
- Add intermediate representation of timeseries schema, and code for
  deserializing TOML timeseries definitions, checking them for
  conflicts, and handling updates. Note that **no actual updates** are
  currently supported. The ingesting code includes a check that version
  numbers are exactly 1, except in tests. We include tests that check
  updates in a number of ways, but until the remainder of the work is
  done, we limit timeseries in the wild to their current version of 1.
- Add a macro for consuming timeseries definitions in TOML, and
  generating the equivalent Rust code. Developers should use this new
  `oximeter::use_timeseries!()` proc macro to create new timeseries.
- Add an integration test in Nexus that pulls in timeseries schema
  ingested with with `oximeter::use_timeseries!()`, and checks them all
  for conflicts. As developers add new timeseries in TOML format, they
  must be added here as well to ensure we detect problems at CI time.
- Updates `kstat-rs` dep to simplify platform-specific code. This is
  part of this commit because we need the definitions of the
  physical-data-link timeseries for our integration test, but those were
  previously only compiled on illumos platforms. They're now included
  everywhere, while the tests remain illumos-only.
  • Loading branch information
bnaecker committed Jun 13, 2024
1 parent e751be2 commit 0933ece
Show file tree
Hide file tree
Showing 42 changed files with 2,888 additions and 531 deletions.
265 changes: 154 additions & 111 deletions Cargo.lock

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ members = [
"nexus/types",
"oximeter/collector",
"oximeter/db",
"oximeter/impl",
"oximeter/instruments",
"oximeter/oximeter-macro-impl",
"oximeter/oximeter",
"oximeter/producer",
"oximeter/timeseries-macro",
"package",
"passwords",
"rpaths",
Expand Down Expand Up @@ -141,10 +143,12 @@ default-members = [
"nexus/types",
"oximeter/collector",
"oximeter/db",
"oximeter/impl",
"oximeter/instruments",
"oximeter/oximeter-macro-impl",
"oximeter/oximeter",
"oximeter/producer",
"oximeter/timeseries-macro",
"package",
"passwords",
"rpaths",
Expand Down Expand Up @@ -371,9 +375,11 @@ oximeter = { path = "oximeter/oximeter" }
oximeter-client = { path = "clients/oximeter-client" }
oximeter-db = { path = "oximeter/db/" }
oximeter-collector = { path = "oximeter/collector" }
oximeter-impl = { path = "oximeter/impl" }
oximeter-instruments = { path = "oximeter/instruments" }
oximeter-macro-impl = { path = "oximeter/oximeter-macro-impl" }
oximeter-producer = { path = "oximeter/producer" }
oximeter-timeseries-macro = { path = "oximeter/timeseries-macro" }
p256 = "0.13"
parse-display = "0.9.0"
partial-io = { version = "0.5.4", features = ["proptest1", "tokio1"] }
Expand Down
2 changes: 1 addition & 1 deletion nexus/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ rcgen.workspace = true
regex.workspace = true
similar-asserts.workspace = true
sp-sim.workspace = true
rustls = { workspace = true }
rustls.workspace = true
subprocess.workspace = true
term.workspace = true
trust-dns-resolver.workspace = true
Expand Down
1 change: 1 addition & 0 deletions nexus/tests/integration_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ mod sp_updater;
mod ssh_keys;
mod subnet_allocation;
mod switch_port;
mod timeseries_schema;
mod unauthorized;
mod unauthorized_coverage;
mod updates;
Expand Down
38 changes: 38 additions & 0 deletions nexus/tests/integration_tests/timeseries_schema.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use std::collections::BTreeMap;

/// This test checks that changes to timeseries schema are all consistent.
///
/// Timeseries schema are described in a TOML format that makes it relatively
/// easy to add new versions of the timeseries. Those definitions are ingested
/// at compile-time and checked for self-consistency, but it's still possible
/// for two unrelated definitions to conflict. This test catches those.
///
/// As developers define new timeseries, those should be added to this test for
/// checking. To define metrics in their own crates, developers use the
/// proc-macro `oximeter::use_timeseries!()`
#[test]
fn timeseries_schema_consistency() {
let mut all_schema = BTreeMap::new();
for list in
[oximeter_instruments::kstat::link::physical_data_link::timeseries_schema()]
{
for schema in list {
let key = (schema.timeseries_name.clone(), schema.version);
if let Some(dup) = all_schema.insert(key, schema.clone()) {
assert_eq!(
dup, schema,
"Timeseries '{}' version {} is duplicated, but the \
schema themselves differ. This will lead to a conflict \
at registration time, so at least one of the schema \
should be renamed, re-versioned, or include different \
fields to differentiate them.",
dup.timeseries_name, dup.version,
);
};
}
}
}
10 changes: 10 additions & 0 deletions openapi/bootstrap-agent.json
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@
"checker": {
"nullable": true,
"description": "Checker to apply to incoming messages.",
"default": null,
"type": "string"
},
"originate": {
Expand All @@ -340,6 +341,7 @@
"shaper": {
"nullable": true,
"description": "Shaper to apply to outgoing messages.",
"default": null,
"type": "string"
}
},
Expand Down Expand Up @@ -437,25 +439,29 @@
"local_pref": {
"nullable": true,
"description": "Apply a local preference to routes received from this peer.",
"default": null,
"type": "integer",
"format": "uint32",
"minimum": 0
},
"md5_auth_key": {
"nullable": true,
"description": "Use the given key for TCP-MD5 authentication with the peer.",
"default": null,
"type": "string"
},
"min_ttl": {
"nullable": true,
"description": "Require messages from a peer have a minimum IP time to live field.",
"default": null,
"type": "integer",
"format": "uint8",
"minimum": 0
},
"multi_exit_discriminator": {
"nullable": true,
"description": "Apply the provided multi-exit discriminator (MED) updates sent to the peer.",
"default": null,
"type": "integer",
"format": "uint32",
"minimum": 0
Expand All @@ -467,13 +473,15 @@
"remote_asn": {
"nullable": true,
"description": "Require that a peer has a specified ASN.",
"default": null,
"type": "integer",
"format": "uint32",
"minimum": 0
},
"vlan_id": {
"nullable": true,
"description": "Associate a VLAN ID with a BGP peer session.",
"default": null,
"type": "integer",
"format": "uint16",
"minimum": 0
Expand Down Expand Up @@ -1192,6 +1200,7 @@
"vlan_id": {
"nullable": true,
"description": "The VLAN id associated with this route.",
"default": null,
"type": "integer",
"format": "uint16",
"minimum": 0
Expand Down Expand Up @@ -1234,6 +1243,7 @@
"vlan_id": {
"nullable": true,
"description": "The VLAN id (if any) associated with this address.",
"default": null,
"type": "integer",
"format": "uint16",
"minimum": 0
Expand Down
10 changes: 10 additions & 0 deletions openapi/nexus-internal.json
Original file line number Diff line number Diff line change
Expand Up @@ -1567,6 +1567,7 @@
"checker": {
"nullable": true,
"description": "Checker to apply to incoming messages.",
"default": null,
"type": "string"
},
"originate": {
Expand All @@ -1579,6 +1580,7 @@
"shaper": {
"nullable": true,
"description": "Shaper to apply to outgoing messages.",
"default": null,
"type": "string"
}
},
Expand Down Expand Up @@ -1676,25 +1678,29 @@
"local_pref": {
"nullable": true,
"description": "Apply a local preference to routes received from this peer.",
"default": null,
"type": "integer",
"format": "uint32",
"minimum": 0
},
"md5_auth_key": {
"nullable": true,
"description": "Use the given key for TCP-MD5 authentication with the peer.",
"default": null,
"type": "string"
},
"min_ttl": {
"nullable": true,
"description": "Require messages from a peer have a minimum IP time to live field.",
"default": null,
"type": "integer",
"format": "uint8",
"minimum": 0
},
"multi_exit_discriminator": {
"nullable": true,
"description": "Apply the provided multi-exit discriminator (MED) updates sent to the peer.",
"default": null,
"type": "integer",
"format": "uint32",
"minimum": 0
Expand All @@ -1706,13 +1712,15 @@
"remote_asn": {
"nullable": true,
"description": "Require that a peer has a specified ASN.",
"default": null,
"type": "integer",
"format": "uint32",
"minimum": 0
},
"vlan_id": {
"nullable": true,
"description": "Associate a VLAN ID with a BGP peer session.",
"default": null,
"type": "integer",
"format": "uint16",
"minimum": 0
Expand Down Expand Up @@ -4257,6 +4265,7 @@
"vlan_id": {
"nullable": true,
"description": "The VLAN id associated with this route.",
"default": null,
"type": "integer",
"format": "uint16",
"minimum": 0
Expand Down Expand Up @@ -4899,6 +4908,7 @@
"vlan_id": {
"nullable": true,
"description": "The VLAN id (if any) associated with this address.",
"default": null,
"type": "integer",
"format": "uint16",
"minimum": 0
Expand Down
Loading

0 comments on commit 0933ece

Please sign in to comment.