Skip to content

OpenTelemetry collector mode - #1535

Merged
unflxw merged 69 commits into
mainfrom
wip-otel-rework
Aug 21, 2026
Merged

OpenTelemetry collector mode#1535
unflxw merged 69 commits into
mainfrom
wip-otel-rework

Conversation

@unflxw

@unflxw unflxw commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Somewhat reviewable re-do of #1515.

While it's still a massive amount of changes, it separates the signal (implementing collector mode for metrics, traces and logs, as well as adding trace context propagation to our existing library and framework integrations) from the noise (making each and every integration's existing tests run both in agent mode and collector mode)

The sections below follow the branch in commit order. Where a run of commits was already reviewed in its own pull request, the section links to that pull request and lists the commits it covers, rather than repeating messages that have already been read. The remaining sections are commits that have had no pull request of their own.

Add OpenTelemetry collector mode

Setting collector_endpoint, or APPSIGNAL_COLLECTOR_ENDPOINT, puts
the integration in collector mode. Appsignal.start then boots an
OpenTelemetry SDK that exports OTLP over HTTP to that endpoint,
alongside the usual C extension agent.

The OpenTelemetry gems are not gemspec dependencies, because collector
mode is optional. The boot checks for them and falls back to the agent
when they are absent or too old.

Emit OpenTelemetry logs in collector mode

Appsignal::Logger goes through a backend. The extension backend
keeps the existing C extension logging, and an OpenTelemetry backend
emits log records through the SDK's logger provider in collector mode.

Ruby severities and formats are mapped to their OTLP equivalents.
appsignal.group and appsignal.format are hard overrides, so a user
attribute cannot spoof them.

Emit OpenTelemetry metrics in collector mode

The custom metric helpers go through a backend. The extension backend
keeps the C extension path, including its out-of-range warning, and an
OpenTelemetry backend records gauges, up/down counters and histograms
through the SDK's meter provider in collector mode.

With the metrics and logger backends both in place, Appsignal.start
can fall back to the agent when the OpenTelemetry SDK fails to boot, so
telemetry is never silently dropped.

Test the metric probes in collector mode

The probes emit through the metric helpers, so they need no changes of
their own. Running each metric example in both modes proves their
output routes to OpenTelemetry when collector mode is active.

Emit OpenTelemetry traces in collector mode

Appsignal::Transaction goes through a backend. The extension backend
keeps the existing C extension behaviour, and now owns the sample data
serialization, the breadcrumb buffering and the agent's error cause
shape. An OpenTelemetry backend maps a transaction to a root span and
each event to a child span.

One Transaction drives two error models, so the backend answers
records_errors_eagerly?. The extension collects errors and then
duplicates the transaction once per error at completion. The
OpenTelemetry backend records each error as an exception span event
straight away.

Three mappings decide the shape of the exported trace:

  • A span's kind comes from the namespace. A web or Action Cable
    transaction is SERVER, and a background job is CONSUMER. A datastore
    client event passes opentelemetry_kind: :client. The kind is set at
    creation, because it cannot change afterwards.
  • An event span is named title || name, and the event name is kept as
    the appsignal.category attribute. A SQL body maps to
    db.query.text, and every other body to appsignal.body.
  • An error attaches to the currently open span, with its cause chain in
    one appsignal.error_causes JSON attribute. A discarded transaction
    sets appsignal.ignore_subtrace, which tells the collector to drop
    it.

Test integrations in collector mode

Every existing event-emission example in the hook, integration, Rack
and loader specs now runs in both agent and collector mode. The
collector-mode half asserts the span shape: its name, category, body,
kind and parent. No library code changes, so this exercises the trace
backend through every integration.

Inject trace context into outgoing HTTP requests

In collector mode the HTTP client integrations write the current W3C
trace context onto their outgoing requests, so the service being called
continues the same trace. Net::HTTP, HTTP.rb, Excon and Faraday each
inject a traceparent header and mark the request span with CLIENT
kind. HTTP.rb injects on every hop, and Excon injects through a
middleware.

Injection is a no-op outside collector mode.

Extract trace context from incoming web requests

In collector mode the Rack middleware, the Rack event handler and the
Webmachine integration read an incoming W3C traceparent header and
pass it to the transaction. A web transaction continues under the
remote span. A job transaction starts its own trace and links back to
it. With no context, or outside collector mode, the root span stands
alone.

Propagate trace context across background jobs

In collector mode each job library carries the W3C trace context across
the enqueue and perform boundary. On enqueue the client writes the
current context onto the job payload and marks the enqueue span with
PRODUCER kind. On perform the server side reads it back and links the
job's trace to the enqueuing span.

Each library carries the context on its own channel, such as a job hash
key, an SQS message attribute, a Que tag, or Active Job's serialized
arguments. When a channel is full the propagation is skipped rather
than the enqueue broken.

Run error blocks eagerly in collector mode

In collector mode an error is recorded when it is added, but its block
was held until completion. Deferral only makes sense in agent mode,
where blocks run against the duplicate transactions fabricated at
completion. The block now runs when the error is added, so side effects
that target the current span, such as breadcrumbs or nested errors,
land where the error was reported rather than on the root span.

@error_blocks was carrying two jobs: the set of distinct errors, which
drives last_errors, the dedup check and the error limit, and the
per-error blocks that only agent mode runs. @errors is now the
distinct-error set used in both modes, and @error_blocks holds only
blocks and is populated only in agent mode.

records_errors_eagerly? is renamed to supports_multiple_errors?.
The question at the call site is whether the backend can hold more than
one error on a transaction, not how it records them.

Mark Sequel query spans as CLIENT kind in collector mode (#1546)

Extract trace context onto an empty context (#1542)

Guard blocks handed to the transaction (#1544)

Drop actionless transactions in collector mode (#1541)

Record the Delayed Job enqueue as a producer span (#1538)

Dual-mode transaction and helper spec coverage (#1547)

Tag integration spans with instrumentation scope (#1551)

Add the appsignal-opentelemetry companion gem (#1554)

Report allocation counts in collector mode (#1550)

Set transaction OpenTelemetry kind and relationship explicitly (#1552)

Document the OpenTelemetry keyword arguments

opentelemetry_kind, opentelemetry_relationship and
opentelemetry_context carried documentation, but opentelemetry_scope
did not, and neither did opentelemetry_kind on Appsignal.instrument.
The type signatures are generated from these comments, so the
undocumented arguments were generated as untyped. They are documented
everywhere they are accepted now, so opentelemetry_scope generates as
a [String, String] pair. The signatures are regenerated to match.

Put event category in collector span name (#1553)

Link bulk enqueued Que jobs instead of parenting

Every job enqueued by Que.bulk_enqueue shares one producer span,
because the whole batch is recorded as a single bulk_enqueue.que
event. A span can have only one parent, so parenting hangs the entire
batch off that one span. The OpenTelemetry messaging conventions ask
for links by default, and allow the producer to be the parent only when
it produced a single message.

Que records nothing that tells its two enqueue paths apart. Both insert
paths write the same columns, and a job's data column holds only its
tags. The enqueue side marks a batch with a tag, and the worker links
instead of parenting when it finds that tag. The tag contains no colon,
so a reader that splits tags on the colon to rebuild the trace context
carrier ignores it.

Que allows five tags per job and the trace context already takes one.
On the batch path the marker and the trace context are kept or dropped
together, because propagating without the marker would make the worker
parent the whole batch. When there is no room for both, the batch is
enqueued without trace context.

Trim shared frames from error cause backtraces (#1559)

Skip trace context when enqueue events are off (#1561)

Emit OpenTelemetry attributes for integrations (#1562)

Fix the negated include_event matcher (#1564)

Add OpenTelemetry support for Excon integration (#1566)

Drop the duplicate queue start attribute

The queue start event on the transaction's root span carried the queue
start time twice, as the event's timestamp and as an attribute of the
same name. A span event carries its own time, so the attribute said
nothing the event did not. Only the timestamp is set now, which is what
the breadcrumb event a little further up the file already does.

The specs asserted the attribute and nothing about the timestamp, even
where an example's own name said it was about the time the queue
started. They assert the timestamp now.

Report a signalled runner script as a failure

The integration runner checked a finished script's exit code to decide
whether it had failed. A process killed by a signal has no exit code, so
reading one raised a NoMethodError on nil.

That hid both what happened to the script and the output the check
prints to explain it. Ask the status whether it succeeded instead,
which answers for a signalled process too, and name the status in the
failure.

Express per-event treatment through event formatters (#1568)

Trim the unreleased changesets

The changesets explained how each change works and why the old
behaviour was wrong. A changelog entry is read by someone deciding
whether it is about them, so it needs the symptom, the new behaviour
and anything they have to act on, not the mechanism. The collector mode
entry also said that no AppSignal data flows through the OpenTelemetry
SDK yet, which will not be true when this ships.

Match monitor's arguments in monitor_and_stop (#1573)

Default instrument_sql to a client span (#1577)

Suppress native events even when hooks are off (#1579)

Name the real SQL engine in db.system.name (#1580)

@backlog-helper

backlog-helper Bot commented Jul 8, 2026

Copy link
Copy Markdown

✔️ All good!

New issue guide | Backlog management | Rules | Feedback

@unflxw unflxw changed the title OpenTelemetry collector mode (narrative reconstruction of wip-opentelemetry) OpenTelemetry collector mode Jul 8, 2026
@unflxw
unflxw force-pushed the wip-otel-rework branch from ad053a7 to f62db5f Compare July 8, 2026 16:32

@unflxw unflxw left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've attempted to clarify and make explicit some decisions contained within this in the comments. Other decisions to point out, many of which were also present in #1515:

  • The agent still runs. This provides host metrics, and preserves compatibility with out StatsD and NGINX metrics support. I believe this is a good idea and would like to backport it to Python's collector mode.
  • The OpenTelemetry gems must be installed manually in order to keep compatibility with Ruby 2.7, for which no compatible version of the OpenTelemetry gems exists. This is a significant hurdle for the installation process. We can soften it with a meta-gem that installs those dependencies, so that at least it's still a one-liner. We can also bump the gem's Ruby version requirement to Ruby 3.1, which regardless of the gem's installation requirement, is a requirement for collector mode as currently implemented.
  • The oddities and differences in how OpenTelemetry trace context propagation is done in the background job libraries mirror the oddities of the corresponding OpenTelemetry contrib instrumentations, or their closest equivalent. This attempts to provide wire compatibility between the two, such that you could enqueue a background job in a collector mode AppSignal-instrumented application, process it in an OpenTelemetry-instrumented application pointed at the same collector, and have the traces link correctly.

Comment on lines +47 to +48
otel_attributes["appsignal.group"] = group.to_s
otel_attributes["appsignal.format"] = FORMAT_NAMES.fetch(format, "autodetect")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread lib/appsignal/helpers/instrumentation.rb
Comment thread lib/appsignal/transaction/opentelemetry_backend.rb Outdated
Comment thread lib/appsignal/transaction/opentelemetry_backend.rb Outdated
Comment thread lib/appsignal/transaction/opentelemetry_backend.rb Outdated
Comment thread lib/appsignal/transaction/opentelemetry_backend.rb
Comment on lines +330 to +335
# The open event span, or the root span when no event is open. Not the OTel
# current span, which may belong to another instrumentation.
def current_span
span, _token = @event_stack.last
span || @span
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The "golden rule" of the AppSignal-to-OpenTelemetry interoperability story is that we don't write AppSignal-flavoured attributes and events to whatever is the current span in the active OpenTelemetry context, preventing AppSignal from "polluting" spans from external OpenTelemetry instrumentations. Instead, we keep a reference to the last AppSignal span created by the transaction: either the transaction's root span, or the span for the currently active event.

This way, whenever we add AppSignal-flavoured events or attributes, we always add them to one of our spans, either by using this helper or by referencing @span (the transaction's root span) directly -- we never add them to the current span in the active OpenTelemetry context.

Comment on lines +81 to +82
def start_event(opentelemetry_kind: nil)
span = tracer.start_span(EVENT_SPAN_PLACEHOLDER_NAME, :kind => opentelemetry_kind)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note that we don't pass an explicit current_span parent when creating the span for an event -- we allow the span an AppSignal event to be a child of whichever the current active span in the OpenTelemetry context is. This allows AppSignal events and OpenTelemetry spans to nest within each other.

Comment on lines +410 to +416
def otel_header_name(env_key)
if env_key.start_with?("HTTP_")
env_key.delete_prefix("HTTP_").downcase.tr("_", "-")
elsif env_key.start_with?("CONTENT_")
env_key.downcase.tr("_", "-")
end
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a divergence between agent mode and collector mode, in that the "Environment" section in agent mode contains other Rack CGI-flavoured attributes in it as well as HTTP headers, but in collector mode, only actual HTTP headers will be emitted as http.request.header.* attributes, and all other CGI-flavoured attributes in it from Rack will be dropped.

Comment thread spec/lib/appsignal/integrations/excon_spec.rb
@unflxw
unflxw requested review from lipskis and tombruijn July 8, 2026 17:42
@unflxw
unflxw marked this pull request as ready for review July 8, 2026 17:42
@tombruijn tombruijn added the feature A new feature for this component. label Jul 9, 2026
Comment thread lib/appsignal/transaction.rb
@backlog-helper

This comment has been minimized.

@unflxw
unflxw force-pushed the wip-otel-rework branch 3 times, most recently from 31d2771 to c220cce Compare July 13, 2026 12:41
@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

1 similar comment
@backlog-helper

This comment has been minimized.

@unflxw
unflxw force-pushed the wip-otel-rework branch 2 times, most recently from e03b091 to 1375eeb Compare July 16, 2026 15:18
@backlog-helper

This comment has been minimized.

1 similar comment
@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

unflxw and others added 28 commits August 21, 2026 15:50
The trace timeline decides what kind of work a span represents from its
OpenTelemetry attributes, and a span with none of them is shown under
"Other". The HTTP client spans had none, so an outgoing request looked
no different from an uninstrumented block of code.

They now carry what the semantic conventions ask for: the request
method, the host and port being called, the full URL, and the response
status code when a response came back. The method is normalised to the
closed set of names the conventions define, keeping the original
alongside.

The URL is built from its parts rather than taken whole, so it can
never carry credentials, and it leaves out the query string. The query
string of a call to somebody else's API is the part most likely to hold
a key, and the application did not choose its shape.
A web transaction's span already carried the SERVER span kind, but the
trace timeline needs an HTTP attribute alongside it before it reads the
span as a web request. Without one the request was shown under "Other".

It now carries what the semantic conventions ask for: the request
method, the path, the scheme, the query string when there is one, and
the response status code when a response was sent. The path is the
concrete path rather than the route template the conventions call
`http.route`, which a Rack application does not necessarily have. The
query string is not filtered here, because the collector already
filters this attribute.

These attributes belong on the transaction's own span, so they are set
where the transaction is created rather than after the app has been
called, when an event span may be open and would take them instead.
The status is only known once the app has responded, so it is set at
the next point where the transaction's span is the one attributes go
on. A request that raised sent no response, so it gets no status.
The trace timeline recognises a datastore call by its `db.system.name`
attribute, and splits caches out from databases by the value. Only SQL
query spans carried it, so a Redis, MongoDB or Elasticsearch query was
shown under "Other".

They now carry the values the conventions define and OpenTelemetry's
own instrumentation for these libraries uses: `redis`, `mongodb` and
`elasticsearch`. Each datastore's own page raises some of the rest to a
requirement, so a Redis span also carries the command and the database,
a MongoDB span the command and the collection, and an Elasticsearch
span the operation and the index.

An Elasticsearch search is an outgoing call to a cluster, so it carries
CLIENT kind rather than the default. Its index has to be read out of
the search itself, so an attribute can now be derived from an event's
payload rather than only mapped from its name. A SQL span is named as a
database span even when the event has no query to record, where before
it was named only when it had one.
The trace timeline recognises background job work by the
`messaging.system` attribute, and uses the span kind to tell an enqueue
apart from the job it enqueued. The job spans carried the kind but not
the attribute, so both were shown under "Other".

Each library takes its own name, which is what OpenTelemetry's
instrumentation for it uses. Shoryuken is the exception, because it
runs on Amazon SQS and so takes `aws_sqs`. The conventions also require
the operation's name and kind, and ask for the destination, which for a
job library is the queue. Those three travel together, so one place
builds them and every integration passes its own messaging system.

Not every job has a queue to name. Que records the queue only on the
job itself, and Delayed Job has backends without queues, so a span with
none is described without one. Shoryuken is the only integration that
can report a batch size, because its server middleware is handed the
whole array. Active Job describes itself only when no adapter
integration already has, because the adapter's answer is more specific.
Template rendering was the last kind of work the trace timeline could
not place, so an Action View or ViewComponent render was shown under
"Other". OpenTelemetry has no semantic convention for template
rendering, and its own Action View instrumentation sets no convention
attribute either, so there is nothing to follow. The timeline does read
an `appsignal.group` attribute before it looks at any convention, so
these spans say which group they belong to directly.
The semantic conventions ask for an `error.type` attribute on a span
whose operation ended with an error, and for it to be left unset
otherwise. The value has to have low cardinality, so it is the
exception's class name rather than its message. This is an attribute of
the span itself. The `exception.type` the gem already reports is an
attribute of the exception event, which is a separate thing.

An error that only passes through an instrumented event is never set on
the transaction, and the event's span has closed by the time the
transaction would record it. It is therefore read where the event is
recorded. When one span collects more than one error the last one wins,
because the attribute holds a single value.
The `include_event` matcher fills in a default for every key the caller
leaves out. In the negated form that made the expectation pass almost
regardless, because an event ruled out by name alone was only matched
if its title was empty too, and nearly every event has a title. A spec
saying an event was not recorded was saying nothing, and would have
gone on passing if that event appeared.

The negated form matches on the given keys alone. The Faraday spec was
the one leaning on this: its claim that Net::HTTP is not recorded a
second time under Faraday now fails if the suppression is taken away.

Iterating a Rack response body's Enumerator turns out to record an
event that a spec said it did not. It reads the body through the same
instrumented `each`, so it is recorded exactly as passing a block is,
and both that spec and its collector-mode counterpart now say so.
Faraday runs on a downstream HTTP client that AppSignal may instrument
too, and it suppresses that client so the request is recorded once.
Excon is one of the two clients this applies to, and it suppresses
through a different integration than Net::HTTP does, so the pairing
needs its own test. Nothing covered the two of them together after
Excon moved to its own gemfile.

Excon is back in the Faraday gemfile and the pairing is tested in both
modes. Removing Faraday's suppression now fails four examples rather
than two.
In collector mode a span says what kind of operation it covers, and an
outgoing HTTP request is a client span. It also says which
instrumentation recorded it, so an Excon request is attributed to
`appsignal-ruby/excon` rather than to the gem as a whole. The rebuilt
Excon integration carries both again.
In collector mode an outgoing HTTP request carries the current trace
context, so the service being called continues the same trace. Excon
exposes the request's headers to a middleware, so the context is
written there.

What the context is taken from has changed. It used to reflect the
event span of one of Excon's instrumentor notifications, and now
reflects the one span open around the whole request. A retried request
shows the difference, because every attempt shares that span and so
carries the same context.
In collector mode a span carries the attributes the semantic
conventions ask for, and for an outgoing HTTP request those are the
request method, the host and port being called, the URL, and the
response status.

Excon could not describe a request this way before. Everything except
the status came from one of Excon's instrumentor notifications and the
status came from another, so the two never met on the same span, and
the span carrying the status carried nothing else. A request is one
span now, so all of it lands together. A failed request is described
too, with the kind of error that ended it.

A pipelined request is the one case where Excon returns before its
response is read, so it has no status to describe. It hands back the
request data rather than a response.
The queue start event on the transaction's root span carried the queue
start time twice, as the event's timestamp and as an attribute of the
same name. A span event carries its own time, so the attribute said
nothing the event did not. Only the timestamp is set now, which is what
the breadcrumb event a little further up the file already does.

The specs asserted the attribute and nothing about the timestamp, even
where an example's own name said it was about the time the queue
started. They assert the timestamp now.
The integration runner checked a finished script's exit code to decide
whether it had failed. A process killed by a signal has no exit code, so
reading one raised a NoMethodError on nil.

That hid both what happened to the script and the output the check
prints to explain it. Ask the status whether it succeeded instead,
which answers for a signalled process too, and name the status in the
failure.
The ViewComponent formatter was registered for two event names,
`render.view_component` and `!render.view_component`. The second could
never be reached, because an event name starting with an exclamation
mark is internal to Rails and this gem skips those before the registry
is consulted. Nothing else can reach it either: only two places look a
formatter up by name, one skips the exclamation mark names and the
other builds its name from a dry-monitor event, which cannot start with
one.

Its only test asserted that the registration existed, so it proved
nothing beyond itself. Both are gone.
An event formatter says what an event's title and body should be, so
implementing `format` is mandatory and a class without it is refused at
registration. A formatter can now say other things about an event, such
as what kind of work it is, and some events have something to say that
way but no title or body to give.

The base class implements `format` and returns nothing, which a
formatter with no title or body can inherit. A formatter that does not
inherit from the base class still has to implement it, which is the
case the check was written for.
Two places record an event that another library reported, and both
looked up what kind of work the event was by name. The
ActiveSupport::Notifications integration kept a list of three event
names that are outgoing datastore calls, and the dry-monitor
integration checked whether the event was called `sql`. The two said
the same thing about the same kind of work in two files, because ROM
reports its queries through dry-monitor while Rails reports its own
through ActiveSupport::Notifications, and both lists had to be edited
whenever a formatter was added.

The event formatter already exists to give a named event its own
treatment, so it declares the kind. Both integrations ask the registry,
and both lists are gone.
The ActiveSupport::Notifications integration held a table of
OpenTelemetry attributes by event name, and a second lookup for the
attributes it has to read out of an event's payload. Both described
events belonging to Rails components the integration otherwise knows
nothing about. Each formatter describes its own event now.

The render events only have a title inside a Rails application, because
the title is the template's path made relative to the application's
root. Whether there is an application to take a root from is answered
by the formatter as it formats an event, rather than by the code that
registers it. Answering it at registration time was wrong, because
AppSignal can be required before Rails is, and the answer was then
wrong for as long as the process ran.
Two events are recorded by a dedicated AppSignal integration that
describes them better than the generic paths can. The
ActiveSupport::Notifications integration held their names in a list, so
it had to know about two integrations it has nothing else to do with.
They are claimed through the registry now, the same way every other
treatment of a named event is, and both generic paths ask it.

Each event is claimed by the integration that records it, as that
integration installs. The claim exists to stop the same work being
recorded twice, so it lasts exactly as long as the integration that
would otherwise record it twice. An application that turns Active Job
or Faraday instrumentation off now gets the notification that library
reports itself, where before it got no event at all.
The instrumentation scope names the library an event was recorded for.
Both paths that record an event another library reported derived it
from where the event arrived, which is right until the two differ. They
differ for ROM, which reports its queries over dry-monitor. That is a
notification bus rather than something that queries a database, so
every ROM query was attributed to dry-monitor, which says nothing about
what the query was.

A formatter can name the library, and both paths use that when there is
one. Anything that declares nothing keeps the scope its path derives.
The changesets explained how each change works and why the old
behaviour was wrong. A changelog entry is read by someone deciding
whether it is about them, so it needs the symptom, the new behaviour
and anything they have to act on, not the mechanism. The collector mode
entry also said that no AppSignal data flows through the OpenTelemetry
SDK yet, which will not be true when this ships.
`monitor_and_stop` accepted `opentelemetry_scope` but not
`opentelemetry_context`, `opentelemetry_kind` or
`opentelemetry_relationship`. It creates its transaction by calling
`monitor`, which accepts all four, so those three were out of reach for
anyone using this helper. Being deprecated is no reason to offer less
than the method it delegates to, so it accepts all four and passes them
on.
`Appsignal.instrument_sql` accepted `opentelemetry_scope` but not
`opentelemetry_kind`. It delegates to `Appsignal.instrument`, which
accepts both, so the span kind was out of reach for anyone using this
helper and its spans got OpenTelemetry's default kind of `:internal`.
Every other path in the gem that records a SQL span already produces a
client span, because a query is an outgoing call to a datastore, so the
public helper was the only SQL path that did not. It accepts
`opentelemetry_kind` and defaults it to `:client`.

The default lives in the signature rather than being substituted when
the argument is forwarded. A Ruby keyword default applies only when the
argument is omitted, so an explicit `nil` still forwards `nil` and
lands on the OpenTelemetry default. That keeps `nil` meaning
"unspecified", which is what it means everywhere else in this codebase.
Active Job and Faraday each record their own event for the enqueue or
the request. AppSignal has its own instrumentation for that same work,
so it claims Active Job's `enqueue.active_job` event and Faraday's
`request.faraday` event, which tells the generic notification paths to
leave them alone.

That claim happened inside each hook's `install` method, which only
runs when the matching config option is on. Turning
`instrument_active_job` or `instrument_faraday` off therefore turned
the claim off too, so AppSignal's own instrumentation stopped running
and the library's own event was reported in its place. A customer who
disables one of these integrations does not expect an event for that
work at all.

Both hook files are already required unconditionally, so the claim sits
in the class body and runs as soon as the file loads. It needs no
`defined?` guard, because claiming an event only touches AppSignal's
own formatter registry and never the `ActiveJob` or `Faraday`
constants, so it cannot fail when the library is absent.
`write_event_body_attributes` wrote the `other_sql` sentinel on every
SQL event unconditionally. Both `finish_event` and `record_event` call
it after an integration's own attributes are already on the span, so
the sentinel always overwrote whatever engine name the integration had
set. Nothing in the gem sets `db.system.name` yet, so this had no
visible effect, but it would silently undo any integration that starts
naming a real engine.

The event frame tracks whether `db.system.name` was set during the
event, and the sentinel is written only when that never happened.
`record_event` has no event frame to track this on, so it always falls
back to the sentinel.

Presence of the key is not enough to count as set.
`add_opentelemetry_attributes` is public API, so a caller can pass an
explicit nil, and `Attributes.format` coerces that to an empty string,
which would leave the key in place. A shared `named_db_system?` check
looks at the formatted value instead, so a blank value still falls back
to the sentinel rather than leaving the span with nothing the
collector's sanitizer recognises.
instrument called Appsignal::EventFormatter.format but never
Appsignal::EventFormatter.opentelemetry_attributes, unlike
ActiveSupportNotificationsIntegration#finish_event, which calls both.
A formatter that only arrives over dry-monitor, such as ROM's SQL
formatter, could define opentelemetry_attributes and it would never
run.

The call is added before finish_event, so the attributes land on the
event's own span rather than on the transaction.
`record_event` accepted `opentelemetry_kind` and `opentelemetry_scope`
but had no way to attach attributes. An `instrument` caller can add
attributes in the window between `start_event` and `finish_event`, but
`record_event` reports a complete event in one call, so a caller such
as DataMapper's integration had no way to describe what it recorded.

The keyword threads through `Transaction#record_event` and
`OpenTelemetryBackend#record_event`, which formats and writes the
attributes to the span before it finishes. `ExtensionBackend` ignores
it, as it does the other OpenTelemetry keywords, because agent mode has
no span to put them on. A caller that names its own `db.system.name`
this way takes priority over the SQL sentinel, through the same
`named_db_system?` check that guards `set_attributes`.
SQL spans carried a sentinel value for `db.system.name` that means the
dialect is unknown, while the Redis, MongoDB and Elasticsearch spans
already named theirs. Each library spells its engine differently, and
none of them use the value the semantic conventions ask for, so the
mapping is kept per library rather than shared. ROM is the exception.
Its dry-monitor payload carries Sequel's own `database_type` symbol, so
it reuses Sequel's mapping.

The values follow the older semantic conventions set, so SQL Server is
`mssql` and Oracle is `oracle` rather than `microsoft.sql_server` and
`oracle.db`. Those are the values the AppSignal collector's sanitizer
recognises today. A name that no mapping recognises returns nil, so the
existing `other_sql` fallback still applies.
ActiveRecord, Sequel and ROM name a real `db.system.name` instead of
the sentinel that means the dialect is unknown. DataMapper's
integration already checks its connection class against `SQL_CLASSES`
to decide whether a query is SQL at all, so the same class names the
engine through the shared `SqlDbSystem` mapping.

This needs `record_event`'s `opentelemetry_attributes` keyword, because
DataMapper reports a complete event in one call and has no window
between start and finish in which to add an attribute.
@unflxw
unflxw merged commit 546bde2 into main Aug 21, 2026
432 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature A new feature for this component.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants