Skip to content

Commit 790a2f1

Browse files
filmilCQ Bot
authored andcommitted
[time][inspect] Adds basic inspect metric collection for UTC adjustments
Adds success and failure counts, as well as rudimentary info of last attempted adjust. This is implemented only for inspect for the time being. Bug: 388898827 Change-Id: I7b926e7fb217c38b66ce08aeddd70ff2d24d7053 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1234127 Fuchsia-Auto-Submit: Filip Filmar <[email protected]> Commit-Queue: Auto-Submit <[email protected]> Reviewed-by: Guocheng Wei <[email protected]>
1 parent 0476869 commit 790a2f1

File tree

6 files changed

+100
-11
lines changed

6 files changed

+100
-11
lines changed

src/sys/time/timekeeper/src/clock_manager.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44

55
use crate::diagnostics::{Diagnostics, Event};
66
use crate::enums::{
7-
ClockCorrectionStrategy, ClockUpdateReason, StartClockSource, Track, WriteRtcOutcome,
7+
ClockCorrectionStrategy, ClockUpdateReason, StartClockSource, Track, UserAdjustUtcOutcome,
8+
WriteRtcOutcome,
89
};
910
use crate::estimator::Estimator;
1011
use crate::rtc::Rtc;
@@ -504,8 +505,20 @@ impl<R: Rtc, D: 'static + Diagnostics> ClockManager<R, D> {
504505
None => estimate_transform,
505506
Some(ref proposal) => adjust_decision
506507
.try_adjust(&estimate_transform, proposal.reference, proposal.utc)
507-
.map(|(tr, _)| tr)
508+
.map(|(tr, offset)| {
509+
self.diagnostics
510+
.record(Event::WriteRtc { outcome: WriteRtcOutcome::Succeeded });
511+
self.diagnostics.record(Event::UserAdjustUtc {
512+
outcome: UserAdjustUtcOutcome::Succeeded,
513+
offset,
514+
});
515+
tr
516+
})
508517
.map_err(|e| {
518+
self.diagnostics.record(Event::UserAdjustUtc {
519+
outcome: UserAdjustUtcOutcome::Failed,
520+
offset: UtcDuration::from_nanos(0),
521+
});
509522
error!("time adjustmend rejected: {:?}", e);
510523
})
511524
.unwrap_or(estimate_transform),

src/sys/time/timekeeper/src/diagnostics/cobalt.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,9 @@ impl Diagnostics for CobaltDiagnostics {
308308
Event::UpdateClock { track, reason } => {
309309
self.record_clock_update(track, reason);
310310
}
311+
Event::UserAdjustUtc { .. } => {
312+
// Don't record anything to Cobalt.
313+
}
311314
}
312315
}
313316
}

src/sys/time/timekeeper/src/diagnostics/inspect.rs

Lines changed: 63 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@
55
use crate::diagnostics::{Diagnostics, Event};
66
use crate::enums::{
77
ClockCorrectionStrategy, ClockUpdateReason, FrequencyDiscardReason, InitializeRtcOutcome, Role,
8-
SampleValidationError, TimeSourceError, Track, WriteRtcOutcome,
8+
SampleValidationError, TimeSourceError, Track, UserAdjustUtcOutcome, WriteRtcOutcome,
99
};
1010
use crate::{MonitorTrack, PrimaryTrack, TimeSource};
1111
use fidl_fuchsia_time_external::Status;
1212
use fuchsia_inspect::{
13-
Inspector, IntProperty, Node, NumericProperty, Property, StringProperty, UintProperty,
13+
BoolProperty, Inspector, IntProperty, Node, NumericProperty, Property, StringProperty,
14+
UintProperty,
1415
};
1516
use fuchsia_runtime::{UtcClock, UtcClockDetails, UtcDuration, UtcInstant};
1617
use fuchsia_sync::Mutex;
@@ -187,6 +188,49 @@ impl RealTimeClockNode {
187188
}
188189
}
189190

191+
/// An inspect `Node` and properties used to describe the history of user UTC
192+
/// adjustments.
193+
struct UserAdjustUtcNode {
194+
/// The count of successful user adjustment UTC attempts.
195+
success_count: UintProperty,
196+
/// The count of failed user adjustment UTC attempts.
197+
failure_count: UintProperty,
198+
/// The value of the offset resulting from the last successful proposal.
199+
last_allowed_offset_nanos: IntProperty,
200+
/// Was last update was a failure?
201+
last_update_failure: BoolProperty,
202+
/// The inspect node these fields are exported to.
203+
_node: Node,
204+
}
205+
206+
impl UserAdjustUtcNode {
207+
pub fn new(node: Node) -> Self {
208+
Self {
209+
success_count: node.create_uint("success_count", 0),
210+
failure_count: node.create_uint("failure_count", 0),
211+
last_allowed_offset_nanos: node.create_int("last_proposed_offset_nanos", 0),
212+
last_update_failure: node.create_bool("last_updated_failure", false),
213+
_node: node,
214+
}
215+
}
216+
}
217+
218+
impl UserAdjustUtcNode {
219+
fn update_user_adjust_utc(&mut self, outcome: UserAdjustUtcOutcome, offset: UtcDuration) {
220+
match outcome {
221+
UserAdjustUtcOutcome::Succeeded => {
222+
self.success_count.add(1);
223+
self.last_update_failure.set(false);
224+
self.last_allowed_offset_nanos.set(offset.into_nanos());
225+
}
226+
UserAdjustUtcOutcome::Failed => {
227+
self.failure_count.add(1);
228+
self.last_update_failure.set(true);
229+
}
230+
}
231+
}
232+
}
233+
190234
/// An inspect `Node` and properties used to describe the health of a time source.
191235
struct TimeSourceNode {
192236
/// The most recent status of the time source.
@@ -210,7 +254,7 @@ impl TimeSourceNode {
210254
status_change: node.create_int("status_change_reference", reference_time()),
211255
failure_counters: HashMap::new(),
212256
rejection_counters: HashMap::new(),
213-
node: node,
257+
node,
214258
}
215259
}
216260

@@ -395,6 +439,8 @@ pub struct InspectDiagnostics {
395439
tracks: Mutex<HashMap<Track, TrackNode>>,
396440
/// Details of interactions with the real time clock.
397441
rtc: Mutex<Option<RealTimeClockNode>>,
442+
/// Details of user utc adjustments.
443+
user_utc_adjustments: Mutex<Option<UserAdjustUtcNode>>,
398444
/// The inspect node used to export the contents of this `InspectDiagnostics`.
399445
node: Node,
400446
}
@@ -406,6 +452,7 @@ impl InspectDiagnostics {
406452
node: &Node,
407453
primary: &PrimaryTrack,
408454
optional_monitor: &Option<MonitorTrack>,
455+
allow_user_utc_adjustments: bool,
409456
) -> Self {
410457
// Record fixed data directly into the node without retaining any references.
411458
node.record_child("initialization", |child| TimeSet::now(&primary.clock).record(child));
@@ -434,11 +481,18 @@ impl InspectDiagnostics {
434481
);
435482
}
436483

484+
let user_utc_adjustments = Mutex::new(if allow_user_utc_adjustments {
485+
Some(UserAdjustUtcNode::new(node.create_child("user_utc_adjustments")))
486+
} else {
487+
None
488+
});
489+
437490
let diagnostics = InspectDiagnostics {
438491
time_sources: Mutex::new(time_sources_hashmap),
439492
tracks: Mutex::new(tracks_hashmap),
440493
rtc: Mutex::new(None),
441494
node: node.clone_weak(),
495+
user_utc_adjustments,
442496
};
443497
let clock = Arc::clone(&primary.clock);
444498
node.record_lazy_child("current", move || {
@@ -513,6 +567,11 @@ impl Diagnostics for InspectDiagnostics {
513567
Event::UpdateClock { track, reason } => {
514568
self.update_track(track, |tn| tn.update_clock(Some(reason)));
515569
}
570+
Event::UserAdjustUtc { outcome, offset } => {
571+
if let Some(ref mut utc_node) = *self.user_utc_adjustments.lock() {
572+
utc_node.update_user_adjust_utc(outcome, offset);
573+
}
574+
}
516575
}
517576
}
518577
}
@@ -589,7 +648,7 @@ mod tests {
589648
false => None,
590649
};
591650

592-
(InspectDiagnostics::new(inspector.root(), &primary, &monitor), primary.clock)
651+
(InspectDiagnostics::new(inspector.root(), &primary, &monitor, false), primary.clock)
593652
}
594653

595654
#[fuchsia::test]

src/sys/time/timekeeper/src/diagnostics/mod.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub use self::inspect::{InspectDiagnostics, INSPECTOR};
1717
use crate::enums::{
1818
ClockCorrectionStrategy, ClockUpdateReason, FrequencyDiscardReason, InitialClockState,
1919
InitializeRtcOutcome, Role, SampleValidationError, StartClockSource, TimeSourceError, Track,
20-
WriteRtcOutcome,
20+
UserAdjustUtcOutcome, WriteRtcOutcome,
2121
};
2222
use fidl_fuchsia_time_external::Status;
2323
use fuchsia_runtime::{UtcDuration, UtcInstant};
@@ -79,6 +79,8 @@ pub enum Event {
7979
StartClock { track: Track, source: StartClockSource },
8080
/// The userspace clock has been updated.
8181
UpdateClock { track: Track, reason: ClockUpdateReason },
82+
/// The UTC clock user adjustment result.
83+
UserAdjustUtc { outcome: UserAdjustUtcOutcome, offset: UtcDuration },
8284
}
8385

8486
/// A standard interface for systems that record events for diagnostic purposes.

src/sys/time/timekeeper/src/enums.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,9 @@ impl Into<CobaltTimeSourceEvent> for TimeSourceError {
257257
}
258258
}
259259
}
260+
261+
#[derive(Clone, Copy, Debug, PartialEq)]
262+
pub enum UserAdjustUtcOutcome {
263+
Failed,
264+
Succeeded,
265+
}

src/sys/time/timekeeper/src/main.rs

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -261,10 +261,17 @@ async fn main() -> Result<()> {
261261
clock: Arc::new(create_monitor_clock(&primary_track.clock)),
262262
});
263263

264+
let enable_user_utc_adjustment = config.serve_fuchsia_time_external_adjust();
265+
264266
info!("initializing diagnostics and serving inspect on servicefs");
265267
let cobalt_experiment = COBALT_EXPERIMENT;
266268
let diagnostics = Arc::new(CompositeDiagnostics::new(
267-
InspectDiagnostics::new(inspector_root, &primary_track, &monitor_track),
269+
InspectDiagnostics::new(
270+
inspector_root,
271+
&primary_track,
272+
&monitor_track,
273+
enable_user_utc_adjustment,
274+
),
268275
CobaltDiagnostics::new(cobalt_experiment, &primary_track, &monitor_track),
269276
));
270277

@@ -287,7 +294,6 @@ async fn main() -> Result<()> {
287294
let cmd_send_clone = cmd_send.clone();
288295
let serve_test_protocols = config.serve_test_protocols();
289296
let serve_fuchsia_time_alarms = config.serve_fuchsia_time_alarms();
290-
let serve_adjust = config.serve_fuchsia_time_external_adjust();
291297
let ps = persistent_state.clone();
292298

293299
if config.has_always_on_counter() {
@@ -356,7 +362,7 @@ async fn main() -> Result<()> {
356362
fs.dir("svc").add_fidl_service(Rpcs::Wake);
357363
info!("serving protocol: fuchsia.time.alarms/Wake");
358364
}
359-
if serve_adjust {
365+
if enable_user_utc_adjustment {
360366
fs.dir("svc").add_fidl_service(Rpcs::Adjust);
361367
info!("serving protocol: fuchsia.time.alarms/Adjust");
362368
}
@@ -388,7 +394,7 @@ async fn main() -> Result<()> {
388394

389395
let rtc_test_server = Rc::new(rtc_testing::Server::new(persistence_health, persistent_state));
390396

391-
let adjust_server = Rc::new(if serve_adjust {
397+
let adjust_server = Rc::new(if enable_user_utc_adjustment {
392398
let cmd_send_clone = cmd_send.clone();
393399
Some(time_adjust::Server::new(cmd_send_clone))
394400
} else {

0 commit comments

Comments
 (0)