Tag integration spans with instrumentation scope - #1551
Conversation
78e1c86 to
dd78094
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the AppSignal Ruby gem’s OpenTelemetry collector-mode pipeline so spans emitted by each integration are recorded under an integration-specific OpenTelemetry instrumentation scope (name + gem version). This enables grouping/aggregation per integration scope in downstream OpenTelemetry UIs while keeping agent-mode behavior unchanged.
Changes:
- Add optional
opentelemetry_scope: [name, version]plumbing from instrumentation helpers /Transaction.createthrough transactions into both collector (OTel) and agent backends. - Resolve collector-mode tracers per-scope and apply the scope to root spans and event spans (including AS::Notifications-derived scopes and Rack/framework middleware scopes).
- Remove the AppSignal-side OTel logger cache and update specs to assert provider swaps are picked up without a reset hook.
Reviewed changes
Copilot reviewed 75 out of 76 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/support/shared_contexts/collector_mode.rb | Drops logger-backend reset and adds scope_of(span) helper for collector-mode assertions. |
| spec/lib/appsignal/transaction/opentelemetry_backend_spec.rb | Adds scope-focused backend specs (root span + event spans + fallback behavior). |
| spec/lib/appsignal/transaction_spec.rb | Verifies Transaction forwards opentelemetry_scope to backend event methods. |
| spec/lib/appsignal/rack/sinatra_instrumentation_spec.rb | Asserts Sinatra request/event spans are recorded under the Sinatra scope. |
| spec/lib/appsignal/rack/rails_instrumentation_spec.rb | Adds collector-mode assertion for Rails scope when middleware creates the transaction. |
| spec/lib/appsignal/rack/instrumentation_middleware_spec.rb | Asserts generic Rack middleware spans use the Rack scope. |
| spec/lib/appsignal/rack/hanami_middleware_spec.rb | Asserts Hanami request spans use the Hanami scope. |
| spec/lib/appsignal/rack/grape_middleware_spec.rb | Asserts Grape request spans use the Grape scope. |
| spec/lib/appsignal/rack/event_handler_spec.rb | Asserts Rack event handler root spans use the Rack scope. |
| spec/lib/appsignal/logger/opentelemetry_backend_spec.rb | Updates specs to reflect per-emit logger resolution (no local cache/reset). |
| spec/lib/appsignal/loaders/padrino_spec.rb | Updates middleware registration expectation to include Padrino scope option. |
| spec/lib/appsignal/integrations/webmachine_spec.rb | Asserts Webmachine request spans use the Webmachine scope. |
| spec/lib/appsignal/integrations/sidekiq_spec.rb | Asserts Sidekiq spans (perform + enqueue producer) use the Sidekiq scope. |
| spec/lib/appsignal/integrations/shoryuken_spec.rb | Asserts Shoryuken spans (perform + enqueue producer) use the Shoryuken scope. |
| spec/lib/appsignal/integrations/resque_spec.rb | Asserts Resque spans (perform + enqueue producer) use the Resque scope. |
| spec/lib/appsignal/integrations/railtie_spec.rb | Asserts Rails error reporter spans use the Rails scope. |
| spec/lib/appsignal/integrations/que_spec.rb | Asserts Que spans (perform + enqueue producer) use the Que scope. |
| spec/lib/appsignal/integrations/puma_spec.rb | Asserts Puma error reporting applies scope only when it creates the transaction. |
| spec/lib/appsignal/integrations/net_http_spec.rb | Asserts Net::HTTP client spans use the Net::HTTP scope. |
| spec/lib/appsignal/integrations/mongo_ruby_driver_spec.rb | Asserts Mongo driver client spans use the Mongo scope. |
| spec/lib/appsignal/integrations/http_spec.rb | Asserts http.rb client spans use the http.rb scope. |
| spec/lib/appsignal/integrations/faraday_spec.rb | Asserts Faraday client spans use the Faraday scope. |
| spec/lib/appsignal/integrations/excon_spec.rb | Asserts Excon client spans use the Excon scope. |
| spec/lib/appsignal/integrations/delayed_job_plugin_spec.rb | Asserts DelayedJob enqueue/perform spans use the DelayedJob scope. |
| spec/lib/appsignal/integrations/data_mapper_spec.rb | Asserts DataMapper spans use the DataMapper scope. |
| spec/lib/appsignal/hooks/sequel_spec.rb | Asserts Sequel hook spans use the Sequel scope. |
| spec/lib/appsignal/hooks/redis_spec.rb | Asserts Redis hook spans use the Redis scope. |
| spec/lib/appsignal/hooks/redis_client_spec.rb | Asserts redis-client hook spans use the redis-client scope. |
| spec/lib/appsignal/hooks/rake_spec.rb | Asserts Rake task spans use the Rake scope. |
| spec/lib/appsignal/hooks/dry_monitor_spec.rb | Asserts dry-monitor spans use the dry-monitor scope. |
| spec/lib/appsignal/hooks/at_exit_spec.rb | Asserts at-exit error reporting spans use the at-exit scope. |
| spec/lib/appsignal/hooks/activejob_spec.rb | Asserts ActiveJob spans (perform + enqueue producer) use the ActiveJob scope. |
| spec/lib/appsignal/hooks/active_support_notifications/instrument_shared_examples.rb | Adds assertions for group-derived AS::Notifications instrumentation scopes and fallback. |
| spec/lib/appsignal/hooks/action_cable_spec.rb | Asserts ActionCable spans use the ActionCable scope. |
| spec/lib/appsignal_spec.rb | Adds collector-mode assertions for monitor/send_error/report_error scope passing. |
| sig/appsignal.rbs | Updates public type signatures to include opentelemetry_scope where applicable. |
| sig/appsignal.rbi | Updates Sorbet signatures to include opentelemetry_scope where applicable. |
| lib/appsignal/transaction/opentelemetry_backend.rb | Adds per-scope tracer resolution and threads scope through root/event span creation. |
| lib/appsignal/transaction/extension_backend.rb | Accepts opentelemetry_scope (ignored in agent mode) for API compatibility. |
| lib/appsignal/transaction/base_backend.rb | Extends backend interface to accept opentelemetry_scope for event methods. |
| lib/appsignal/transaction.rb | Threads opentelemetry_scope through create, initialization, and event APIs. |
| lib/appsignal/rack/sinatra_instrumentation.rb | Sets default Sinatra scope in middleware options. |
| lib/appsignal/rack/rails_instrumentation.rb | Sets Rails scope in middleware options. |
| lib/appsignal/rack/instrumentation_middleware.rb | Sets default Rack scope in middleware options. |
| lib/appsignal/rack/hanami_middleware.rb | Sets default Hanami scope in middleware options. |
| lib/appsignal/rack/grape_middleware.rb | Sets Grape scope in middleware options. |
| lib/appsignal/rack/event_handler.rb | Creates Rack transactions and wrapping event spans under the Rack scope. |
| lib/appsignal/rack/body_wrapper.rb | Tags response-body instrumentation spans with the Rack scope. |
| lib/appsignal/rack/abstract_middleware.rb | Adds opentelemetry_scope option handling and passes it to transactions/events. |
| lib/appsignal/logger/opentelemetry_backend.rb | Removes logger caching/reset; resolves logger from provider per emit. |
| lib/appsignal/loaders/padrino.rb | Passes Padrino scope option to Sinatra base instrumentation middleware. |
| lib/appsignal/integrations/webmachine.rb | Tags Webmachine transaction/event spans with Webmachine scope. |
| lib/appsignal/integrations/sidekiq.rb | Tags Sidekiq transaction/event spans with Sidekiq scope (including enqueue producer spans). |
| lib/appsignal/integrations/shoryuken.rb | Tags Shoryuken transaction/event spans with Shoryuken scope (including enqueue producer spans). |
| lib/appsignal/integrations/resque.rb | Tags Resque transaction/event spans with Resque scope (including enqueue producer spans). |
| lib/appsignal/integrations/redis.rb | Tags Redis spans with Redis scope. |
| lib/appsignal/integrations/redis_client.rb | Tags redis-client spans with redis-client scope. |
| lib/appsignal/integrations/rake.rb | Tags Rake task transaction/event spans with Rake scope. |
| lib/appsignal/integrations/railtie.rb | Tags Rails error reporter transactions with Rails scope. |
| lib/appsignal/integrations/que.rb | Tags Que transaction/event spans with Que scope (including enqueue producer spans). |
| lib/appsignal/integrations/puma.rb | Tags Puma low-level error transactions with Puma scope (when it creates them). |
| lib/appsignal/integrations/net_http.rb | Tags Net::HTTP client spans with Net::HTTP scope. |
| lib/appsignal/integrations/mongo_ruby_driver.rb | Tags Mongo driver event spans with Mongo scope. |
| lib/appsignal/integrations/http.rb | Tags http.rb client spans with http.rb scope. |
| lib/appsignal/integrations/faraday.rb | Tags Faraday client spans with Faraday scope. |
| lib/appsignal/integrations/excon.rb | Tags Excon client spans with Excon scope. |
| lib/appsignal/integrations/dry_monitor.rb | Tags dry-monitor spans with dry-monitor scope. |
| lib/appsignal/integrations/delayed_job_plugin.rb | Tags DelayedJob transaction/event spans with DelayedJob scope (including enqueue producer spans). |
| lib/appsignal/integrations/data_mapper.rb | Tags DataMapper spans with DataMapper scope. |
| lib/appsignal/integrations/active_support_notifications.rb | Derives instrumentation scope from AS::Notifications event name group. |
| lib/appsignal/integrations/action_cable.rb | Tags ActionCable transactions with ActionCable scope. |
| lib/appsignal/hooks/sequel.rb | Tags Sequel hook spans with Sequel scope. |
| lib/appsignal/hooks/at_exit.rb | Tags at-exit error reporting transactions with at-exit scope. |
| lib/appsignal/hooks/active_job.rb | Tags ActiveJob transaction/event spans with ActiveJob scope (including enqueue producer spans). |
| lib/appsignal/hooks/action_cable.rb | Tags ActionCable subscribe/unsubscribe transactions/events with ActionCable scope. |
| lib/appsignal/helpers/instrumentation.rb | Adds opentelemetry_scope to helpers and forwards it into created transactions/events. |
Files not reviewed (1)
- sig/appsignal.rbi: File type not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
|
This is a message from the daily scheduled checks. |
Introduce an optional "collector mode": when `collector_endpoint` (or `APPSIGNAL_COLLECTOR_ENDPOINT`) is set, `Appsignal.start` boots an OpenTelemetry SDK that exports OTLP/HTTP to that endpoint alongside the usual C-extension agent. No AppSignal-collected data routes through the SDK yet — this commit stands up the mode, its configuration, and the seams the later commits build on. The OpenTelemetry gems stay optional (not gemspec dependencies): the collector boot-checks for them and falls back to the agent when they are absent or too old. Configuration gains the collector attribute options (service name, filter/send/response settings) that shape the exported resource. Also adds the shared test infrastructure the rest of the work relies on: the `-collector` gemfile variants, an in-memory OTLP mock server, and the agent/collector dual-mode example helpers.
Route `Appsignal::Logger` through a backend seam: the extension backend keeps the existing C-extension logging, and a new OpenTelemetry backend emits log records through the SDK's logger provider when collector mode is active. `Appsignal::Backends.logger` picks between them. The OpenTelemetry backend maps Ruby severities and formats to their OTLP equivalents and attaches `appsignal.group`/`appsignal.format` as hard overrides so user attributes can't spoof them.
Route the custom metric helpers (`set_gauge`, `increment_counter`, `add_distribution_value`) through `Appsignal::Backends.metrics`: the extension backend keeps the C-extension path (including the out-of-range warning), and a new OpenTelemetry backend records gauges, up/down counters and histograms through the SDK's meter provider when collector mode is active. With both the metrics and logger backends in place, `Appsignal.start` can fall back to the agent when the OpenTelemetry SDK fails to boot, so telemetry is never silently dropped.
Dual-mode the GVL, MRI and Sidekiq probe specs so each metric example runs in both agent and collector mode. The probes emit through the metric helpers, so no probe code changes — this just proves their output routes to OpenTelemetry when collector mode is active.
Back `Appsignal::Transaction` with a pluggable backend. The extension backend keeps the existing C-extension behaviour (it now owns the sample-data serialization, breadcrumb buffering and the agent's error-cause shape). A new OpenTelemetry backend maps a transaction to an OTel root span and each event to a child span, chosen by `Appsignal::Backends.transaction`. Because one `Transaction` drives two error models, the backend exposes `records_errors_eagerly?`: the extension collects errors then duplicates the transaction per error at completion, while the OTel backend records each as an `exception` span event straight away. Key mapping decisions, for reviewers: - Span kind comes from the namespace (web/action_cable => SERVER, background_job => CONSUMER); datastore client events pass `opentelemetry_kind: :client`, set at creation since the kind is immutable. - An event span's name is `title || name`, with the event name kept as `appsignal.category`; SQL bodies map to `db.query.text`, everything else to `appsignal.body`. - Errors attach to the currently-open span with the cause chain in one `appsignal.error_causes` JSON attribute; a discarded transaction sets `appsignal.ignore_subtrace` so the collector drops it.
Dual-mode the hook, integration, Rack and loader specs so each existing event-emission example runs in both agent and collector mode, asserting the OpenTelemetry span shape (name, category, body, kind, parent) in collector mode. No library changes — this exercises the trace backend through every integration. Trace-context propagation (incoming extraction, outgoing injection) is left to the propagation commits, so the specs that also cover it are dual-moded there alongside their library changes.
In collector mode, HTTP client integrations write the current W3C trace context onto their outgoing requests so the downstream service continues the same trace. Net::HTTP, HTTP.rb, Excon and Faraday each inject a `traceparent` header (Excon via a dedicated middleware, HTTP.rb on every hop), and mark the request span with CLIENT kind. Injection is a no-op outside collector mode, so agent-mode behaviour is unchanged. Excon moves to its own gemfile now that it is exercised directly rather than only through Faraday's adapter.
In collector mode, the Rack middleware, Rack event handler and the Webmachine integration read an incoming W3C `traceparent` header and pass it to the transaction. A web (SERVER) transaction continues under the remote span; a job (CONSUMER) transaction starts its own trace linked back to it; with no context, or outside collector mode, it stays a plain root span. `Transaction.create` gains an `opentelemetry_context:` keyword that threads through to the backend, which parents or links the root span accordingly.
Carry the W3C trace context across the enqueue/perform boundary for Sidekiq, Resque, Que, Shoryuken and Active Job. On enqueue (in collector mode) each 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: a job hash key, an SQS message attribute, a Que tag, or Active Job's serialized arguments. It skips propagation rather than breaking the enqueue when a channel is full, and is a no-op outside collector mode.
In collector mode each error is recorded when it is added, but its block was still held until completion. Deferring only makes sense in agent mode, where blocks run against the duplicate transactions fabricated at completion. In collector mode there is no benefit to holding them back. Run the block when the error is added instead. Side effects that target the current span, such as breadcrumbs, nested errors or custom instrumentation, then land where the error was reported rather than on the root span at completion. None of those belong in an error block, but this puts them in the right place if they are used. With blocks no longer deferred in collector mode, the error tracking that supported deferral was doing two jobs at once. `@error_blocks` was both 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. Split them. `@errors` is now the distinct-error set, used in both modes, and `@error_blocks` holds only blocks and is populated only in agent mode. `@errors` is a Set, so membership is by object identity, matching how the errors were deduplicated as hash keys before. Finally, rename `records_errors_eagerly?` to `supports_multiple_errors?`. The old name described the mechanism. The real question at the call site is whether the backend can hold more than one error on a transaction.
The sequel-rails gem emits its queries as `sql.sequel` ActiveSupport::Notifications events. Those are recorded through the generic notifications integration, not the dedicated Sequel hook, so the hook's CLIENT-kind tagging never applied to them. The notifications integration only listed `sql.active_record` as an outgoing datastore call, so Sequel queries were exported with the default (INTERNAL) span kind in collector mode. Add `sql.sequel` to that list so a Sequel query is CLIENT regardless of which path records it, matching ActiveRecord and the dedicated Sequel hook.
The `extract_rack_context` and `extract_job_context` helpers called `OpenTelemetry.propagation.extract` without a `context:` argument, so it defaulted to `Context.current`. The W3C extractor returns its base context unchanged when the carrier has no `traceparent`, so a request with no incoming trace context inherited whatever span happened to be attached to the fiber. In collector mode that let a span left over from an earlier request become the parent of a later request's root span, merging unrelated requests into a single trace. Extract onto `Context.empty` instead, so extraction reflects only the carrier. A real incoming `traceparent` still parents (web) or links (jobs) exactly as before.
A transaction runs blocks supplied by user code in a few places: the error block passed to `set_error`, `send_error`, and `report_error`, and the `after_create` and `before_complete` hooks. Any of them can raise. These blocks can run far from where they were defined. In agent mode an error block runs at completion, not when the error is added. So a raise propagated out of whatever drove creation or completion, surfacing the error in unrelated code. In collector mode it also skipped the backend's completion step, which left the transaction's OpenTelemetry context attached to the fiber. That leaked context then became the parent of the next request's root span on the reused thread, merging unrelated requests into one trace. Agent mode never showed the leak because its extension handle is not a per-fiber stack. Wrap each error block once, where user code hands it in, so it stays protected wherever it later runs. `send_error` and `report_error` name themselves when they wrap the block, so the log points the customer at the call they made rather than at an internal method name. The `after_create` and `before_complete` hooks are guarded where they run, because they can be registered with a block or by pushing a method onto the set, so there is no single entry point to wrap them at. On failure, log the error and the block's definition site, then carry on so creation and completion always finish. The error is swallowed rather than re-raised, because the block can run far from its caller and re-raising would surface it in code that has nothing to do with it.
A web request that never sets an action name (for example a static asset served without a controller) has no action to group by. Agent mode does not report such a transaction. In collector mode the root span is created with a placeholder name, `appsignal.transaction <namespace>`, so every actionless request was surfacing under that one shared placeholder action. Track whether an action was set and, when it was not, flag the root span with `appsignal.ignore_subtrace` on completion, exactly as `discard` does. The AppSignal Collector (0.10.0 and newer) then drops the subtrace, so these requests no longer appear, matching agent mode.
Enqueuing a Delayed Job already records an `enqueue.delayed_job` event on the active transaction, so enqueues made from a web request or another job show up in the event timeline. In collector mode that event now also opens a producer span, matching the other job backends and OpenTelemetry's own Delayed Job instrumentation. Delayed Job has no envelope to carry trace context across the enqueue/perform boundary, so -- like OpenTelemetry -- nothing is injected and the producer and consumer spans are not linked. Dual-mode the Delayed Job specs so the collector-mode span shape runs alongside the agent-mode coverage, adding a `delayed_job-collector` gemfile for the collector run.
Audit of the transaction and helper specs for behaviors covered in only one mode, agent or collector, that should be covered in both. Most gaps were agent-only leftovers that assert through `to_h`-based matchers, which do nothing in collector mode. Fold the two send_error block specs into one dual-moded spec. It sets an active transaction, calls `send_error` with a block, and checks the block's metadata and the error land on `send_error`'s own transaction while the active one is restored and left untouched. In collector mode this confirms the active trace continues rather than being dropped or left open. Cover more behaviors in both modes: the `set_error` and `report_error` blocks, `set_empty_params!`, the `add_params`, `add_session_data`, `add_headers` and `add_custom_data` merge and block variants, `set_action` and `set_namespace` setting the value, `report_error` not completing the active transaction, and a transaction that reports a cause-less error after one with causes. Also add the one missing collector metric-coercion case, a symbol metric name, to the OpenTelemetry metrics backend spec.
The OpenTelemetry logs SDK already caches the logger in its provider, keyed by name and version. Caching the logger again in this backend duplicated that, and the only thing the local cache added was avoiding a provider registry lookup on each emit. Resolve the logger from the provider on every emit instead. This is a registry lookup, not a rebuild, and it always reflects the currently configured provider. That removes the need for the test-only reset! hook, which existed only to drop this cache when the provider was swapped between tests. Update the specs to match. Drop the "logger caching" examples and prove instead that a swapped logger provider is picked up on the next emit without any reset. Remove the reset! calls from the backend spec and from the collector-mode shared context.
Add an optional `opentelemetry_scope:` argument, a `[name, version]` pair that names the OpenTelemetry instrumentation scope a span belongs to. It is threaded from the instrumentation helpers and from `Transaction.create` through the transaction to the backends, the same way `opentelemetry_kind` already is. The instrument, instrument_sql, monitor, monitor_and_stop, send_error and report_error helpers all take it and forward it to the transaction they create. In collector mode the OpenTelemetry backend resolves a tracer for that scope and uses it for the root span and for each event span. A nil scope, or one with a blank name, falls back to the default `appsignal-ruby` scope at the gem version, so every span always carries a scope. The agent backend accepts the argument and ignores it, because agent mode has no notion of instrumentation scope. This wires the mechanism only. Integrations start passing their own scope in later changes. Until then every span keeps the default scope, so behaviour is unchanged. Add a `scope_of` helper to the collector-mode shared context so specs can read a span's instrumentation scope, and cover the plumbing. The scope is forwarded to the backend, the root and event spans are recorded under the given scope, a blank or missing scope falls back to the default, and the error and monitor helpers record their created transaction under a passed scope.
ActiveSupport::Notifications bridges many Rails components through one code path, so a single scope would lump them all together. Derive the instrumentation scope from the event name group, which is the part after the last dot. That puts each component under its own scope, such as appsignal-ruby/active_record and appsignal-ruby/action_view. An event name without a group falls back to the default scope.
Assert the group-derived scope in collector mode: sql.active_record under appsignal-ruby/active_record, sql.sequel under appsignal-ruby/sequel, a dotted custom name under its group, and a name with no group under the default appsignal-ruby scope.
Give the Rack middleware an opentelemetry_scope option and pass it when it creates the request transaction and its wrapping event. The generic middleware, the response body wrapper and the Rack event handler all use the appsignal-ruby/rack scope, so their spans are grouped under the Rack scope in collector mode. Each web framework middleware sets its own scope through this option: appsignal-ruby/sinatra, appsignal-ruby/grape, appsignal-ruby/hanami, appsignal-ruby/padrino and appsignal-ruby/rails. Their request spans are grouped under the matching scope.
Assert in collector mode that the generic Rack middleware and the Rack event handler record their request spans under the appsignal-ruby/rack scope, and that the Sinatra, Grape, Hanami and Rails middlewares record theirs under their framework scope. The Rails case covers the middleware creating the transaction itself, since a full Rails stack usually has the Rack event handler create it first under the Rack scope. Update the Padrino loader spec too. The loader now registers the Sinatra base instrumentation middleware with an opentelemetry_scope option, so the middleware registration assertion expects it.
Each of these integrations now passes its own OpenTelemetry instrumentation scope, a [name, version] pair at the gem version, when it creates a transaction or records an event. In collector mode their spans are grouped under that scope. Background jobs use appsignal-ruby/sidekiq, -que, -shoryuken, -resque, -delayed_job and -active_job. HTTP clients use appsignal-ruby/net_http, -http_rb, -faraday and -excon. Datastores use appsignal-ruby/redis, -redis_client, -sequel, -mongo, -data_mapper and -dry_monitor. Action Cable, Webmachine and Rake use appsignal-ruby/action_cable, -webmachine and -rake. Error reporting through report_error passes a scope too. Puma low-level errors use appsignal-ruby/puma, the Rails error reporter uses appsignal-ruby/rails and the at-exit hook uses appsignal-ruby/at_exit. These only apply the scope when they create the transaction. When an error is reported onto an existing transaction, that transaction keeps its own scope.
Assert in collector mode that each integration records its spans under its own scope. This covers the background jobs, the HTTP clients and the datastores, Action Cable, Webmachine and Rake, and the Puma, Rails and at-exit error reporters. The Puma case also covers that reporting onto an existing transaction keeps that transaction's scope rather than the Puma one.
dd78094 to
506a9f5
Compare
The instrumentation scope name is part of the data that leaves the gem, so a customer can see it and group their spans by it. That makes it a customer-visible change and it needs its own changelog entry, separate from the entry for collector mode itself.
506a9f5 to
ee4fd8a
Compare
|
Merged into The commits were replayed onto the branch rather than merged through this pull request. Because the branch history was then rewritten, the commits on the branch have different SHAs than the ones here, which is why GitHub does not mark this as merged. The eight code and test commits are on the branch. The ninth commit, which only added a changeset, was dropped: the collector mode work stays under the single changeset on Where this met #1552, the keyword arguments were combined rather than either side taking precedence. |
The motivation for implementing this PR comes from the research into how our product exposes allocation counts that led to #1550, and specifically the sample breakdown. The OpenTelemetry-idiomatic equivalent of AppSignal category groups would be instrumentation scopes.
The changes in #1535 already emit the event category, from which we could extract the group, but if we were to build an equivalent to sample breakdowns or allocation breakdowns that was idiomatic to OpenTelemetry, we'd likely want to build it based on instrumentation scopes. Adding instrumentation scopes to each of our integrations enables that change, avoiding the need for retrofitting a category-based solution in the future to account for data from previous versions of the gem.
In collector mode, this makes each AppSignal integration emit its spans under its own OpenTelemetry instrumentation scope, named after the integration and tagged with the gem version. Grouping spans by scope lets the UI show per-scope statistics later, and it works the same way for spans sent by any other OpenTelemetry setup.
The commits come in implementation and test pairs, with the plain scope-passing folded together and the two changes that do more than pass a scope through, the ActiveSupport::Notifications bridge and the Rack layer, split out on their own.
Resolve OTel logger per emit, drop the cache
The OpenTelemetry logs SDK already caches the logger in its provider,
keyed by name and version. Caching the logger again in this backend
duplicated that, and the only thing the local cache added was avoiding a
provider registry lookup on each emit.
Resolve the logger from the provider on every emit instead. This is a
registry lookup, not a rebuild, and it always reflects the currently
configured provider. That removes the need for the test-only reset!
hook, which existed only to drop this cache when the provider was
swapped between tests.
Update the specs to match. Drop the "logger caching" examples and prove
instead that a swapped logger provider is picked up on the next emit
without any reset. Remove the reset! calls from the backend spec and
from the collector-mode shared context.
Add opentelemetry_scope to instrumentation
Add an optional
opentelemetry_scope:argument, a[name, version]pair that names the OpenTelemetry instrumentation scope a span belongs
to. It is threaded from the instrumentation helpers and from
Transaction.createthrough the transaction to the backends, the sameway
opentelemetry_kindalready is. The instrument, instrument_sql,monitor, monitor_and_stop, send_error and report_error helpers all take
it and forward it to the transaction they create.
In collector mode the OpenTelemetry backend resolves a tracer for that
scope and uses it for the root span and for each event span. A nil
scope, or one with a blank name, falls back to the default
appsignal-rubyscope at the gem version, so every span always carriesa scope. The agent backend accepts the argument and ignores it, because
agent mode has no notion of instrumentation scope.
This wires the mechanism only. Integrations start passing their own
scope in later changes. Until then every span keeps the default scope,
so behaviour is unchanged.
Add a
scope_ofhelper to the collector-mode shared context so specscan read a span's instrumentation scope, and cover the plumbing. The
scope is forwarded to the backend, the root and event spans are recorded
under the given scope, a blank or missing scope falls back to the
default, and the error and monitor helpers record their created
transaction under a passed scope.
Derive AS::N span scope from the event group
ActiveSupport::Notifications bridges many Rails components through one
code path, so a single scope would lump them all together. Derive the
instrumentation scope from the event name group, which is the part after
the last dot. That puts each component under its own scope, such as
appsignal-ruby/active_record and appsignal-ruby/action_view. An event
name without a group falls back to the default scope.
Test AS::N derived instrumentation scope
Assert the group-derived scope in collector mode: sql.active_record
under appsignal-ruby/active_record, sql.sequel under
appsignal-ruby/sequel, a dotted custom name under its group, and a name
with no group under the default appsignal-ruby scope.
Tag Rack and web framework spans with scope
Give the Rack middleware an opentelemetry_scope option and pass it when
it creates the request transaction and its wrapping event. The generic
middleware, the response body wrapper and the Rack event handler all use
the appsignal-ruby/rack scope, so their spans are grouped under the Rack
scope in collector mode.
Each web framework middleware sets its own scope through this option:
appsignal-ruby/sinatra, appsignal-ruby/grape, appsignal-ruby/hanami,
appsignal-ruby/padrino and appsignal-ruby/rails. Their request spans are
grouped under the matching scope.
Test Rack and web framework span scope
Assert in collector mode that the generic Rack middleware and the Rack
event handler record their request spans under the appsignal-ruby/rack
scope, and that the Sinatra, Grape, Hanami and Rails middlewares record
theirs under their framework scope. The Rails case covers the middleware
creating the transaction itself, since a full Rails stack usually has
the Rack event handler create it first under the Rack scope.
Update the Padrino loader spec too. The loader now registers the Sinatra
base instrumentation middleware with an opentelemetry_scope option, so
the middleware registration assertion expects it.
Tag remaining integration spans with scope
Each of these integrations now passes its own OpenTelemetry
instrumentation scope, a [name, version] pair at the gem version, when
it creates a transaction or records an event. In collector mode their
spans are grouped under that scope.
Background jobs use appsignal-ruby/sidekiq, -que, -shoryuken, -resque,
-delayed_job and -active_job. HTTP clients use appsignal-ruby/net_http,
-http_rb, -faraday and -excon. Datastores use appsignal-ruby/redis,
-redis_client, -sequel, -mongo, -data_mapper and -dry_monitor. Action
Cable, Webmachine and Rake use appsignal-ruby/action_cable, -webmachine
and -rake.
Error reporting through report_error passes a scope too. Puma low-level
errors use appsignal-ruby/puma, the Rails error reporter uses
appsignal-ruby/rails and the at-exit hook uses appsignal-ruby/at_exit.
These only apply the scope when they create the transaction. When an
error is reported onto an existing transaction, that transaction keeps
its own scope.
Test remaining integration scopes
Assert in collector mode that each integration records its spans under
its own scope. This covers the background jobs, the HTTP clients and the
datastores, Action Cable, Webmachine and Rake, and the Puma, Rails and
at-exit error reporters. The Puma case also covers that reporting onto
an existing transaction keeps that transaction's scope rather than the
Puma one.
Add changeset for per-integration scopes
The instrumentation scope name is part of the data that leaves the
gem, so a customer can see it and group their spans by it. That makes
it a customer-visible change and it needs its own changelog entry,
separate from the entry for collector mode itself.