Add switch/sled metadata to ddm/mgd related timeseries #6955
Conversation
Includes: - package-manifest update to match the properties added to dpd - removal of client call to get sled idenifier information (no longer used) - switch_slot added to oximeter schemas for dendrite/switch-data-link
Related to oxidecomputer/dendrite#1033. New timeseries (from sled-data-link) as we've added switch information in the metadata.
zeeshanlakhani
force-pushed
the
zl/mgd-ddm-meta
branch
from
aed11e2
to
24c967f
Compare
| @@ -5,7 +5,7 @@ name = "bfd_session" | |||
| description = "A Bidirectional Forwarding Protocol (BFD) session" | |||
| authz_scope = "fleet" | |||
| versions = [ | |||
| { version = 1, fields = [ "hostname", "rack_id", "sled_id", "peer" ] }, | |||
| { version = 1, fields = [ "hostname", "peer", "rack_id", "sled_id", "sled_model", "sled_revision", "sled_serial", "switch_id", "switch_fab", "switch_lot", "switch_wafer", "switch_wafer_loc_x", "switch_wafer_loc_y", "switch_model", "switch_revision", "switch_serial", "switch_slot" ] }, | |||
There was a problem hiding this comment.
@bnaecker, there's a question about which switch and sled fields we really need. Is it just switch_id (what Ry recommended)? Do we include anything beyond sled_id for sled stuff? I laid everything out as we get all the data (either via smf or dpd-client), but it's easy to restrict this. Thoughts?
zeeshanlakhani
force-pushed
the
zl/tfportd-oximeter
branch
2 times, most recently
from
4c9a71d
to
819eded
Compare
There was a problem hiding this comment.
This looks pretty straightforward. It might be worth having @bnaecker sanity check the oximeter stuff. It looks fine to me, but I haven't worked with any of the schema stuff before.
|
|
||
| tfport_config = tfport_config | ||
| .add_property( | ||
| "host", |
There was a problem hiding this comment.
These should really be dpd_host and dpd_port or dendrite_host and dendrite_port. I know the command line options are just host and port, but that's not ideal either.
There was a problem hiding this comment.
Good point. Since this is in draft (and needs other PRs in first), I'll make these changes in #6918 for tfportd.
|
|
||
| for address in addresses { | ||
| tfport_config = tfport_config.add_property( | ||
| "address", |
There was a problem hiding this comment.
Yep, will do for these and tfport in the sep PRs.
|
|
||
| if let Some(i) = info { | ||
| tfport_config = tfport_config | ||
| .add_property( |
There was a problem hiding this comment.
Rather than repeating this same code 5 (6?) times, maybe add a helper function to do it? That will also help avoid any future errors if we decide to add/change a property.
There was a problem hiding this comment.
Yeah. For sure. This piece of code is generally super repetitive, but I'll make this change for sled properties.
|
@Nieuwejaar I'll leave this in draft and wait on the dpd/tfport changes to go in first before updating. |
zeeshanlakhani
force-pushed
the
zl/tfportd-oximeter
branch
2 times, most recently
from
01f2a0b
to
59a6171
Compare
No description provided.