Various fixes

.github/dependabot.yml

@@ -5,3 +5,7 @@ updates:
     directory: "/" # Location of package manifests
     schedule:
       interval: "weekly"
+  - package-ecosystem: "julia"
+    directory: "/"
+    schedule:
+      interval: "weekly"

.github/workflows/CI.yml

@@ -22,21 +22,21 @@ jobs:
       matrix:
         version:
           - "1.10"
+          - "1"
         os:
           - ubuntu-latest
         arch:
           - x64
     steps:
-      - uses: actions/checkout@v4
-      - uses: julia-actions/setup-julia@v2
+      - uses: actions/checkout@v7
+      - uses: julia-actions/setup-julia@v3
         with:
           version: ${{ matrix.version }}
           arch: ${{ matrix.arch }}
-      - uses: julia-actions/cache@v2
-      - uses: julia-actions/julia-buildpkg@v1
+      - uses: julia-actions/cache@v3
       - uses: julia-actions/julia-runtest@v1
       - uses: julia-actions/julia-processcoverage@v1
-      - uses: codecov/codecov-action@v5
+      - uses: codecov/codecov-action@v7
         with:
           token: ${{ secrets.CODECOV_TOKEN }}
           files: lcov.info

Project.toml

@@ -15,7 +15,7 @@ UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"
 [compat]
 CodecZlib = "0.7.8"
 Dates = "1.10"
-HTTP = "1.10"
+HTTP = "2"
 JSON = "1.1"
 Logging = "1.10"
 PkgVersion = "0.3.3"

src/Sentry.jl

@@ -27,8 +27,12 @@ export capture_message,
     Error,
     init
 
-function init(dsn=nothing ; traces_sample_rate=nothing, traces_sampler=nothing, debug=false, release=nothing)
-    main_hub.initialised && @warn "Sentry already initialised."
+function init(dsn=nothing ; traces_sample_rate=nothing, traces_sampler=nothing, debug=false, release=nothing, shutdown_timeout=DEFAULT_SHUTDOWN_TIMEOUT)
+    if main_hub.initialised
+        # Returning early, otherwise we would leak another send_worker task.
+        @warn "Sentry already initialised."
+        return nothing
+    end
     if dsn === nothing
         dsn = get(ENV, "SENTRY_DSN", nothing)
         if dsn === nothing
@@ -38,12 +42,8 @@ function init(dsn=nothing ; traces_sample_rate=nothing, traces_sampler=nothing,
         end
     end
 
-    if !main_hub.initialised
-        atexit(clear_queue)
-    end
-
-
     main_hub.debug = debug
+    main_hub.shutdown_timeout = shutdown_timeout
     main_hub.dsn = dsn
 
     upstream, project_id, public_key = parse_dsn(dsn)
@@ -62,7 +62,8 @@ function init(dsn=nothing ; traces_sample_rate=nothing, traces_sampler=nothing,
         main_hub.traces_sampler = NoSamples()
     end
 
-    main_hub.sender_task = @async send_worker()
+    main_hub.sender_task = Threads.@spawn send_worker()
+    atexit(clear_queue)
     bind(main_hub.queued_tasks, main_hub.sender_task)
     main_hub.initialised = true
 
@@ -158,10 +159,12 @@ function PrepareBody(event::Event, buf)
     println(buf, JSON.json(item_header))
     println(buf, item_str)
 
-    for attachment in event.attachments
+    for (i, attachment) in enumerate(event.attachments)
         attachment_str = JSON.json((;data=attachment))
+        # `filename` is required by sentry for attachment items.
         attachment_header = (; type="attachment",
                              length=sizeof(attachment_str),
+                             filename="attachment-$i.json",
                              content_type="application/json")
 
         println(buf, JSON.json(attachment_header))
@@ -214,6 +217,7 @@ function PrepareBody(transaction::Transaction, buf)
             # root_span...,
             root_span.start_timestamp,
             root_span.timestamp,
+            main_hub.release,
             tags = MergeTags(global_tags, root_span.tags),
 
             contexts = (; trace),
@@ -223,7 +227,7 @@ function PrepareBody(transaction::Transaction, buf)
 
     item_header = (; type="transaction",
                    content_type="application/json",
-                   length=sizeof(item_str)+1) # +1 for the newline to come
+                   length=sizeof(item_str))
 
 
     println(buf, JSON.json(envelope_header))
@@ -254,7 +258,7 @@ function send_envelope(task::TaskPayload)
     if main_hub.dsn === "fake"
         body = String(transcode(CodecZlib.GzipDecompressor, body))
         lines = map(eachline(IOBuffer(body))) do line
-            line = JSON.Parser.parse(line)
+            line = JSON.parse(line)
             line = JSON.json(line, 4)
         end
         @info "Would have sent this body"
@@ -263,18 +267,19 @@ function send_envelope(task::TaskPayload)
     end
     r = HTTP.request("POST", target, headers, body)
     if r.status == 200
-        return r.body
+        return Vector{UInt8}(r.body)
     else
-        throw(HTTP.Exceptions.StatusError(r.status, "POST", target, r))
+        throw(HTTP.Exceptions.StatusError(r.status, r))
     end
     return nothing
 end
 
 function send_worker()
-    while true
+    # Iterating the channel drains whatever is still buffered once it has been
+    # closed, and then finishes, which is what lets clear_queue wait for the
+    # sends themselves to complete rather than just for the queue to empty.
+    for event in main_hub.queued_tasks
         try
-            event = take!(main_hub.queued_tasks)
-            yield()
             send_envelope(event)
         catch exc
             if main_hub.debug
@@ -286,10 +291,9 @@ function send_worker()
 end
 
 function clear_queue()
-    while isready(main_hub.queued_tasks)
-        @info "Waiting for queue to finish before closing"
-        # send_envelope(take!(main_hub.queued_tasks))
-        sleep(1)
+    close(main_hub.queued_tasks)
+    if timedwait(() -> istaskdone(main_hub.sender_task), main_hub.shutdown_timeout) === :timed_out
+        @warn "Timed out sending queued events to sentry"
     end
 end
 
@@ -300,7 +304,19 @@ end
 function capture_event(task::TaskPayload)
     main_hub.initialised || return
 
-    push!(main_hub.queued_tasks, task)
+    try
+        push!(main_hub.queued_tasks, task)
+    catch exc
+        if !(exc isa InvalidStateException)
+            rethrow()
+        end
+
+        # The queue is closed while shutting down, so there is nothing left to
+        # send it to. Never let that propagate into the calling program.
+        if main_hub.debug
+            @error "Could not queue an event for sentry" exc
+        end
+    end
 end
 
 function capture_message(message, level::LogLevel=Info ; kwds...)
@@ -323,7 +339,7 @@ end
 
 # This assumes that we are calling from within a catch
 capture_exception(exc::Exception) = capture_exception([(exc, catch_backtrace())])
-function capture_exception(exceptions=catch_stack())
+function capture_exception(exceptions=Base.current_exceptions())
     main_hub.initialised || return
 
     formatted_excs = map(exceptions) do (exc,strace)

src/structs.jl

@@ -53,6 +53,10 @@ sample(sampler::RatioSampler) = rand() < sampler.ratio
 sample(sampler::Function) = sampler()
 
 const TaskPayload = Union{Event,Transaction}
+
+# Seconds to wait for queued events to be sent while the program is exiting
+const DEFAULT_SHUTDOWN_TIMEOUT = 10.0
+
 # This is to supposedly support the "unified api" of the sentry sdk. I'm not a
 # fan, so it will only go partway to this goal.
 # Note: a proper implementation here would make Hub a module.
@@ -68,6 +72,7 @@ Base.@kwdef mutable struct Hub
     release::Union{Nothing,String} = nothing
 
     debug::Bool = false
+    shutdown_timeout::Float64 = DEFAULT_SHUTDOWN_TIMEOUT
 
     last_send_time = nothing
     queued_tasks = Channel{TaskPayload}(100)

src/transactions.jl

@@ -3,7 +3,13 @@
 # * Transactions
 #----------------------------
 
-struct InhibitTransaction end
+# Mimics a transaction, so that it is cleared from the task local storage once
+# the outermost inhibited transaction finishes. Otherwise a single declined
+# sample would lock out tracing for the remaining life of the task.
+mutable struct InhibitTransaction
+    num_open_spans::Int
+end
+InhibitTransaction() = InhibitTransaction(0)
 
 function start_transaction(func ; kwds...)
     previous = get(task_local_storage(), :sentry_transaction, nothing)
@@ -20,7 +26,7 @@ function start_transaction(; name="", force_new=(name!=""), trace_id=:auto, pare
     # trace_id === nothing && return nothing
     # Need to pass through nothings so that we can hit an InhibitTransaction
     t = get_transaction(; name, trace_id, force_new)
-    if t === nothing || t === InhibitTransaction()
+    if t === nothing || t isa InhibitTransaction
         return t
     end
 
@@ -45,7 +51,13 @@ function finish_transaction(current, previous)
     task_local_storage(:sentry_transaction, previous)
 end
 finish_transaction(::Nothing) = nothing
-finish_transaction(::InhibitTransaction) = nothing
+function finish_transaction(inhibit::InhibitTransaction)
+    inhibit.num_open_spans -= 1
+    if inhibit.num_open_spans <= 0
+        task_local_storage(:sentry_transaction, nothing)
+    end
+    nothing
+end
 function finish_transaction((transaction, parent_span, span))
     complete(span)
     if transaction.root_span !== span
@@ -70,11 +82,12 @@ function get_transaction(; force_new=false, trace_id=:auto, kwds...)
         transaction = get(task_local_storage(), :sentry_transaction, nothing)
     end
 
-    if transaction === InhibitTransaction()
+    if transaction isa InhibitTransaction
+        transaction.num_open_spans += 1
         return transaction
     elseif transaction === nothing
         if trace_id === nothing
-            transaction = InhibitTransaction()
+            transaction = InhibitTransaction(1)
             task_local_storage(:sentry_transaction, transaction)
             return transaction
         elseif sample(main_hub.traces_sampler)
@@ -83,15 +96,10 @@ function get_transaction(; force_new=false, trace_id=:auto, kwds...)
             end
             transaction = Transaction(; trace_id = trace_id, kwds...)
         else
-            transaction = InhibitTransaction()
+            transaction = InhibitTransaction(1)
             task_local_storage(:sentry_transaction, transaction)
             return transaction
         end
-        # TODO: Note that the cases which store an InhibitTransaction in here
-        # are bad. They will lock out the start_transaction context manager from
-        # taking effect in any future cases. Instead, we should track this and
-        # later undo its effects once the outermost transaction is completed
-        # (meaning the InhibitTransaction itself should mimic a transaction).
         task_local_storage(:sentry_transaction, transaction)
         return (; transaction, parent_span=nothing)
     else
@@ -106,7 +114,9 @@ end
 
 set_task_transaction(::Nothing) = nothing
 function set_task_transaction(::InhibitTransaction)
-    task_local_storage(:sentry_transaction, InhibitTransaction())
+    # Starts at 1 with no matching finish, so that the parent's sampling
+    # decision holds for the whole life of this task.
+    task_local_storage(:sentry_transaction, InhibitTransaction(1))
 end
 function set_task_transaction((transaction, ignored, parent_span))
     task_local_storage(:sentry_transaction, transaction)