metricdog: add boot time related metrics to send-boot-success #3204
Conversation
etungsten
force-pushed
the
timing-metrics
branch
from
a03da34
to
7849944
Compare
|
Push above addresses @arnaldo2792's comments. Push below rebases onto develop |
etungsten
force-pushed
the
timing-metrics
branch
from
7849944
to
613bb6a
Compare
etungsten
force-pushed
the
timing-metrics
branch
from
613bb6a
to
43cb552
Compare
sources/metricdog/src/metricdog.rs
Outdated
| values.insert( | ||
| "network_ready_time".to_string(), | ||
| self.host_check.network_ready_time()?, | ||
| ); |
There was a problem hiding this comment.
If we want these values to be stable, to simplify querying the results - would it make sense to have a real struct with known, optional fields? Or an enum with two variants - BootStatus and HealthStatus?
etungsten
force-pushed
the
timing-metrics
branch
from
43cb552
to
80b5d0a
Compare
|
Push above addresses @bcressey's comments. Adds Unit tests pass. Generated metrics event: Updated PR description with new testing. |
Refactors to accommodate a new trait for checking host status using systemd utilities. Refactors how query parameters are constructed with structs to denote fields more stably.
etungsten
force-pushed
the
timing-metrics
branch
from
80b5d0a
to
e4735d5
Compare
| }) | ||
| } | ||
|
|
||
| fn is_active(service: &str) -> Result<bool> { |
There was a problem hiding this comment.
Nit: My knee-jerk reaction anytime I see an &str as a funtion argument is to suggest to use AsRef<str> to make it more friendly to use and robust. Given these functions are only used in this module... I can see why we wouldn't do that. It just looked odd to my eyes given most of our other code does so.
Issue number:
N/A
Description of changes:
Testing done:
Checked emitted event from
metricdogupon first boot and was able to serialize out the following:{ ... "sender": "metricdog", "version": "1.15.0", "variant": "aws-k8s-1.27", "arch": "x86_64", "region": "us-west-2", "seed": 2039, "version_lock": "latest", "ignore_waves": true, "event": "boot_success", "is_first_boot": true, "preconfigured_time_ms": 12841, "configured_time_ms": 16096, "network_ready_time_ms": 7966, "filesystem_ready_time_ms": 4624 }After a reboot, metricdog sent another
send-boot-successevent with the following parameters:{ .. "sender": "metricdog", "version": "1.15.0", "variant": "aws-k8s-1.27", "arch": "x86_64", "region": "us-west-2", "seed": 2039, "version_lock": "latest", "ignore_waves": true, "event": "boot_success", "is_first_boot": false, "preconfigured_time_ms": 5090, "configured_time_ms": 5780, "network_ready_time_ms": 4399, "filesystem_ready_time_ms": 2072 }Note how
is_first_bootis no longer true and it took less time for the host to become ready as expected.Checked a health-ping event to make sure there's no regression:
{ .. "sender": "metricdog", "version": "1.15.0", "variant": "aws-k8s-1.27", "arch": "x86_64", "region": "us-west-2", "seed": 2039, "version_lock": "latest", "ignore_waves": true, "event": "health_ping", "is_healthy": true, "failed_services": {} }Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.