chore: fix some small things (#3)

addressing some feedback from @kopecs 
> * https://github.com/semgrep/pyro-caml/blob/5937a13eee7b5f2313ed5d7d11a3d7726a35970f/src/main.rs#L127
Safety comment
But also, do we need this for a subprocess or something? Does https://docs.rs/pretty_env_logger/latest/pretty_env_logger/fn.formatted_timed_builder.html and filter_level or whatever work?
> * https://github.com/semgrep/pyro-caml/blob/5937a13eee7b5f2313ed5d7d11a3d7726a35970f/src/ocaml_intf.rs#L28
Safety comment (same for others in this file)
Stupid why is this unsafe and the docs for the library have no details. I mean, I get it, FFI, but still.
> * https://github.com/semgrep/pyro-caml/blob/5937a13eee7b5f2313ed5d7d11a3d7726a35970f/src/backend.rs#L21-L25
OK, I might not understand this, but what does the runtime actually do here? I think I am missing what the actual interaction is. We have our own thread local runtime to call an ffi function?
> * https://github.com/semgrep/pyro-caml/blob/5937a13eee7b5f2313ed5d7d11a3d7726a35970f/src/backend.rs#L99
seems not so useful to log that this was just called?


As far as the runtime point goes, we can't send runtimes across threads, so this was the easiest way for me to get one where it needs to be. I probably could be more clever about where/when I create it but 🤷

Author: ajbt200128

src/backend.rs

@@ -96,7 +96,6 @@ impl Backend for CamlSpy {
     }
 
     fn sample_rate(&self) -> Result<u32> {
-        log::debug!(target:LOG_TAG, "request sample rate");
         Ok(self.config.sample_rate)
     }
 

src/main.rs

@@ -123,9 +123,9 @@ fn make_agent_builder(
 fn main() {
     let cli = Cli::parse();
 
-    // there is probably a better way to do this
-    unsafe { std::env::set_var("RUST_LOG", cli.verbosity.log_level_filter().to_string()) };
-    pretty_env_logger::init_timed();
+    pretty_env_logger::formatted_timed_builder()
+        .filter_level(cli.verbosity.log_level_filter())
+        .init();
 
     let bin = cli.binary;
     let args = cli.args;

src/ocaml_intf.rs

@@ -25,6 +25,11 @@ impl From<CamlIntfError> for PyroscopeError {
 impl From<ocaml::Error> for CamlIntfError {
     fn from(err: ocaml::Error) -> Self {
         if let ocaml::Error::Caml(ocaml::CamlError::Exception(exc)) = &err {
+            // # Safety
+            // exc.exception_to_string() is unsafe because it may dereference
+            // a raw pointer inside the ocaml runtime. However, since we have
+            // an &Runtime in the call stack, we assume the caller has ensured
+            // the runtime is valid. (I think)
             match unsafe { exc.exception_to_string() } {
                 Ok(s) => CamlIntfError(format!("Ocaml exception: {}", s)),
                 Err(utf8error) => CamlIntfError(format!(
@@ -98,10 +103,22 @@ pub fn read_poll(
     cursor: Cursor,
     interval: f64,
 ) -> Result<Vec<CamlStackTrace>, CamlIntfError> {
+    // # Safety
+    //
+    // it is unclear why this function is unsafe from the ocaml-rs docs
+    // but we assume the caller must ensure the gc is valid, and we convert the
+    // path and int for the caller, so there is no risk they coerced a bad
+    // value into an ocaml::Value
     Ok(unsafe { read_poll_ml(gc, cursor, interval as ocaml::Float) }?.into_vec())
 }
 
 pub fn create_cursor(gc: &Runtime, path: &Path, pid: u32) -> Cursor {
+    // Safety
+    //
+    // it is unclear why this function is unsafe from the ocaml-rs docs but we
+    // assume the caller must ensure the gc is valid, and we conver the path and
+    // int for the caller, so there is no risk they coerced a bad value into an
+    // ocaml::Value
     unsafe { create_cursor_ml(gc, path.to_str().unwrap().to_string(), pid as ocaml::Int) }
         .expect("failed to create cursor")
 }