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

feat: add levels to Trace_core. #29

Merged
merged 4 commits into from
Mar 8, 2024
Merged

feat: add levels to Trace_core. #29

merged 4 commits into from
Mar 8, 2024

Conversation

c-cube
Copy link
Owner

@c-cube c-cube commented Feb 26, 2024

these levels are used to control the verbosity levels.

c-cube added 2 commits March 1, 2024 15:18
these levels are used to control the verbosity levels.
Copy link
Contributor

@tatchi tatchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add tests? I'm getting an error when I set the current log level to Info in t1.ml

git diff
diff --git a/test/t1.ml b/test/t1.ml
index 87e4792..a4c6e32 100644
--- a/test/t1.ml
+++ b/test/t1.ml
@@ -1,6 +1,7 @@
 let run () =
   Trace.set_process_name "main";
   Trace.set_thread_name "t1";
+  Trace.set_current_level Trace.Level.Info;
 
File "test/dune", line 3, characters 8-10:
3 |   (name t1)
            ^^
trace-tef error: cannot find span -9223372036854775808
trace-tef error: cannot find span -92

Looks like it's trying to exit a span that wasn't created due to check_level being false

let[@inline] with_span ?level ?__FUNCTION__ ~__FILE__ ~__LINE__ ?data name f =
match A.get collector with
| Some collector when check_level ?level () ->
with_span_collector_ collector ?__FUNCTION__ ~__FILE__ ~__LINE__ ?data name
f
| _ ->
(* fast path: no collector, no span *)
f Collector.dummy_span

src/core/level.ml Show resolved Hide resolved
@tatchi
Copy link
Contributor

tatchi commented Mar 7, 2024

Looks like it's trying to exit a span that wasn't created due to check_level being false

I wonder how it will behave with message. Say we have a current log level equals to Info and we do:

Trace.with_span ~level:Trace.Level.Trace "main" @@ fun span ->
  Trace.message ~level:Trace.Level.Info ~span "world";

I'm not sure exactly how message is supposed to work but looking at this code I understand that we want to "attach" a message to a span. But since the span was created with a level > Info, it maybe won't exist?

@c-cube
Copy link
Owner Author

c-cube commented Mar 7, 2024

I'm not sure exactly how message is supposed to work but looking at this code I understand that we want to "attach" a message to a span. But since the span was created with a level > Info, it maybe won't exist?

Indeed, the span won't exist, so depending on the backend, the message would attach to the closest span with level <= Info. Some backends don't have this notion at all.

@c-cube
Copy link
Owner Author

c-cube commented Mar 7, 2024

Thanks a lot for finding the bug in t1.ml btw, I just fixed it.

@c-cube c-cube merged commit e76a977 into main Mar 8, 2024
4 checks passed
@c-cube c-cube deleted the wip-levels branch March 14, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants