fix(otel): ExtendedTask's otel enabled status set during init (#4334)

NEWS.md

@@ -1,5 +1,6 @@
 # shiny (development version)
 
+* `ExtendedTask` now captures the OpenTelemetry recording state at initialization time rather than at invocation time, ensuring consistent span recording behavior regardless of runtime configuration changes. (#4334)
 * Added `withOtelCollect()` and `localOtelCollect()` functions to temporarily control OpenTelemetry collection levels during reactive expression creation. These functions allow you to enable or disable telemetry collection for specific modules or sections of code where reactive expressions are being created. (#4333)
 * OpenTelemetry code attributes now include both preferred (`code.file.path`, `code.line.number`, `code.column.number`) and deprecated (`code.filepath`, `code.lineno`, `code.column`) attribute names to follow OpenTelemetry semantic conventions while maintaining backward compatibility. The deprecated names will be removed in a future release after Logfire supports the preferred names. (#4325)
 * Timer tests are now skipped on CRAN. (#4327)

R/extended-task.R

@@ -41,6 +41,28 @@
 #'   is, a function that quickly returns a promise) and allows even that very
 #'   session to immediately unblock and carry on with other user interactions.
 #'
+#' @section OpenTelemetry Integration:
+#'
+#' When an `ExtendedTask` is created, if OpenTelemetry tracing is enabled for
+#' `"reactivity"` (see [withOtelCollect()]), the `ExtendedTask` will record
+#' spans for each invocation of the task. The tracing level at `invoke()` time
+#' does not affect whether spans are recorded; only the tracing level when
+#' calling `ExtendedTask$new()` matters.
+#'
+#' The OTel span will be named based on the label created from the variable the
+#' `ExtendedTask` is assigned to. If no label can be determined, the span will
+#' be named `<anonymous>`. Similar to other Shiny OpenTelemetry spans, the span
+#' will also include source reference attributes and session ID attributes.
+#'
+#' ```r
+#' withOtelCollect("all", {
+#'   my_task <- ExtendedTask$new(function(...) { ... })
+#' })
+#'
+#' # Span recorded for this invocation: ExtendedTask my_task
+#' my_task$invoke(...)
+#' ```
+#'
 #' @examplesIf rlang::is_interactive() && rlang::is_installed("mirai")
 #' library(shiny)
 #' library(bslib)
@@ -143,6 +165,10 @@ ExtendedTask <- R6Class("ExtendedTask", portable = TRUE, cloneable = FALSE,
         otel_srcref_attributes(call_srcref, "ExtendedTask"),
         otel_session_id_attrs(domain)
       ) %||% list()
+
+      # Capture this value at init-time, not run-time
+      # This way, the span is only created if otel was enabled at time of creation... just like other spans
+      private$is_recording_otel <- has_otel_collect("reactivity")
     },
     #' @description
     #' Starts executing the long-running operation. If this `ExtendedTask` is
@@ -175,7 +201,7 @@ ExtendedTask <- R6Class("ExtendedTask", portable = TRUE, cloneable = FALSE,
         private$invocation_queue$add(list(args = args, call = call))
       } else {
 
-        if (has_otel_collect("reactivity")) {
+        if (private$is_recording_otel) {
           private$otel_span <- start_otel_span(
             private$otel_span_label,
             attributes = private$otel_attrs
@@ -253,6 +279,7 @@ ExtendedTask <- R6Class("ExtendedTask", portable = TRUE, cloneable = FALSE,
     otel_span_label = NULL,
     otel_log_label_add_to_queue = NULL,
     otel_attrs = list(),
+    is_recording_otel = FALSE,
     otel_span = NULL,
 
     do_invoke = function(args, call = NULL) {

man/ExtendedTask.Rd

@@ -45,6 +45,29 @@ is, a function that quickly returns a promise) and allows even that very
 session to immediately unblock and carry on with other user interactions.
 }
 
+\section{OpenTelemetry Integration}{
+
+
+When an \code{ExtendedTask} is created, if OpenTelemetry tracing is enabled for
+\code{"reactivity"} (see \code{\link[=withOtelCollect]{withOtelCollect()}}), the \code{ExtendedTask} will record
+spans for each invocation of the task. The tracing level at \code{invoke()} time
+does not affect whether spans are recorded; only the tracing level when
+calling \code{ExtendedTask$new()} matters.
+
+The OTel span will be named based on the label created from the variable the
+\code{ExtendedTask} is assigned to. If no label can be determined, the span will
+be named \verb{<anonymous>}. Similar to other Shiny OpenTelemetry spans, the span
+will also include source reference attributes and session ID attributes.
+
+\if{html}{\out{<div class="sourceCode r">}}\preformatted{withOtelCollect("all", \{
+  my_task <- ExtendedTask$new(function(...) \{ ... \})
+\})
+
+# Span recorded for this invocation: ExtendedTask my_task
+my_task$invoke(...)
+}\if{html}{\out{</div>}}
+}
+
 \examples{
 \dontshow{if (rlang::is_interactive() && rlang::is_installed("mirai")) withAutoprint(\{ # examplesIf}
 library(shiny)

tests/testthat/helper-otel.R

@@ -0,0 +1,31 @@
+# Helper function to create a mock otel span
+create_mock_otel_span <- function(name = "test_span") {
+  structure(
+    list(
+      name = name,
+      activate = function(...) NULL,
+      end = function(...) NULL
+    ),
+    class = "otel_span"
+  )
+}
+
+# Helper function to create a mock tracer
+create_mock_tracer <- function() {
+  structure(
+    list(
+      name = "mock_tracer",
+      is_enabled = function() TRUE,
+      start_span = function(name, ...) create_mock_otel_span(name)
+    ),
+    class = "otel_tracer"
+  )
+}
+
+# Helper function to create a mock logger
+create_mock_logger <- function() {
+  structure(
+    list(name = "mock_logger"),
+    class = "otel_logger"
+  )
+}

tests/testthat/test-otel-extended-task.R

@@ -0,0 +1,219 @@
+# Tests for ExtendedTask otel behavior
+
+ex_task_42 <- function() {
+  ExtendedTask$new(function() {
+    promises::promise_resolve(42)
+  })
+}
+
+test_that("ExtendedTask captures otel collection state at initialization", {
+  # Test that has_otel_collect is called at init, not at invoke time
+  withr::local_options(list(shiny.otel.collect = "reactivity"))
+
+  # Enable otel tracing
+  local_mocked_bindings(
+    otel_is_tracing_enabled = function() TRUE
+  )
+
+  task <- ex_task_42()
+
+  # Check that is_recording_otel is captured at init time
+  expect_true(task$.__enclos_env__$private$is_recording_otel)
+})
+
+test_that("ExtendedTask sets is_recording_otel to FALSE when otel disabled", {
+  # Enable otel tracing
+  local_mocked_bindings(
+    otel_is_tracing_enabled = function() FALSE
+  )
+
+  # Test with all level
+  withr::with_options(list(shiny.otel.collect = "all"), {
+    task1 <- ex_task_42()
+    expect_false(task1$.__enclos_env__$private$is_recording_otel)
+  })
+
+  # Test with reactivity level
+  withr::with_options(list(shiny.otel.collect = "reactivity"), {
+    task1 <- ex_task_42()
+    expect_false(task1$.__enclos_env__$private$is_recording_otel)
+  })
+
+  # Test with session level (should be FALSE)
+  withr::with_options(list(shiny.otel.collect = "session"), {
+    task2 <- ex_task_42()
+    expect_false(task2$.__enclos_env__$private$is_recording_otel)
+  })
+
+  # Test with none level (should be FALSE)
+  withr::with_options(list(shiny.otel.collect = "none"), {
+    task3 <- ex_task_42()
+    expect_false(task3$.__enclos_env__$private$is_recording_otel)
+  })
+})
+
+test_that("ExtendedTask sets is_recording_otel based on has_otel_collect at init", {
+  # Enable otel tracing
+  local_mocked_bindings(
+    otel_is_tracing_enabled = function() TRUE
+  )
+
+  # Test with all level
+  withr::with_options(list(shiny.otel.collect = "all"), {
+    task1 <- ex_task_42()
+    expect_true(task1$.__enclos_env__$private$is_recording_otel)
+  })
+
+  # Test with reactivity level
+  withr::with_options(list(shiny.otel.collect = "reactivity"), {
+    task1 <- ex_task_42()
+    expect_true(task1$.__enclos_env__$private$is_recording_otel)
+  })
+
+  # Test with session level (should be FALSE)
+  withr::with_options(list(shiny.otel.collect = "session"), {
+    task2 <- ex_task_42()
+    expect_false(task2$.__enclos_env__$private$is_recording_otel)
+  })
+
+  # Test with none level (should be FALSE)
+  withr::with_options(list(shiny.otel.collect = "none"), {
+    task3 <- ex_task_42()
+    expect_false(task3$.__enclos_env__$private$is_recording_otel)
+  })
+})
+
+test_that("ExtendedTask uses init-time otel setting even if option changes later", {
+
+  # Enable otel tracing
+  local_mocked_bindings(
+    otel_is_tracing_enabled = function() TRUE
+  )
+
+  # Test that changing the option after init doesn't affect the task
+  withr::with_options(list(shiny.otel.collect = "reactivity"), {
+    task <- ex_task_42()
+  })
+
+  # Capture the initial state
+  expect_true(task$.__enclos_env__$private$is_recording_otel)
+
+  # Change the option after initialization
+  withr::with_options(list(shiny.otel.collect = "none"), {
+    # The task should still have the init-time setting
+    expect_true(task$.__enclos_env__$private$is_recording_otel)
+  })
+
+})
+
+test_that("ExtendedTask respects session level otel collection", {
+  # Test that session level doesn't enable reactivity spans
+  withr::local_options(list(shiny.otel.collect = "session"))
+
+  task <- ex_task_42()
+
+  # Should not record otel at session level
+  expect_false(task$.__enclos_env__$private$is_recording_otel)
+})
+
+test_that("ExtendedTask respects reactive_update level otel collection", {
+  # Test that reactive_update level doesn't enable reactivity spans
+  withr::local_options(list(shiny.otel.collect = "reactive_update"))
+
+  task <- ex_task_42()
+
+  # Should not record otel at reactive_update level
+  expect_false(task$.__enclos_env__$private$is_recording_otel)
+})
+
+test_that("ExtendedTask creates span only when is_recording_otel is TRUE", {
+  # Test that span is only created when otel is enabled
+  withr::local_options(list(shiny.otel.collect = "reactivity"))
+
+  span_created <- FALSE
+
+  local_mocked_bindings(
+    start_otel_span = function(...) {
+      span_created <<- TRUE
+      create_mock_otel_span("extended_task")
+    },
+    otel_is_tracing_enabled = function() TRUE
+  )
+
+  with_shiny_otel_record({
+    withReactiveDomain(MockShinySession$new(), {
+      task <- ex_task_42()
+
+      # Reset the flag
+      span_created <- FALSE
+
+      # Invoke the task
+      isolate({
+        task$invoke()
+      })
+    })
+  })
+
+
+  # Span should have been created because is_recording_otel is TRUE
+  expect_true(span_created)
+})
+
+test_that("ExtendedTask does not create span when is_recording_otel is FALSE", {
+  # Test that span is not created when otel is disabled
+  withr::local_options(list(shiny.otel.collect = "none"))
+
+  span_created <- FALSE
+
+  local_mocked_bindings(
+    start_otel_span = function(...) {
+      span_created <<- TRUE
+      create_mock_otel_span("extended_task")
+    }
+  )
+
+  withReactiveDomain(MockShinySession$new(), {
+    task <- ex_task_42()
+
+    # Invoke the task
+    isolate({
+      task$invoke()
+    })
+  })
+
+  # Span should not have been created because is_recording_otel is FALSE
+  expect_false(span_created)
+})
+
+
+test_that("Multiple ExtendedTask invocations use same is_recording_otel value", {
+  # Enable otel tracing
+  withr::local_options(list(shiny.otel.collect = "reactivity"))
+  local_mocked_bindings(
+    otel_is_tracing_enabled = function() TRUE
+  )
+
+  withReactiveDomain(MockShinySession$new(), {
+    task <- ex_task_42()
+
+    # Verify is_recording_otel is TRUE at init
+    expect_true(task$.__enclos_env__$private$is_recording_otel)
+
+    # Change option after initialization (should not affect the task)
+    withr::with_options(
+      list(shiny.otel.collect = "none"),
+      {
+        # The task should still have the init-time setting
+        expect_true(task$.__enclos_env__$private$is_recording_otel)
+
+        # Verify is_recording_otel doesn't change on invocation
+        isolate({
+          task$invoke()
+        })
+
+        # Still should be TRUE after invoke
+        expect_true(task$.__enclos_env__$private$is_recording_otel)
+      }
+    )
+  })
+})

tests/testthat/test-otel-label.R

@@ -68,9 +68,12 @@ test_that("reactive bindCache labels are created", {
 })
 
 test_that("ExtendedTask otel labels are created", {
-  ex_task <- ExtendedTask$new(function() { promises::then(promises::promise_resolve(42), force) })
+  # Record everything
+  localOtelCollect("all")
 
   info <- with_shiny_otel_record({
+    ex_task <- ExtendedTask$new(function() { promises::then(promises::promise_resolve(42), force) })
+
     ex_task$invoke()
     while(!later::loop_empty()) {
       later::run_now()
@@ -79,30 +82,22 @@ test_that("ExtendedTask otel labels are created", {
 
   trace <- info$traces[[1]]
 
-  expect_equal(
-    trace$name,
-    "ExtendedTask ex_task"
-  )
-
+  expect_equal(trace$name, "ExtendedTask ex_task")
 
+  # Module test
   withReactiveDomain(MockShinySession$new(), {
-    ex2_task <- ExtendedTask$new(function() { promises::then(promises::promise_resolve(42), force) })
-
     info <- with_shiny_otel_record({
+      ex2_task <- ExtendedTask$new(function() { promises::then(promises::promise_resolve(42), force) })
       ex2_task$invoke()
       while(!later::loop_empty()) {
         later::run_now()
       }
     })
-
   })
 
   trace <- info$traces[[1]]
 
-  expect_equal(
-    trace$name,
-    "ExtendedTask mock-session:ex2_task"
-  )
+  expect_equal(trace$name, "ExtendedTask mock-session:ex2_task")
 })
 
 

tests/testthat/test-otel-shiny.R

@@ -1,37 +1,5 @@
 # Tests for otel-shiny.R functions
 
-# Helper function to create a mock otel span
-create_mock_otel_span <- function(name = "test_span") {
-  structure(
-    list(
-      name = name,
-      activate = function(...) NULL,
-      end = function(...) NULL
-    ),
-    class = "otel_span"
-  )
-}
-
-# Helper function to create a mock tracer
-create_mock_tracer <- function() {
-  structure(
-    list(
-      name = "mock_tracer",
-      is_enabled = function() TRUE,
-      start_span = function(name, ...) create_mock_otel_span(name)
-    ),
-    class = "otel_tracer"
-  )
-}
-
-# Helper function to create a mock logger
-create_mock_logger <- function() {
-  structure(
-    list(name = "mock_logger"),
-    class = "otel_logger"
-  )
-}
-
 test_that("otel_tracer_name constant is correct", {
   expect_equal(otel_tracer_name, "co.posit.r-package.shiny")
 })