From 390f6d3b9597a024af3b8733ad1c6961fd14fa14 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 16:36:56 -0500 Subject: [PATCH] chore(otel): Rename `shiny.otel.bind` to `shiny.otel.collect` (#4321) Co-authored-by: Barret Schloerke --- DESCRIPTION | 3 +- NEWS.md | 2 +- R/bind-cache.R | 6 +- R/bind-event.R | 10 +- R/extended-task.R | 4 +- R/mock-session.R | 2 +- R/otel-collect.R | 60 +++++++++ R/{otel-bind.R => otel-enable.R} | 83 ++---------- R/otel-reactive-update.R | 2 +- R/otel-session.R | 4 +- R/reactives.R | 44 +++---- R/shiny-options.R | 2 +- R/shiny.R | 2 +- R/shinywrappers.R | 4 +- man/shinyOptions.Rd | 2 +- tests/testthat/test-otel-bind.R | 142 --------------------- tests/testthat/test-otel-collect.R | 142 +++++++++++++++++++++ tests/testthat/test-otel-error.R | 2 +- tests/testthat/test-otel-label.R | 2 +- tests/testthat/test-otel-mock.R | 2 +- tests/testthat/test-otel-reactive-update.R | 14 +- tests/testthat/test-otel-session.R | 20 +-- 22 files changed, 276 insertions(+), 278 deletions(-) create mode 100644 R/otel-collect.R rename R/{otel-bind.R => otel-enable.R} (75%) delete mode 100644 tests/testthat/test-otel-bind.R create mode 100644 tests/testthat/test-otel-collect.R diff --git a/DESCRIPTION b/DESCRIPTION index 2077dda94..cedcdf729 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -184,7 +184,8 @@ Collate: 'modules.R' 'notifications.R' 'otel-attr-srcref.R' - 'otel-bind.R' + 'otel-collect.R' + 'otel-enable.R' 'otel-error.R' 'otel-label.R' 'otel-reactive-update.R' diff --git a/NEWS.md b/NEWS.md index b62f8f3ed..79d9504fb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,7 @@ * Added support for [OpenTelemetry](https://opentelemetry.io/) via [`{otel}`](https://otel.r-lib.org/index.html). By default, if `otel::is_tracing_enabled()` returns `TRUE`, then `{shiny}` will record all OpenTelemetery spans. See [`{otelsdk}`'s Collecting Telemetry Data](https://otelsdk.r-lib.org/reference/collecting.html) for more details on configuring OpenTelemetry. -* Supported values for `options(shiny.otel.bind)` (or `Sys.getenv("SHINY_OTEL_BIND")`): +* Supported values for `options(shiny.otel.collect)` (or `Sys.getenv("SHINY_OTEL_COLLECT")`): * `"none"` - No Shiny OpenTelemetry tracing. * `"session"` - Adds session start/end spans. * `"reactive_update"` - Spans for any synchronous/asynchronous reactive update. (Includes `"session"` features). diff --git a/R/bind-cache.R b/R/bind-cache.R index 202ad6a9f..7fdd20262 100644 --- a/R/bind-cache.R +++ b/R/bind-cache.R @@ -506,7 +506,7 @@ bindCache.reactiveExpr <- function(x, ..., cache = "app") { rm(list = ".", envir = .GenericCallEnv, inherits = FALSE) } - with_no_otel_bind({ + with_no_otel_collect({ res <- reactive(label = label, domain = domain, { cache <- resolve_cache_object(cache, domain) hybrid_chain( @@ -523,8 +523,8 @@ bindCache.reactiveExpr <- function(x, ..., cache = "app") { impl$.otelAttrs <- append_otel_srcref_attrs(x_otel_attrs, call_srcref) }) - if (has_otel_bind("reactivity")) { - res <- bind_otel_reactive_expr(res) + if (has_otel_collect("reactivity")) { + res <- enable_otel_reactive_expr(res) } res } diff --git a/R/bind-event.R b/R/bind-event.R index 3daea0a10..df93ef301 100644 --- a/R/bind-event.R +++ b/R/bind-event.R @@ -216,7 +216,7 @@ bindEvent.reactiveExpr <- function(x, ..., ignoreNULL = TRUE, ignoreInit = FALSE initialized <- FALSE - with_no_otel_bind({ + with_no_otel_collect({ res <- reactive(label = label, domain = domain, ..stacktraceon = FALSE, { hybrid_chain( { @@ -244,8 +244,8 @@ bindEvent.reactiveExpr <- function(x, ..., ignoreNULL = TRUE, ignoreInit = FALSE }) - if (has_otel_bind("reactivity")) { - res <- bind_otel_reactive_expr(res) + if (has_otel_collect("reactivity")) { + res <- enable_otel_reactive_expr(res) } res @@ -343,8 +343,8 @@ bindEvent.Observer <- function(x, ..., ignoreNULL = TRUE, ignoreInit = FALSE, call_srcref <- get_call_srcref(-1) x$.otelAttrs <- append_otel_srcref_attrs(x$.otelAttrs, call_srcref) - if (has_otel_bind("reactivity")) { - x <- bind_otel_observe(x) + if (has_otel_collect("reactivity")) { + x <- enable_otel_observe(x) } invisible(x) diff --git a/R/extended-task.R b/R/extended-task.R index dc7db3e4c..4b3069ac7 100644 --- a/R/extended-task.R +++ b/R/extended-task.R @@ -118,7 +118,7 @@ ExtendedTask <- R6Class("ExtendedTask", portable = TRUE, cloneable = FALSE, private$func <- func # Do not show these private reactive values in otel spans - with_no_otel_bind({ + with_no_otel_collect({ private$rv_status <- reactiveVal("initial", label = "ExtendedTask$private$status") private$rv_value <- reactiveVal(NULL, label = "ExtendedTask$private$value") private$rv_error <- reactiveVal(NULL, label = "ExtendedTask$private$error") @@ -175,7 +175,7 @@ ExtendedTask <- R6Class("ExtendedTask", portable = TRUE, cloneable = FALSE, private$invocation_queue$add(list(args = args, call = call)) } else { - if (has_otel_bind("reactivity")) { + if (has_otel_collect("reactivity")) { private$otel_span <- start_otel_span( private$otel_span_label, attributes = private$otel_attrs diff --git a/R/mock-session.R b/R/mock-session.R index fdcc188a4..5f0977511 100644 --- a/R/mock-session.R +++ b/R/mock-session.R @@ -436,7 +436,7 @@ MockShinySession <- R6Class( if (!is.function(func)) stop(paste("Unexpected", class(func), "output for", name)) - with_no_otel_bind({ + with_no_otel_collect({ obs <- observe({ # We could just stash the promise, but we get an "unhandled promise error". This bypasses prom <- NULL diff --git a/R/otel-collect.R b/R/otel-collect.R new file mode 100644 index 000000000..2dcb6efe8 --- /dev/null +++ b/R/otel-collect.R @@ -0,0 +1,60 @@ +otel_collect_choices <- c( + "none", + "session", + "reactive_update", + "reactivity", + "all" +) + +# Check if the collect level is sufficient +otel_collect_is_enabled <- function( + impl_level, + # Listen to option and fall back to the env var + opt_collect_level = getOption("shiny.otel.collect", Sys.getenv("SHINY_OTEL_COLLECT", "all")) +) { + opt_collect_level <- as_otel_collect(opt_collect_level) + + which(opt_collect_level == otel_collect_choices) >= + which(impl_level == otel_collect_choices) +} + +# Check if tracing is enabled and if the collect level is sufficient +has_otel_collect <- function(collect) { + # Only check pkg author input iff loaded with pkgload + if (IS_SHINY_LOCAL_PKG) { + stopifnot(length(collect) == 1, any(collect == otel_collect_choices)) + } + + otel_is_tracing_enabled() && otel_collect_is_enabled(collect) +} + +# Run expr with otel collection disabled +with_no_otel_collect <- function(expr) { + withr::with_options( + list( + shiny.otel.collect = "none" + ), + expr + ) +} + + +## -- Helpers ----------------------------------------------------- + +# shiny.otel.collect can be: +# "none"; To do nothing / fully opt-out +# "session" for session/start events +# "reactive_update" (includes "session" features) and reactive_update spans +# "reactivity" (includes "reactive_update" features) and spans for all reactive things +# "all" - Anything that Shiny can do. (Currently equivalent to the "reactivity" level) + +as_otel_collect <- function(collect = "all") { + if (!is.character(collect)) { + stop("`collect` must be a character vector.") + } + + # Match to collect enum + collect <- match.arg(collect, otel_collect_choices, several.ok = FALSE) + + return(collect) +} diff --git a/R/otel-bind.R b/R/otel-enable.R similarity index 75% rename from R/otel-bind.R rename to R/otel-enable.R index b7b954ce0..55473ed13 100644 --- a/R/otel-bind.R +++ b/R/otel-enable.R @@ -1,67 +1,3 @@ -otel_bind_choices <- c( - "none", - "session", - "reactive_update", - "reactivity", - "all" -) - -# Check if the bind level is sufficient -otel_bind_is_enabled <- function( - impl_level, - # Listen to option and fall back to the env var - opt_bind_level = getOption("shiny.otel.bind", Sys.getenv("SHINY_OTEL_BIND", "all")) -) { - opt_bind_level <- as_otel_bind(opt_bind_level) - - which(opt_bind_level == otel_bind_choices) >= - which(impl_level == otel_bind_choices) -} - -# Check if tracing is enabled and if the bind level is sufficient -has_otel_bind <- function(bind) { - # Only check pkg author input iff loaded with pkgload - if (IS_SHINY_LOCAL_PKG) { - stopifnot(length(bind) == 1, any(bind == otel_bind_choices)) - } - - otel_is_tracing_enabled() && otel_bind_is_enabled(bind) -} - -# Run expr with otel binding disabled -with_no_otel_bind <- function(expr) { - withr::with_options( - list( - shiny.otel.bind = "none" - ), - expr - ) -} - - -## -- Helpers ----------------------------------------------------- - -# shiny.otel.bind can be: -# "none"; To do nothing / fully opt-out -# "session" for session/start events -# "reactive_update" (includes "session" features) and reactive_update spans -# "reactivity" (includes "reactive_update" features) and spans for all reactive things -# "all" - Anything that Shiny can do. (Currently equivalent to the "reactivity" level) - -as_otel_bind <- function(bind = "all") { - if (!is.character(bind)) { - stop("`bind` must be a character vector.") - } - - # Match to bind enum - bind <- match.arg(bind, otel_bind_choices, several.ok = FALSE) - - return(bind) -} - - -# ------------------------------------------ - # # Approach # Use flags on the reactive object to indicate whether to record OpenTelemetry spans. # @@ -75,7 +11,7 @@ as_otel_bind <- function(bind = "all") { #' #' @description #' -#' `bind_otel_*()` methods add OpenTelemetry flags for [reactive()] expressions +#' `enable_otel_*()` methods add OpenTelemetry flags for [reactive()] expressions #' and `render*` functions (like [renderText()], [renderTable()], ...). #' #' Wrapper to creating an active reactive OpenTelemetry span that closes when @@ -115,7 +51,7 @@ as_otel_bind <- function(bind = "all") { #' Dev note - Barret 2025-10: #' Typically, an OpenTelemetry span (`otel_span`) will inherit from the parent #' span. This works well and we can think of the hierarchy as a tree. With -#' `options("shiny.otel.bind" = )`, we are able to control with a sliding +#' `options("shiny.otel.collect" = )`, we are able to control with a sliding #' dial how much of the tree we are interested in: "none", "session", #' "reactive_update", "reactivity", and finally "all". #' @@ -152,18 +88,19 @@ as_otel_bind <- function(bind = "all") { #' create it themselves and let natural inheritance take over. #' #' Given this, I will imagine that app authors will set -#' `options("shiny.otel.bind" = "reactive_update")` as their default behavior. +#' `options("shiny.otel.collect" = "reactive_update")` as their default behavior. #' Enough to know things are happening, but not overwhelming from **everything** #' that is reactive. #' -#' To _light up_ a specific area, users can call `withr::with_options(list("shiny.otel.bind" = "all"), { ... })`. +#' To _light up_ a specific area, users can call `withr::with_options(list("shiny.otel.collect" = "all"), { ... })`. #' #' @param x The object to add caching to. #' @param ... Future parameter expansion. #' @noRd NULL -bind_otel_reactive_val <- function(x) { + +enable_otel_reactive_val <- function(x) { impl <- attr(x, ".impl", exact = TRUE) # Set flag for otel logging when setting the value @@ -174,7 +111,7 @@ bind_otel_reactive_val <- function(x) { x } -bind_otel_reactive_values <- function(x) { +enable_otel_reactive_values <- function(x) { impl <- .subset2(x, "impl") # Set flag for otel logging when setting values @@ -185,7 +122,7 @@ bind_otel_reactive_values <- function(x) { x } -bind_otel_reactive_expr <- function(x) { +enable_otel_reactive_expr <- function(x) { domain <- reactive_get_domain(x) @@ -199,7 +136,7 @@ bind_otel_reactive_expr <- function(x) { x } -bind_otel_observe <- function(x) { +enable_otel_observe <- function(x) { x$.isRecordingOtel <- TRUE x$.otelLabel <- otel_span_label_observer(x, domain = x$.domain) @@ -209,7 +146,7 @@ bind_otel_observe <- function(x) { -bind_otel_shiny_render_function <- function(x) { +enable_otel_shiny_render_function <- function(x) { valueFunc <- force(x) otel_span_label <- NULL diff --git a/R/otel-reactive-update.R b/R/otel-reactive-update.R index ee273b3a7..9d5923e47 100644 --- a/R/otel-reactive-update.R +++ b/R/otel-reactive-update.R @@ -12,7 +12,7 @@ #' @noRd otel_span_reactive_update_init <- function(..., domain) { - if (!has_otel_bind("reactive_update")) return() + if (!has_otel_collect("reactive_update")) return() # Ensure cleanup is registered only once per session if (is.null(domain$userData[["_otel_has_reactive_cleanup"]])) { diff --git a/R/otel-session.R b/R/otel-session.R index 502eb62b6..4fb81f0da 100644 --- a/R/otel-session.R +++ b/R/otel-session.R @@ -12,7 +12,7 @@ #' @noRd otel_span_session_start <- function(expr, ..., domain) { - if (!has_otel_bind("session")) { + if (!has_otel_collect("session")) { return(force(expr)) } @@ -29,7 +29,7 @@ otel_span_session_start <- function(expr, ..., domain) { otel_span_session_end <- function(expr, ..., domain) { - if (!has_otel_bind("session")) { + if (!has_otel_collect("session")) { return(force(expr)) } diff --git a/R/reactives.R b/R/reactives.R index 23d365cf0..25f2d5c91 100644 --- a/R/reactives.R +++ b/R/reactives.R @@ -248,8 +248,8 @@ reactiveVal <- function(value = NULL, label = NULL) { .impl = rv ) - if (has_otel_bind("reactivity")) { - ret <- bind_otel_reactive_val(ret) + if (has_otel_collect("reactivity")) { + ret <- enable_otel_reactive_val(ret) } ret @@ -651,8 +651,8 @@ reactiveValues <- function(...) { impl$mset(args) - # Add otel binding after `$mset()` so that we don't log the initial values - # Add otel binding after `.label` so that any logging uses the correct label + # Add otel collection after `$mset()` so that we don't log the initial values + # Add otel collection after `.label` so that any logging uses the correct label values <- maybeAddReactiveValuesOtel(values) values @@ -669,7 +669,7 @@ checkName <- function(x) { # @param values A ReactiveValues object # @param readonly Should this object be read-only? # @param ns A namespace function (either `identity` or `NS(namespace)`) -# @param withOtel Should otel binding be attempted? +# @param withOtel Should otel collection be attempted? .createReactiveValues <- function(values = NULL, readonly = FALSE, ns = identity, withOtel = TRUE) { @@ -690,11 +690,11 @@ checkName <- function(x) { } maybeAddReactiveValuesOtel <- function(x) { - if (!has_otel_bind("reactivity")) { + if (!has_otel_collect("reactivity")) { return(x) } - bind_otel_reactive_values(x) + enable_otel_reactive_values(x) } #' @export @@ -1140,8 +1140,8 @@ reactive <- function( class = c("reactiveExpr", "reactive", "function") ) - if (has_otel_bind("reactivity")) { - ret <- bind_otel_reactive_expr(ret) + if (has_otel_collect("reactivity")) { + ret <- enable_otel_reactive_expr(ret) } ret @@ -1590,8 +1590,8 @@ observe <- function( o$.otelAttrs <- otel_srcref_attributes(call_srcref) } - if (has_otel_bind("reactivity")) { - o <- bind_otel_observe(o) + if (has_otel_collect("reactivity")) { + o <- enable_otel_observe(o) } invisible(o) @@ -2011,7 +2011,7 @@ reactive_poll_impl <- function( re_finalized <- FALSE env <- environment() - with_no_otel_bind({ + with_no_otel_collect({ cookie <- reactiveVal( isolate(checkFunc()), label = sprintf("%s %s cookie", fnName, label) @@ -2500,7 +2500,7 @@ observeEvent <- function(eventExpr, handlerExpr, ) } - with_no_otel_bind({ + with_no_otel_collect({ handler <- inject(observe( !!handlerQ, label = label, @@ -2524,8 +2524,8 @@ observeEvent <- function(eventExpr, handlerExpr, if (!is.null(call_srcref)) { o$.otelAttrs <- otel_srcref_attributes(call_srcref) } - if (has_otel_bind("reactivity")) { - o <- bind_otel_observe(o) + if (has_otel_collect("reactivity")) { + o <- enable_otel_observe(o) } invisible(o) @@ -2557,7 +2557,7 @@ eventReactive <- function(eventExpr, valueExpr, ) } - with_no_otel_bind({ + with_no_otel_collect({ value_r <- inject(reactive(!!valueQ, domain = domain, label = label)) r <- inject(bindEvent( @@ -2573,8 +2573,8 @@ eventReactive <- function(eventExpr, valueExpr, impl <- attr(r, "observable", exact = TRUE) impl$.otelAttrs <- otel_srcref_attributes(call_srcref) } - if (has_otel_bind("reactivity")) { - r <- bind_otel_reactive_expr(r) + if (has_otel_collect("reactivity")) { + r <- enable_otel_reactive_expr(r) } @@ -2710,7 +2710,7 @@ debounce <- function(r, millis, priority = 100, domain = getDefaultReactiveDomai millis <- function() origMillis } - with_no_otel_bind({ + with_no_otel_collect({ trigger <- reactiveVal(NULL, label = sprintf("debounce %s trigger", label)) # the deadline for the timer to fire; NULL if not scheduled when <- reactiveVal(NULL, label = sprintf("debounce %s when", label)) @@ -2781,7 +2781,7 @@ debounce <- function(r, millis, priority = 100, domain = getDefaultReactiveDomai er_impl$.otelAttrs <- append_otel_srcref_attrs(er_impl$.otelAttrs, call_srcref) }) - with_no_otel_bind({ + with_no_otel_collect({ # Force the value of er to be immediately cached upon creation. It's very hard # to explain why this observer is needed, but if you want to understand, try # commenting it out and studying the unit test failure that results. @@ -2814,7 +2814,7 @@ throttle <- function(r, millis, priority = 100, domain = getDefaultReactiveDomai millis <- function() origMillis } - with_no_otel_bind({ + with_no_otel_collect({ trigger <- reactiveVal(0, label = sprintf("throttle %s trigger", label)) # Last time we fired; NULL if never lastTriggeredAt <- reactiveVal(NULL, label = sprintf("throttle %s last triggered at", label)) @@ -2837,7 +2837,7 @@ throttle <- function(r, millis, priority = 100, domain = getDefaultReactiveDomai pending(FALSE) } - with_no_otel_bind({ + with_no_otel_collect({ # Responsible for tracking when f() changes. observeEvent(try(r(), silent = TRUE), { if (pending()) { diff --git a/R/shiny-options.R b/R/shiny-options.R index 63c77c109..55d4c530b 100644 --- a/R/shiny-options.R +++ b/R/shiny-options.R @@ -160,7 +160,7 @@ getShinyOption <- function(name, default = NULL) { # ' side devmode features. Currently the primary feature is the client-side # ' error console.} ### end shiny.client_devmode -#' \item{shiny.otel.bind (defaults to `Sys.getenv("SHINY_OTEL_BIND", "all")`)}{Determines how Shiny will +#' \item{shiny.otel.collect (defaults to `Sys.getenv("SHINY_OTEL_COLLECT", "all")`)}{Determines how Shiny will #' interact with OpenTelemetry. #' #' Supported values: diff --git a/R/shiny.R b/R/shiny.R index 3965138ef..d8cc2330c 100644 --- a/R/shiny.R +++ b/R/shiny.R @@ -1158,7 +1158,7 @@ ShinySession <- R6Class( attr(label, "srcfile") <- srcfile # Do not bind this `observe()` call - obs <- with_no_otel_bind(observe(..stacktraceon = FALSE, { + obs <- with_no_otel_collect(observe(..stacktraceon = FALSE, { private$sendMessage(recalculating = list( name = name, status = 'recalculating' diff --git a/R/shinywrappers.R b/R/shinywrappers.R index 6edad5319..b3d1684c1 100644 --- a/R/shinywrappers.R +++ b/R/shinywrappers.R @@ -151,8 +151,8 @@ markRenderFunction <- function( otelAttrs = otelAttrs ) - if (has_otel_bind("reactivity")) { - ret <- bind_otel_shiny_render_function(ret) + if (has_otel_collect("reactivity")) { + ret <- enable_otel_shiny_render_function(ret) } ret diff --git a/man/shinyOptions.Rd b/man/shinyOptions.Rd index b4b1077b1..821316f7a 100644 --- a/man/shinyOptions.Rd +++ b/man/shinyOptions.Rd @@ -130,7 +130,7 @@ ragg package. See \code{\link[=plotPNG]{plotPNG()}} for more information.} Cairo package. See \code{\link[=plotPNG]{plotPNG()}} for more information.} \item{shiny.devmode (defaults to \code{NULL})}{Option to enable Shiny Developer Mode. When set, different default \code{getOption(key)} values will be returned. See \code{\link[=devmode]{devmode()}} for more details.} -\item{shiny.otel.bind (defaults to \code{Sys.getenv("SHINY_OTEL_BIND", "all")})}{Determines how Shiny will +\item{shiny.otel.collect (defaults to \code{Sys.getenv("SHINY_OTEL_COLLECT", "all")})}{Determines how Shiny will interact with OpenTelemetry. Supported values: diff --git a/tests/testthat/test-otel-bind.R b/tests/testthat/test-otel-bind.R deleted file mode 100644 index 7d343998a..000000000 --- a/tests/testthat/test-otel-bind.R +++ /dev/null @@ -1,142 +0,0 @@ -test_that("otel_bind_is_enabled works with valid bind levels", { - # Test with default "all" option - expect_true(otel_bind_is_enabled("none")) - expect_true(otel_bind_is_enabled("session")) - expect_true(otel_bind_is_enabled("reactive_update")) - expect_true(otel_bind_is_enabled("reactivity")) - expect_true(otel_bind_is_enabled("all")) -}) - -test_that("otel_bind_is_enabled respects hierarchy with 'none' option", { - # With "none" option, nothing should be enabled - expect_false(otel_bind_is_enabled("session", "none")) - expect_false(otel_bind_is_enabled("reactive_update", "none")) - expect_false(otel_bind_is_enabled("reactivity", "none")) - expect_false(otel_bind_is_enabled("all", "none")) - expect_true(otel_bind_is_enabled("none", "none")) -}) - -test_that("otel_bind_is_enabled respects hierarchy with 'session' option", { - # With "session" option, only "none" and "session" should be enabled - expect_true(otel_bind_is_enabled("none", "session")) - expect_true(otel_bind_is_enabled("session", "session")) - expect_false(otel_bind_is_enabled("reactive_update", "session")) - expect_false(otel_bind_is_enabled("reactivity", "session")) - expect_false(otel_bind_is_enabled("all", "session")) -}) - -test_that("otel_bind_is_enabled respects hierarchy with 'reactive_update' option", { - # With "reactive_update" option, "none", "session", and "reactive_update" should be enabled - expect_true(otel_bind_is_enabled("none", "reactive_update")) - expect_true(otel_bind_is_enabled("session", "reactive_update")) - expect_true(otel_bind_is_enabled("reactive_update", "reactive_update")) - expect_false(otel_bind_is_enabled("reactivity", "reactive_update")) - expect_false(otel_bind_is_enabled("all", "reactive_update")) -}) - -test_that("otel_bind_is_enabled respects hierarchy with 'reactivity' option", { - # With "reactivity" option, all except "all" should be enabled - expect_true(otel_bind_is_enabled("none", "reactivity")) - expect_true(otel_bind_is_enabled("session", "reactivity")) - expect_true(otel_bind_is_enabled("reactive_update", "reactivity")) - expect_true(otel_bind_is_enabled("reactivity", "reactivity")) - expect_false(otel_bind_is_enabled("all", "reactivity")) -}) - -test_that("otel_bind_is_enabled respects hierarchy with 'all' option", { - # With "all" option (default), everything should be enabled - expect_true(otel_bind_is_enabled("none", "all")) - expect_true(otel_bind_is_enabled("session", "all")) - expect_true(otel_bind_is_enabled("reactive_update", "all")) - expect_true(otel_bind_is_enabled("reactivity", "all")) - expect_true(otel_bind_is_enabled("all", "all")) -}) - -test_that("otel_bind_is_enabled uses shiny.otel.bind option", { - # Test that option is respected - withr::with_options( - list(shiny.otel.bind = "session"), - { - expect_true(otel_bind_is_enabled("none")) - expect_true(otel_bind_is_enabled("session")) - expect_false(otel_bind_is_enabled("reactive_update")) - } - ) - - withr::with_options( - list(shiny.otel.bind = "reactivity"), - { - expect_true(otel_bind_is_enabled("reactive_update")) - expect_true(otel_bind_is_enabled("reactivity")) - expect_false(otel_bind_is_enabled("all")) - } - ) -}) - -test_that("otel_bind_is_enabled falls back to SHINY_OTEL_BIND env var", { - # Remove option to test env var fallback - withr::local_options(list(shiny.otel.bind = NULL)) - - # Test env var is respected - withr::local_envvar(list(SHINY_OTEL_BIND = "session")) - expect_true(otel_bind_is_enabled("none")) - expect_true(otel_bind_is_enabled("session")) - expect_false(otel_bind_is_enabled("reactive_update")) - - withr::local_envvar(list(SHINY_OTEL_BIND = "none")) - expect_true(otel_bind_is_enabled("none")) - expect_false(otel_bind_is_enabled("session")) -}) - -test_that("otel_bind_is_enabled option takes precedence over env var", { - # Set conflicting option and env var - withr::local_options(shiny.otel.bind = "session") - withr::local_envvar(SHINY_OTEL_BIND = "all") - - # Option should take precedence - expect_true(otel_bind_is_enabled("session")) - expect_false(otel_bind_is_enabled("reactive_update")) -}) - -test_that("otel_bind_is_enabled defaults to 'all' when no option or env var", { - # Remove both option and env var - withr::local_options(list(shiny.otel.bind = NULL)) - withr::local_envvar(list(SHINY_OTEL_BIND = NA)) - - # Should default to "all" - expect_true(otel_bind_is_enabled("all")) - expect_true(otel_bind_is_enabled("reactivity")) - expect_true(otel_bind_is_enabled("none")) -}) - -# Tests for as_otel_bind() -test_that("as_otel_bind validates and returns valid bind levels", { - expect_equal(as_otel_bind("none"), "none") - expect_equal(as_otel_bind("session"), "session") - expect_equal(as_otel_bind("reactive_update"), "reactive_update") - expect_equal(as_otel_bind("reactivity"), "reactivity") - expect_equal(as_otel_bind("all"), "all") -}) - -test_that("as_otel_bind uses default value", { - expect_equal(as_otel_bind(), "all") -}) - -test_that("as_otel_bind errors on invalid input types", { - expect_error(as_otel_bind(123), "`bind` must be a character vector.") - expect_error(as_otel_bind(NULL), "`bind` must be a character vector.") - expect_error(as_otel_bind(TRUE), "`bind` must be a character vector.") - expect_error(as_otel_bind(list("all")), "`bind` must be a character vector.") -}) - -test_that("as_otel_bind errors on invalid bind levels", { - expect_error(as_otel_bind("invalid"), "'arg' should be one of") - expect_error(as_otel_bind("unknown"), "'arg' should be one of") - expect_error(as_otel_bind(""), "'arg' should be one of") -}) - -test_that("as_otel_bind errors on multiple values", { - # match.arg with several.ok = FALSE should error on multiple values - expect_error(as_otel_bind(c("all", "none")), "'arg' must be of length 1") - expect_error(as_otel_bind(c("session", "reactivity")), "'arg' must be of length 1") -}) diff --git a/tests/testthat/test-otel-collect.R b/tests/testthat/test-otel-collect.R new file mode 100644 index 000000000..38a551c2e --- /dev/null +++ b/tests/testthat/test-otel-collect.R @@ -0,0 +1,142 @@ +test_that("otel_collect_is_enabled works with valid collect levels", { + # Test with default "all" option + expect_true(otel_collect_is_enabled("none")) + expect_true(otel_collect_is_enabled("session")) + expect_true(otel_collect_is_enabled("reactive_update")) + expect_true(otel_collect_is_enabled("reactivity")) + expect_true(otel_collect_is_enabled("all")) +}) + +test_that("otel_collect_is_enabled respects hierarchy with 'none' option", { + # With "none" option, nothing should be enabled + expect_false(otel_collect_is_enabled("session", "none")) + expect_false(otel_collect_is_enabled("reactive_update", "none")) + expect_false(otel_collect_is_enabled("reactivity", "none")) + expect_false(otel_collect_is_enabled("all", "none")) + expect_true(otel_collect_is_enabled("none", "none")) +}) + +test_that("otel_collect_is_enabled respects hierarchy with 'session' option", { + # With "session" option, only "none" and "session" should be enabled + expect_true(otel_collect_is_enabled("none", "session")) + expect_true(otel_collect_is_enabled("session", "session")) + expect_false(otel_collect_is_enabled("reactive_update", "session")) + expect_false(otel_collect_is_enabled("reactivity", "session")) + expect_false(otel_collect_is_enabled("all", "session")) +}) + +test_that("otel_collect_is_enabled respects hierarchy with 'reactive_update' option", { + # With "reactive_update" option, "none", "session", and "reactive_update" should be enabled + expect_true(otel_collect_is_enabled("none", "reactive_update")) + expect_true(otel_collect_is_enabled("session", "reactive_update")) + expect_true(otel_collect_is_enabled("reactive_update", "reactive_update")) + expect_false(otel_collect_is_enabled("reactivity", "reactive_update")) + expect_false(otel_collect_is_enabled("all", "reactive_update")) +}) + +test_that("otel_collect_is_enabled respects hierarchy with 'reactivity' option", { + # With "reactivity" option, all except "all" should be enabled + expect_true(otel_collect_is_enabled("none", "reactivity")) + expect_true(otel_collect_is_enabled("session", "reactivity")) + expect_true(otel_collect_is_enabled("reactive_update", "reactivity")) + expect_true(otel_collect_is_enabled("reactivity", "reactivity")) + expect_false(otel_collect_is_enabled("all", "reactivity")) +}) + +test_that("otel_collect_is_enabled respects hierarchy with 'all' option", { + # With "all" option (default), everything should be enabled + expect_true(otel_collect_is_enabled("none", "all")) + expect_true(otel_collect_is_enabled("session", "all")) + expect_true(otel_collect_is_enabled("reactive_update", "all")) + expect_true(otel_collect_is_enabled("reactivity", "all")) + expect_true(otel_collect_is_enabled("all", "all")) +}) + +test_that("otel_collect_is_enabled uses shiny.otel.collect option", { + # Test that option is respected + withr::with_options( + list(shiny.otel.collect = "session"), + { + expect_true(otel_collect_is_enabled("none")) + expect_true(otel_collect_is_enabled("session")) + expect_false(otel_collect_is_enabled("reactive_update")) + } + ) + + withr::with_options( + list(shiny.otel.collect = "reactivity"), + { + expect_true(otel_collect_is_enabled("reactive_update")) + expect_true(otel_collect_is_enabled("reactivity")) + expect_false(otel_collect_is_enabled("all")) + } + ) +}) + +test_that("otel_collect_is_enabled falls back to SHINY_OTEL_COLLECT env var", { + # Remove option to test env var fallback + withr::local_options(list(shiny.otel.collect = NULL)) + + # Test env var is respected + withr::local_envvar(list(SHINY_OTEL_COLLECT = "session")) + expect_true(otel_collect_is_enabled("none")) + expect_true(otel_collect_is_enabled("session")) + expect_false(otel_collect_is_enabled("reactive_update")) + + withr::local_envvar(list(SHINY_OTEL_COLLECT = "none")) + expect_true(otel_collect_is_enabled("none")) + expect_false(otel_collect_is_enabled("session")) +}) + +test_that("otel_collect_is_enabled option takes precedence over env var", { + # Set conflicting option and env var + withr::local_options(shiny.otel.collect = "session") + withr::local_envvar(SHINY_OTEL_COLLECT = "all") + + # Option should take precedence + expect_true(otel_collect_is_enabled("session")) + expect_false(otel_collect_is_enabled("reactive_update")) +}) + +test_that("otel_collect_is_enabled defaults to 'all' when no option or env var", { + # Remove both option and env var + withr::local_options(list(shiny.otel.collect = NULL)) + withr::local_envvar(list(SHINY_OTEL_COLLECT = NA)) + + # Should default to "all" + expect_true(otel_collect_is_enabled("all")) + expect_true(otel_collect_is_enabled("reactivity")) + expect_true(otel_collect_is_enabled("none")) +}) + +# Tests for as_otel_collect() +test_that("as_otel_collect validates and returns valid collect levels", { + expect_equal(as_otel_collect("none"), "none") + expect_equal(as_otel_collect("session"), "session") + expect_equal(as_otel_collect("reactive_update"), "reactive_update") + expect_equal(as_otel_collect("reactivity"), "reactivity") + expect_equal(as_otel_collect("all"), "all") +}) + +test_that("as_otel_collect uses default value", { + expect_equal(as_otel_collect(), "all") +}) + +test_that("as_otel_collect errors on invalid input types", { + expect_error(as_otel_collect(123), "`collect` must be a character vector.") + expect_error(as_otel_collect(NULL), "`collect` must be a character vector.") + expect_error(as_otel_collect(TRUE), "`collect` must be a character vector.") + expect_error(as_otel_collect(list("all")), "`collect` must be a character vector.") +}) + +test_that("as_otel_collect errors on invalid collect levels", { + expect_error(as_otel_collect("invalid"), "'arg' should be one of") + expect_error(as_otel_collect("unknown"), "'arg' should be one of") + expect_error(as_otel_collect(""), "'arg' should be one of") +}) + +test_that("as_otel_collect errors on multiple values", { + # match.arg with several.ok = FALSE should error on multiple values + expect_error(as_otel_collect(c("all", "none")), "'arg' must be of length 1") + expect_error(as_otel_collect(c("session", "reactivity")), "'arg' must be of length 1") +}) diff --git a/tests/testthat/test-otel-error.R b/tests/testthat/test-otel-error.R index 552488786..a6ee45762 100644 --- a/tests/testthat/test-otel-error.R +++ b/tests/testthat/test-otel-error.R @@ -39,7 +39,7 @@ test_server_with_otel_error <- function(session, server, expr, sanitize = FALSE, withr::with_options( list( - shiny.otel.bind = "all", + shiny.otel.collect = "all", shiny.otel.sanitize.errors = sanitize ), { diff --git a/tests/testthat/test-otel-label.R b/tests/testthat/test-otel-label.R index aa66b9c1c..59bbe58d9 100644 --- a/tests/testthat/test-otel-label.R +++ b/tests/testthat/test-otel-label.R @@ -1,4 +1,4 @@ -# Tests for label methods used in otel-bind.R +# Tests for label methods used in otel-collect.R test_that("otel_span_label_reactive generates correct labels", { # Create mock reactive with observable attribute x_reactive <- reactive({ 42 }) diff --git a/tests/testthat/test-otel-mock.R b/tests/testthat/test-otel-mock.R index 1b8d6c183..0bc30d2f8 100644 --- a/tests/testthat/test-otel-mock.R +++ b/tests/testthat/test-otel-mock.R @@ -46,7 +46,7 @@ test_server_with_otel <- function(session, server, expr, bind = "all", args = li stopifnot(inherits(session, "MockShinySession")) stopifnot(is.function(server)) - withr::with_options(list(shiny.otel.bind = bind), { + withr::with_options(list(shiny.otel.collect = bind), { info <- with_shiny_otel_record({ # rlang quosure magic to capture and pass through `expr` testServer(server, {{ expr }}, args = args, session = session) diff --git a/tests/testthat/test-otel-reactive-update.R b/tests/testthat/test-otel-reactive-update.R index e90f9c8f3..038d32280 100644 --- a/tests/testthat/test-otel-reactive-update.R +++ b/tests/testthat/test-otel-reactive-update.R @@ -11,8 +11,8 @@ create_mock_otel_span <- function(name, attributes = NULL, ended = FALSE) { test_that("otel_span_reactive_update_init returns early when otel not enabled", { domain <- MockShinySession$new() - # Convince has_otel_bind to return FALSE - withr::local_options(list(shiny.otel.bind = "none")) + # Convince has_otel_collect to return FALSE + withr::local_options(list(shiny.otel.collect = "none")) # Should return early without creating span result <- otel_span_reactive_update_init(domain = domain) @@ -37,10 +37,10 @@ test_that("otel_span_reactive_update_init sets up session cleanup on first call" ) domain <- TestMockShinySession$new() - withr::local_options(list(shiny.otel.bind = "reactive_update")) + withr::local_options(list(shiny.otel.collect = "reactive_update")) local_mocked_bindings( - has_otel_bind = function(level) level == "reactive_update", + has_otel_collect = function(level) level == "reactive_update", start_otel_span = function(name, ..., attributes = NULL) create_mock_otel_span(name, attributes = attributes), otel_session_id_attrs = function(domain) list(session_id = "mock-session-id") ) @@ -64,7 +64,7 @@ test_that("otel_span_reactive_update_init errors when span already exists", { domain$userData[["_otel_span_reactive_update"]] <- existing_otel_span local_mocked_bindings( - has_otel_bind = function(level) level == "reactive_update" + has_otel_collect = function(level) level == "reactive_update" ) expect_error( @@ -94,7 +94,7 @@ test_that("otel_span_reactive_update_init doesn't setup cleanup twice", { domain$userData[["_otel_has_reactive_cleanup"]] <- TRUE local_mocked_bindings( - has_otel_bind = function(level) level == "reactive_update", + has_otel_collect = function(level) level == "reactive_update", start_otel_span = function(...) create_mock_otel_span("reactive_update") ) @@ -199,7 +199,7 @@ test_that("session cleanup callback works correctly", { mock_otel_span <- create_mock_otel_span("reactive_update") with_mocked_bindings( - has_otel_bind = function(level) level == "reactive_update", + has_otel_collect = function(level) level == "reactive_update", start_otel_span = function(...) mock_otel_span, otel_session_id_attrs = function(domain) list(session_id = "test"), { diff --git a/tests/testthat/test-otel-session.R b/tests/testthat/test-otel-session.R index 28aa38608..3b3b55adb 100644 --- a/tests/testthat/test-otel-session.R +++ b/tests/testthat/test-otel-session.R @@ -44,8 +44,8 @@ test_that("otel_span_session_start returns early when otel not enabled", { domain <- create_mock_session_domain() test_value <- "initial" - # Mock has_otel_bind to return FALSE - withr::local_options(list(shiny.otel.bind = "none")) + # Mock has_otel_collect to return FALSE + withr::local_options(list(shiny.otel.collect = "none")) result <- otel_span_session_start({ test_value <- "modified" @@ -67,7 +67,7 @@ test_that("otel_span_session_start sets up session end callback", { test_value <- "initial" # Mock dependencies - withr::local_options(list(shiny.otel.bind = "session")) + withr::local_options(list(shiny.otel.collect = "session")) local_mocked_bindings( as_attributes = function(x) x, @@ -75,7 +75,7 @@ test_that("otel_span_session_start sets up session end callback", { ) with_mocked_bindings( - has_otel_bind = function(level) level == "session", + has_otel_collect = function(level) level == "session", otel_session_id_attrs = function(domain) list(session.id = domain$token), otel_session_attrs = function(domain) list(PATH_INFO = "/app"), with_otel_span = function(name, expr, attributes = NULL) { @@ -105,8 +105,8 @@ test_that("otel_span_session_end returns early when otel not enabled", { domain <- create_mock_session_domain() test_value <- "initial" - # Mock has_otel_bind to return FALSE - withr::local_options(list(shiny.otel.bind = "none")) + # Mock has_otel_collect to return FALSE + withr::local_options(list(shiny.otel.collect = "none")) result <- otel_span_session_end({ test_value <- "modified" @@ -124,10 +124,10 @@ test_that("otel_span_session_end creates span when enabled", { test_value <- "initial" # Mock dependencies - withr::local_options(list(shiny.otel.bind = "session")) + withr::local_options(list(shiny.otel.collect = "session")) with_mocked_bindings( - has_otel_bind = function(level) level == "session", + has_otel_collect = function(level) level == "session", otel_session_id_attrs = function(domain) list(session.id = domain$token), with_otel_span = function(name, expr, attributes = NULL) { span_created <<- TRUE @@ -252,7 +252,7 @@ test_that("integration test - session start with full request", { span_attributes <- NULL # Mock dependencies - withr::local_options(list(shiny.otel.bind = "session")) + withr::local_options(list(shiny.otel.collect = "session")) local_mocked_bindings( as_attributes = function(x) x, @@ -260,7 +260,7 @@ test_that("integration test - session start with full request", { ) with_mocked_bindings( - has_otel_bind = function(level) level == "session", + has_otel_collect = function(level) level == "session", otel_session_id_attrs = otel_session_id_attrs, # Use real function otel_session_attrs = otel_session_attrs, # Use real function with_otel_span = function(name, expr, attributes = NULL) {