Skip to content

Commit 30631ea

Browse files
committed
fix(metrics): output correct process metrics
Before this commit we would output an "async" start event when a process awould start nd then a "complete" event when it would be finished. The "async" start event is unnecessary and this commit removes it. All the information recorded in the "async" start event is therefore moved to the complete event. The new output is now properly displayed by the various visualization tools (perfetto, chrome) Signed-off-by: Rudi Grinberg <me@rgrinberg.com> <!-- ps-id: d5f448b8-a896-4612-bc4e-e8d16c41fc51 -->
1 parent 0972be8 commit 30631ea

File tree

2 files changed

+22
-29
lines changed

2 files changed

+22
-29
lines changed

CHANGES.md

+4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
Unreleased
22
----------
33

4+
- Fix `--trace-file` output. Dune now emits a single *complete* event for every
5+
executed process. Unterminated *async* events are no longer written. (#6892,
6+
@rgrinberg)
7+
48
- Fix preprocessing with `staged_pps` (#6748, fixes #6644, @rgrinberg)
59

610
- Make `dune describe workspace` return consistent dependencies for

src/dune_engine/process.ml

+18-29
Original file line numberDiff line numberDiff line change
@@ -577,29 +577,22 @@ end = struct
577577
fail ~loc ~annots paragraphs
578578
end
579579

580-
let report_process_start stats ~metadata ~id ~pid ~prog ~args ~now =
580+
let report_process_finished stats ~metadata ~prog ~pid ~args ~started_at
581+
(times : Proc.Times.t) =
581582
let common =
582583
let name =
583584
match metadata.name with
584585
| Some n -> n
585586
| None -> Filename.basename prog
586587
in
587-
let ts = Timestamp.of_float_seconds now in
588+
let ts = Timestamp.of_float_seconds started_at in
588589
Event.common_fields ~cat:("process" :: metadata.categories) ~name ~ts ()
589590
in
590591
let args =
591592
[ ("process_args", `List (List.map args ~f:(fun arg -> `String arg)))
592593
; ("pid", `Int (Pid.to_int pid))
593594
]
594595
in
595-
let event =
596-
Event.async (Chrome_trace.Id.create (`Int id)) ~args Start common
597-
in
598-
Dune_stats.emit stats event;
599-
(common, args)
600-
601-
let report_process_end stats (common, args) ~now (times : Proc.Times.t) =
602-
let common = Event.set_ts common (Timestamp.of_float_seconds now) in
603596
let dur = Event.Timestamp.of_float_seconds times.elapsed_time in
604597
let event = Event.complete ~args ~dur common in
605598
Dune_stats.emit stats event
@@ -692,7 +685,7 @@ let run_internal ?dir ?(stdout_to = Io.stdout) ?(stderr_to = Io.stderr)
692685
(stdout, stderr)
693686
| _ -> ((`No_capture, stdout_to), (`No_capture, stderr_to))
694687
in
695-
let event_common, started_at, pid =
688+
let started_at, pid =
696689
(* Output.fd might create the file with Unix.openfile. We need to make
697690
sure to call it before doing the chdir as the path might be
698691
relative. *)
@@ -705,26 +698,21 @@ let run_internal ?dir ?(stdout_to = Io.stdout) ?(stderr_to = Io.stderr)
705698
| false -> env
706699
in
707700
let env = Env.to_unix env |> Spawn.Env.of_list in
708-
let started_at, pid =
701+
let started_at =
709702
(* jeremiedimino: I think we should do this just before the [execve]
710703
in the stub for [Spawn.spawn] to be as precise as possible *)
711-
let now = Unix.gettimeofday () in
712-
( now
713-
, Spawn.spawn () ~prog:prog_str ~argv ~env ~stdout ~stderr ~stdin
714-
~setpgid:Spawn.Pgid.new_process_group
715-
~cwd:
716-
(match dir with
717-
| None -> Inherit
718-
| Some dir -> Path (Path.to_string dir))
719-
|> Pid.of_int )
704+
Unix.gettimeofday ()
720705
in
721-
let event_common =
722-
Option.map config.stats ~f:(fun stats ->
723-
( stats
724-
, report_process_start stats ~metadata ~id ~pid ~prog:prog_str
725-
~args ~now:started_at ))
706+
let pid =
707+
Spawn.spawn () ~prog:prog_str ~argv ~env ~stdout ~stderr ~stdin
708+
~setpgid:Spawn.Pgid.new_process_group
709+
~cwd:
710+
(match dir with
711+
| None -> Inherit
712+
| Some dir -> Path (Path.to_string dir))
713+
|> Pid.of_int
726714
in
727-
(event_common, started_at, pid)
715+
(started_at, pid)
728716
in
729717
Io.release stdout_to;
730718
Io.release stderr_to;
@@ -736,8 +724,9 @@ let run_internal ?dir ?(stdout_to = Io.stdout) ?(stderr_to = Io.stderr)
736724
; resource_usage = process_info.resource_usage
737725
}
738726
in
739-
Option.iter event_common ~f:(fun (stats, common) ->
740-
report_process_end stats common ~now:process_info.end_time times);
727+
Option.iter config.stats ~f:(fun stats ->
728+
report_process_finished stats ~metadata ~prog:prog_str ~pid ~args
729+
~started_at times);
741730
Option.iter response_file ~f:Path.unlink;
742731
let actual_stdout =
743732
match stdout_capture with

0 commit comments

Comments
 (0)