From 1ebd474423c90e3d900e624253ab00d7d1615afc Mon Sep 17 00:00:00 2001
From: Simon Cruanes <simon.cruanes.2007@m4x.org>
Date: Sun, 8 Feb 2026 06:05:49 +0000
Subject: [PATCH] fix: critical bugs found in code review

Bug #1: Fix worker count logic in generic_consumer
- Was: min 2 (max 500 n_workers) - always created 2 workers
- Now: max 2 (min 500 n_workers) - properly clamps between 2-500
- Impact: Worker configuration was completely ignored

Bug #2: Handle missing dot in __FUNCTION__ name
- Added exception handling for String.rindex in trace span creation
- Prevents crash when tracing top-level or non-module functions
- Uses option type for module_path when no dot is present
- Scoped try/catch to only parsing logic
---
 src/client/generic_consumer.ml   |  2 +-
 src/trace/opentelemetry_trace.ml | 30 ++++++++++++++++++++----------
 tests/implicit_scope/sync/dune   |  7 ++++++-
 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/src/client/generic_consumer.ml b/src/client/generic_consumer.ml
index 4446811b..8489ffd9 100644
--- a/src/client/generic_consumer.ml
+++ b/src/client/generic_consumer.ml
@@ -194,7 +194,7 @@ end = struct
     in
 
     (* start workers *)
-    let n_workers = min 2 (max 500 self.config.n_workers) in
+    let n_workers = max 2 (min 500 self.config.n_workers) in
 
     ignore (Atomic.fetch_and_add self.n_workers n_workers : int);
     for _i = 1 to n_workers do
diff --git a/src/trace/opentelemetry_trace.ml b/src/trace/opentelemetry_trace.ml
index 1cda56ad..95e439ad 100644
--- a/src/trace/opentelemetry_trace.ml
+++ b/src/trace/opentelemetry_trace.ml
@@ -71,17 +71,27 @@ open struct
     (* add more data if [__FUNCTION__] is present *)
     (match __FUNCTION__ with
     | Some __FUNCTION__ when OTEL.Span.is_not_dummy otel_sp ->
-      let last_dot = String.rindex __FUNCTION__ '.' in
-      let module_path = String.sub __FUNCTION__ 0 last_dot in
-      let function_name =
-        String.sub __FUNCTION__ (last_dot + 1)
-          (String.length __FUNCTION__ - last_dot - 1)
+      let function_name, module_path =
+        try
+          let last_dot = String.rindex __FUNCTION__ '.' in
+          let module_path = String.sub __FUNCTION__ 0 last_dot in
+          let function_name =
+            String.sub __FUNCTION__ (last_dot + 1)
+              (String.length __FUNCTION__ - last_dot - 1)
+          in
+          function_name, Some module_path
+        with Not_found ->
+          (* __FUNCTION__ has no dot, use it as-is *)
+          __FUNCTION__, None
       in
-      OTEL.Span.add_attrs otel_sp
-        [
-          "code.function", `String function_name;
-          "code.namespace", `String module_path;
-        ]
+      let attrs =
+        ("code.function", `String function_name)
+        ::
+        (match module_path with
+        | Some module_path -> [ "code.namespace", `String module_path ]
+        | None -> [])
+      in
+      OTEL.Span.add_attrs otel_sp attrs
     | _ -> ());
 
     Span_otel otel_sp
diff --git a/tests/implicit_scope/sync/dune b/tests/implicit_scope/sync/dune
index 4a6f6141..eb17095c 100644
--- a/tests/implicit_scope/sync/dune
+++ b/tests/implicit_scope/sync/dune
@@ -1,4 +1,9 @@
 (tests
  (names test_implicit_scope_sync)
  (package opentelemetry-client-cohttp-lwt)
- (libraries threads alcotest opentelemetry unix opentelemetry-client-cohttp-lwt))
+ (libraries
+  threads
+  alcotest
+  opentelemetry
+  unix
+  opentelemetry-client-cohttp-lwt))