Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for tracing #31

Closed
2 of 4 tasks
Thomasdezeeuw opened this issue Jul 27, 2018 · 8 comments · Fixed by #426
Closed
2 of 4 tasks

Add support for tracing #31

Thomasdezeeuw opened this issue Jul 27, 2018 · 8 comments · Fixed by #426
Labels
enhancement An improvement of the API. priority:low Low priority issue.
Milestone

Comments

@Thomasdezeeuw
Copy link
Owner

Thomasdezeeuw commented Jul 27, 2018

Locally, e.g. https://golang.org/cmd/trace.
Or distributed among different serivces, e.g. https://zipkin.io.

Related issues:

@Thomasdezeeuw Thomasdezeeuw added the priority:low Low priority issue. label Jul 27, 2018
@Thomasdezeeuw
Copy link
Owner Author

Also see https://opentracing.io/ and tokio-rs/tokio#561.

@Thomasdezeeuw
Copy link
Owner Author

Also see #13.

@Thomasdezeeuw
Copy link
Owner Author

Tracing pr for Tokio: tokio-rs/tokio#827, and the prototype repo: https://github.com/hawkw/tokio-trace-prototype.

@Thomasdezeeuw Thomasdezeeuw added the enhancement An improvement of the API. label Feb 10, 2019
@Thomasdezeeuw
Copy link
Owner Author

@Thomasdezeeuw
Copy link
Owner Author

Another tracing crate (to be) used by Tokio: https://crates.io/crates/tracing.

@Thomasdezeeuw
Copy link
Owner Author

Thomasdezeeuw commented Oct 10, 2020

Some API I though of:

impl<M> actor::Context<M> {
    /// Enables tracing for this actor.
    ///
    /// Tracing is disabled once this context is dropped.
    fn enable_tracing(&mut self) {
        /* ... */
    }
}

@Thomasdezeeuw
Copy link
Owner Author

Thomasdezeeuw commented Nov 26, 2020

Some things to monitor/trace (also relating to #217):

Time below is a timestamp, timespan a tuple (start, end timestamp).

Running actors/per process:

  • Time started.
  • Last time run.
  • Timespans run.
  • Total run time.
  • Times restarted (actor only).

Runtime:

  • Time started.
  • Timespans running processes.
  • Timespans polling event sources.
  • Number of actors spawned:
    • Thread-local.
    • Thread-safe.
    • Synchronous.

Scheduler (shared and local): might too much detail

  • Time spend removing process.
  • Time spend adding process.

Sending messages/RPC:

  • Time messages starting to send.
  • Time message added to the inbox.
  • Time message removed from the inbox.
  • Time of RPC response/no response.

Inbox:

  • Number of messages in the inbox.
  • Last time a message was added.
  • Last time a message was removed.

Timers:

  • Timer passed or not.

Miscellaneous:

  • Blocking on mutexes.

@Thomasdezeeuw
Copy link
Owner Author

Patch to detect if tracing was already enabled:

From c8ddee49bdbb0097f4a7d42bc865337f581b0899 Mon Sep 17 00:00:00 2001
From: Thomas de Zeeuw <thomasdezeeuw@gmail.com>
Date: Sat, 27 Feb 2021 13:17:59 +0100
Subject: [PATCH] Return new enum Setting in Setup::enable_tracing

This new enum Setting indicates if the setting, in this case enabling
tracing, was successful or was already done previously.
---
 src/rt/mod.rs               |  2 +-
 src/rt/setup.rs             | 23 +++++++++++++++++++----
 tests/functional/runtime.rs | 26 ++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/src/rt/mod.rs b/src/rt/mod.rs
index bc1431f..c7bd323 100644
--- a/src/rt/mod.rs
+++ b/src/rt/mod.rs
@@ -153,7 +153,7 @@
 pub use access::Access;
 pub use error::Error;
 pub use options::{ActorOptions, SyncActorOptions};
-pub use setup::Setup;
+pub use setup::{Setting, Setup};
 pub use signal::Signal;
 
 use coordinator::Coordinator;
diff --git a/src/rt/setup.rs b/src/rt/setup.rs
index 07cd6bd..658bed3 100644
--- a/src/rt/setup.rs
+++ b/src/rt/setup.rs
@@ -111,13 +111,18 @@ pub const fn auto_cpu_affinity(mut self) -> Self {
     ///
     /// See the [`mod@trace`] module for more information.
     ///
-    /// Returns an error if a file at `path` already exists or can't create the
-    /// file.
-    pub fn enable_tracing<P: AsRef<Path>>(&mut self, path: P) -> Result<(), Error> {
+    /// Returns [`Setting::Enabled`] on success, [`Setting::AlreadyEnabled`] if
+    /// tracing has already been enabled or an error if a file at `path` already
+    /// exists or can't be created.
+    pub fn enable_tracing<P: AsRef<Path>>(&mut self, path: P) -> Result<Setting, Error> {
+        if self.trace_log.is_some() {
+            return Ok(Setting::AlreadyEnabled);
+        }
+
         match trace::Log::open(path.as_ref(), coordinator::TRACE_ID) {
             Ok(trace_log) => {
                 self.trace_log = Some(trace_log);
-                Ok(())
+                Ok(Setting::Enabled)
             }
             Err(err) => Err(Error::setup_trace(err)),
         }
@@ -181,3 +186,13 @@ pub fn build(mut self) -> Result<Runtime, Error> {
         })
     }
 }
+
+/// Indicates if a setting has been enabled, or has previously already been
+/// enabled.
+#[derive(Debug, Eq, PartialEq)]
+pub enum Setting {
+    /// The setting has been enabled.
+    Enabled,
+    /// The setting has already been enabled previously.
+    AlreadyEnabled,
+}
diff --git a/tests/functional/runtime.rs b/tests/functional/runtime.rs
index c247755..eb1b4b5 100644
--- a/tests/functional/runtime.rs
+++ b/tests/functional/runtime.rs
@@ -9,7 +9,7 @@
 
 use heph::actor::{self, SyncContext, ThreadLocal, ThreadSafe};
 use heph::rt::options::{ActorOptions, Priority, SyncActorOptions};
-use heph::rt::Runtime;
+use heph::rt::{Runtime, Setting};
 use heph::supervisor::NoSupervisor;
 
 use crate::util::temp_file;
@@ -152,7 +152,7 @@ fn tracing() {
 
     // Generate a simple trace.
     let mut setup = Runtime::setup();
-    setup.enable_tracing(&trace_path).unwrap();
+    assert_eq!(setup.enable_tracing(&trace_path).unwrap(), Setting::Enabled);
     setup.build().unwrap().start().unwrap();
 
     // Convert the trace just to make sure it's valid.
@@ -177,6 +177,28 @@ fn tracing() {
     }
 }
 
+#[test]
+fn tracing_already_enabled() {
+    let trace_path1 = temp_file("runtime_tracing_already_enabled1.bin.trace");
+    let trace_path2 = temp_file("runtime_tracing_already_enabled2.bin.trace");
+
+    let mut setup = Runtime::setup();
+    assert_eq!(
+        setup.enable_tracing(&trace_path1).unwrap(),
+        Setting::Enabled
+    );
+    // Same path.
+    assert_eq!(
+        setup.enable_tracing(&trace_path1).unwrap(),
+        Setting::AlreadyEnabled
+    );
+    // Different path.
+    assert_eq!(
+        setup.enable_tracing(&trace_path2).unwrap(),
+        Setting::AlreadyEnabled
+    );
+}
+
 #[derive(Clone)] // Needed in setup function.
 struct WaitFuture {
     #[allow(clippy::type_complexity)]
-- 
2.30.1

@Thomasdezeeuw Thomasdezeeuw added this to the First release milestone Mar 2, 2021
This was referenced Mar 17, 2021
@Thomasdezeeuw Thomasdezeeuw unpinned this issue Mar 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement of the API. priority:low Low priority issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant