Support OpenTelemetry via collector mode#1515
Conversation
b09cc27 to
195c6c6
Compare
tombruijn
left a comment
There was a problem hiding this comment.
I skimmed it. So far so good :)
197726b to
d724cd1
Compare
6058d8b to
8daa4c4
Compare
There was a problem hiding this comment.
Pull request overview
Introduces an experimental “collector mode” for the Ruby integration by adding a collector_endpoint config option that boots an OpenTelemetry SDK (OTLP/HTTP protobuf) for traces/metrics/logs, while keeping the existing AppSignal agent running. The PR also refactors transaction/metrics/logging to dispatch through mode-specific backends and adds extensive unit + integration test coverage (including a lightweight in-process OTLP mock server) plus CI support via *-collector.gemfile variants.
Changes:
- Add collector mode configuration (
collector_endpoint, collector-only filtering/resource options) with mode-mismatch warnings and Ruby-version gating. - Introduce backend dispatch (
Appsignal::Backends) and implement OpenTelemetry backends for transactions, metrics, andAppsignal::Logger. - Add OTLP mock collector + integration runners/specs and update many specs to assert behavior in both agent and collector modes.
Reviewed changes
Copilot reviewed 116 out of 118 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/support/testing.rb | Update test transaction helpers to expose backend instead of ext. |
| spec/support/shared_contexts/collector_mode.rb | Collector-mode shared context wiring in-memory OTel exporters/providers for specs. |
| spec/support/shared_contexts/agent_mode.rb | Agent-mode shared context to start agent + keep transactions readable post-complete. |
| spec/support/matchers/transaction.rb | Matchers updated to read completion/queue_start via transaction.backend. |
| spec/support/helpers/transaction_helpers.rb | Helper updated to pass backend: instead of ext:. |
| spec/support/helpers/otlp_collector_server.rb | Add minimal TCP OTLP/HTTP mock collector for integration specs. |
| spec/support/helpers/mode_helpers.rb | Add it_in_both_modes helper for dual-mode examples. |
| spec/support/helpers/dependency_helper.rb | Add opentelemetry_present? helper for optional OTel dependencies. |
| spec/spec_helper.rb | Load shared_contexts; filter collector-mode specs when OTel absent; suite hooks for OTLP server/WebMock. |
| spec/lib/appsignal/transaction/opentelemetry_backend_spec.rb | Add unit tests for OTel transaction backend span/context/event behavior. |
| spec/lib/appsignal/transaction/extension_backend_spec.rb | Add unit tests for agent-mode transaction backend delegation/shape. |
| spec/lib/appsignal/rack/event_handler_spec.rb | Split response-status counter assertions into agent vs collector expectations. |
| spec/lib/appsignal/probes/sidekiq_spec.rb | Scope Sidekiq specs to appropriate modes; add collector-mode metric assertions. |
| spec/lib/appsignal/probes/mri_spec.rb | Add collector-mode metric assertions for thread count gauge. |
| spec/lib/appsignal/probes/gvl_spec.rb | Add collector-mode metric assertions for GVL delta gauge. |
| spec/lib/appsignal/probes_spec.rb | Make probe-thread test less fragile by asserting thread replacement directly. |
| spec/lib/appsignal/opentelemetry_spec.rb | Add unit tests for configure/start/shutdown/resource mapping/endpoint normalization. |
| spec/lib/appsignal/metrics/opentelemetry_backend_spec.rb | Add unit tests for OTel metrics backend instruments/attrs/caching. |
| spec/lib/appsignal/logger/opentelemetry_backend_spec.rb | Add unit tests for OTel log backend severity/attrs/scope/caching. |
| spec/lib/appsignal/integrations/net_http_spec.rb | Add collector-mode assertions for event spans emitted by net/http integration. |
| spec/lib/appsignal/integrations/mongo_ruby_driver_spec.rb | Add agent vs collector assertions for Mongo event span + duration metric. |
| spec/lib/appsignal/integrations/data_mapper_spec.rb | Add collector-mode event span assertions for DataMapper log listener. |
| spec/lib/appsignal/integrations/active_support_event_reporter_spec.rb | Add collector-mode assertions for logs routed via OTel backend. |
| spec/lib/appsignal/hooks/sequel_spec.rb | Add collector-mode event span assertions for Sequel SQL spans. |
| spec/lib/appsignal/hooks/redis_spec.rb | Refactor to support both modes; add collector-mode event span assertions. |
| spec/lib/appsignal/hooks/redis_client_spec.rb | Refactor to support both modes; add collector-mode event span assertions. |
| spec/lib/appsignal/hooks/excon_spec.rb | Refactor to support both modes; add collector-mode event span assertions. |
| spec/lib/appsignal/hooks/dry_monitor_spec.rb | Refactor to support both modes; add collector-mode event span assertions. |
| spec/lib/appsignal/hooks/activejob_spec.rb | Add dual-mode queue job count metric assertions; keep transaction-shape agent-only. |
| spec/lib/appsignal/hooks/active_support_notifications/start_finish_shared_examples.rb | Add dual-mode span/event assertions for start/finish paths. |
| spec/lib/appsignal/hooks/active_support_notifications/instrument_shared_examples.rb | Add dual-mode span/event assertions for instrument paths + error/throw/complete cases. |
| spec/lib/appsignal/hooks/active_support_notifications/finish_with_state_shared_examples.rb | Add dual-mode span/event assertions for finish_with_state. |
| spec/lib/appsignal/hooks/active_support_notifications_spec.rb | Remove hardcoded agent setup to allow mode contexts to control start. |
| spec/lib/appsignal/hooks/action_mailer_spec.rb | Add collector-mode metric assertions for action_mailer_process counter. |
| spec/lib/appsignal/config_spec.rb | Add/verify new config options, env mappings, gates, and mode-mismatch warnings. |
| spec/lib/appsignal/backends_spec.rb | Add tests for backend selection based on collector_mode?. |
| spec/integration/stop_spec.rb | Remove redundant exitstatus assertion (Runner now raises on non-zero exits). |
| spec/integration/runners/stop_with_trap.rb | Switch runner config to rely on Runner-provided env vars + Appsignal.start. |
| spec/integration/runners/collector_mode_traces.rb | New runner emitting trace spans via Appsignal monitor/instrument APIs. |
| spec/integration/runners/collector_mode_stop_flush.rb | New runner verifying Appsignal.stop flushes metrics/logs via provider shutdown. |
| spec/integration/runners/collector_mode_mixed_api.rb | New runner exercising mixed raw OTel + AppSignal span parenting rules. |
| spec/integration/runners/collector_mode_metrics.rb | New runner emitting metric helpers in collector mode. |
| spec/integration/runners/collector_mode_logs.rb | New runner emitting Appsignal::Logger records in collector mode. |
| spec/integration/runners/collector_mode_log_trace_correlation.rb | New runner validating log trace/span correlation IDs. |
| spec/integration/runners/collector_mode_fork.rb | New runner validating OTel metric export continues after fork (Ruby 3.1+). |
| spec/integration/runners/collector_mode_emit.rb | New runner asserting resource attribute mapping + emits all 3 OTLP signals. |
| spec/integration/runner.rb | Enhance Runner with default env + per-run working dir + non-zero exit raising. |
| spec/integration/collector_mode_traces_spec.rb | Integration spec decoding OTLP trace requests and asserting span tree/attrs. |
| spec/integration/collector_mode_stop_flush_spec.rb | Integration spec asserting stop triggers buffered exports (metrics/logs). |
| spec/integration/collector_mode_spec.rb | Integration spec asserting collector mode config + resource attrs for all signals. |
| spec/integration/collector_mode_mixed_api_spec.rb | Integration spec asserting correct nesting relative to ambient OTel context. |
| spec/integration/collector_mode_metrics_spec.rb | Integration spec asserting metric helper exports and scope name. |
| spec/integration/collector_mode_logs_spec.rb | Integration spec asserting log exports, severity mapping, and attributes. |
| spec/integration/collector_mode_log_trace_correlation_spec.rb | Integration spec asserting log records carry active span trace/span IDs. |
| spec/integration/collector_mode_fork_spec.rb | Integration spec asserting periodic metric reader fork hooks work (non-JRuby). |
| sig/appsignal.rbs | Add signatures/docs for collector_mode_configured? / collector_mode?; add Metrics module stub. |
| sig/appsignal.rbi | Add Sorbet RBI for collector-mode predicates; add Metrics module stub. |
| Rakefile | Add build-matrix support for generated *-collector.gemfile variants + CI runs. |
| lib/appsignal/utils/stdout_and_logger_message.rb | Add error helper (stderr + logger) alongside existing warning helper. |
| lib/appsignal/transaction/opentelemetry_backend.rb | Implement collector-mode transaction backend emitting OTel spans for events. |
| lib/appsignal/transaction/extension_backend.rb | Implement agent-mode transaction backend delegating to extension handle. |
| lib/appsignal/transaction.rb | Switch transaction internals from @ext to backend abstraction (@backend). |
| lib/appsignal/opentelemetry/dependencies.rb | Centralize minimum supported OTel gem versions for collector mode. |
| lib/appsignal/opentelemetry/attributes.rb | Add shared attribute coercion logic for OTel metrics/logs backends. |
| lib/appsignal/opentelemetry.rb | Implement OTel SDK configuration, shutdown, and resource mapping for collector mode. |
| lib/appsignal/metrics/opentelemetry_backend.rb | Implement collector-mode metric helper routing via OTel meter/instruments. |
| lib/appsignal/metrics/extension_backend.rb | Extract existing extension-backed metric helper logic into dedicated backend. |
| lib/appsignal/logger/opentelemetry_backend.rb | Implement collector-mode logger routing via OTel logs SDK with attributes. |
| lib/appsignal/logger/extension_backend.rb | Extract existing extension-backed logger emission into dedicated backend. |
| lib/appsignal/logger.rb | Dispatch logger emits via backend; validate format values; remove unused set dependency. |
| lib/appsignal/integrations/mongo_ruby_driver.rb | Emit Mongo command body as JSON string for backend compatibility. |
| lib/appsignal/helpers/metrics.rb | Dispatch metric helpers via backend abstraction (agent vs collector). |
| lib/appsignal/config.rb | Add collector options, Ruby-version gate, collector predicates, and mode-mismatch warnings. |
| lib/appsignal/backends.rb | Add central backend selector for metrics/logger/transaction based on collector mode. |
| lib/appsignal.rb | Boot OTel SDK during start when collector mode configured; shutdown OTel on stop. |
| gemfiles/collector.rb | Add Gemfile fragment to install optional OTel gems using centralized version floors. |
| gemfiles/no_dependencies-collector.gemfile | Generated collector gemfile layering collector.rb on base gemfile. |
| gemfiles/code_ownership-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/ownership-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/capistrano2-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/capistrano3-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/dry-monitor-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/excon-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/grape-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/hanami-2.0-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/hanami-2.1-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/hanami-2.2-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/http5-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/http6-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/padrino-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/psych-3-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/psych-4-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/que-1-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/que-2-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/rails-6.0-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/rails-6.1-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/rails-7.0-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/rails-7.1-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/rails-7.2-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/rails-8.0-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/rails-8.1-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/redis-4-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/redis-5-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/resque-2-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/sequel-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/sidekiq-7-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/sidekiq-8-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/sinatra-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/webmachine1-collector.gemfile | Generated collector gemfile variant. |
| gemfiles/webmachine2-collector.gemfile | Generated collector gemfile variant. |
| build_matrix.yml | Add list of Ruby versions that should also run -collector gemfiles. |
| appsignal.gemspec | Document OTel deps as optional (not gemspec deps) for collector mode. |
| .changesets/add-collector-mode.md | Changeset documenting collector mode and required optional gems. |
| .changesets/add-collector-attribute-options.md | Changeset documenting collector-only config/resource options + mismatch warnings. |
Files not reviewed (1)
- sig/appsignal.rbi: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a6e5146 to
ff250d0
Compare
Match Net::HTTP, which records only the method, scheme, and host. The formatter no longer returns the path as the event body.
Record the request.faraday event from our own Faraday middleware with Appsignal.instrument, as a client event, instead of relying on Faraday's ActiveSupport notifications middleware. This matches the Net::HTTP integration and removes ActiveSupport as a dependency of Faraday support. Trace context is now injected from within that event, so the outgoing request still carries the Faraday client span's traceparent. The generic ActiveSupport::Notifications path now skips request.faraday through SUPPRESSED_EVENT_NAMES instead of marking it a client event, so a manually added notifications middleware doesn't record the request a second time.
Merging main brought RuboCop 1.87 and its config, which now disables Naming/AccessorMethodName and Metrics/AbcSize project-wide. Remove the inline disable directives those cops no longer need.
Address review feedback on the enqueue instrumentation tests. The hook spec now asserts the client middleware ends up registered exactly once, mirroring Sidekiq's own chain that dedupes by class, so the double registration across the server and client config blocks can't produce duplicate enqueue events unnoticed. The agent-mode pass-through spec now asserts that enqueuing without an active transaction records nothing, matching the collector-mode spec that already checks no span is emitted.
Rebasing onto main reverted the HTTP.rb integration to the older perform-based hook. Restore main's request/Session instrumentation (one event per request, spanning redirects, and http6 chained requests through Session) and keep per-hop trace-context injection on a separate perform prepend.
Restore refinements that live on main but were reverted by the rebase: the nested-safe HTTP client suppression, the MongoDB body via Appsignal::Utils::JSON, and the matching spec updates for Sidekiq and the ActiveSupport notifications suppression.
Regenerate .github/workflows/ci.yml from build_matrix.yml so it reflects the current build matrix after the rebase.
Prepend QueBulkClientPlugin on Que 2 so a bulk_enqueue records one bulk_enqueue.que producer event for the whole batch and injects the trace context once; the inner enqueues pass through. Single enqueue already recorded enqueue.que, so extract the shared recording into record_enqueue and cover both single and bulk on Que 1 and Que 2. This work was present before the wip-opentelemetry rebase against main but dropped during it; the accompanying changeset was dropped too, so restore it here. See #1522 for the main-based backport.
Add a Shoryuken client middleware that records an enqueue.shoryuken event on the active transaction (both modes) and, in collector mode, writes the trace context onto the outgoing message as SQS message attributes so the job links back to the enqueuer. The server middleware reads that context to link the job's trace. Registered on both the client and server configs, since workers enqueue jobs too. Adds the shoryuken gemfiles, build matrix entry and CI jobs, and the shoryuken_present? dependency helper. This work was present before the wip-opentelemetry rebase against main but dropped during it; restore it here. See #1521 for the main-based backport.
Shoryuken loads the AWS SDK, which since v3 requires aws-sdk-sqs to be installed separately. Older Rubies resolve that combination in CI and fail to load shoryuken without the gem present.
Active Job enqueues through an adapter (Sidekiq, Resque, Que), each of which has its own enqueue instrumentation. A single enqueue was recorded twice: as the Active Job event and once as the nested adapter event. Active Job now records the enqueue inside a suppression block, so the adapter's own enqueue instrumentation skips recording while nested. The enqueue is recorded once, as the outermost event. This mirrors how the Faraday integration suppresses the downstream HTTP client. The adapter integrations react to this flag in their own commits.
When enqueuing through Active Job, the enqueue is already recorded as an `enqueue.active_job` event. The Sidekiq client middleware now skips recording its own `enqueue.sidekiq` event while enqueue events are suppressed, so the enqueue is recorded once. The trace context is still injected onto the job, so the performed job links back to the enqueuer.
When enqueuing through Active Job, the enqueue is already recorded as an `enqueue.active_job` event. The Resque push wrapper now skips recording its own `enqueue.resque` event while enqueue events are suppressed, so the enqueue is recorded once. The trace context is still injected onto the job, so the performed job links back to the enqueuer.
When enqueuing through Active Job, the enqueue is already recorded as an `enqueue.active_job` event. The Que enqueue wrapper now skips recording its own `enqueue.que` (or `bulk_enqueue.que`) event while enqueue events are suppressed, so the enqueue is recorded once. The trace context is injected onto the job's tags, so the performed job links back to the enqueuer.
When enqueuing through Active Job, the enqueue is already recorded as an `enqueue.active_job` event. The Shoryuken client middleware now skips recording its own `enqueue.shoryuken` event while enqueue events are suppressed, so the enqueue is recorded once. The trace context is still injected onto the message, so the performed job links back to the enqueuer.
The `enqueue.active_job` event carries an "enqueue <Class> job" title so the timeline shows which job was enqueued. In collector mode this becomes the event span name; the dotted name stays as the category.
The `enqueue.sidekiq` event now carries an "enqueue <Class> job" title so the timeline shows which job was enqueued. In collector mode this becomes the event span name; the dotted name stays as the category.
The `enqueue.resque` event now carries an "enqueue <Class> job" title so the timeline shows which job was enqueued. In collector mode it becomes the event span name; the dotted name stays as the category.
The `enqueue.que` event now carries an "enqueue <Class> job" title, and `bulk_enqueue.que` a "bulk enqueue <Class> jobs" title, so the timeline shows which job was enqueued. The class is resolved the way Que resolves it: an explicit `:job_class`, else the class the enqueue was called on. For a bulk enqueue called on `Que::Job` itself the class isn't known up front, so the title is left class-less.
The `enqueue.shoryuken` event now carries an "enqueue <Class> job" title when the enqueue goes through a Shoryuken worker, so the timeline shows which job was enqueued. A raw `send_message` has no worker class, so they fall back to "enqueue on <queue>". In collector mode this becomes the event span name; the dotted name stays as the category.
The inner enqueues in a bulk_enqueue block detected the batch by reading Que's internal :que_jobs_to_bulk_insert thread-local. Set our own flag in the bulk_enqueue wrapper instead, so the pass-through doesn't depend on Que internals.
Rebasing onto main pulled in the backports' agent-mode enqueue specs, which the wip dual-mode specs already cover. Keep the dual-mode versions and drop the duplicates.
main's enqueue backport split Shoryuken CI into a 6 and 7 version matrix, while wip tested a single Shoryuken in collector mode. Adopt the version matrix and generate collector variants for both, so each version runs in both modes.
|
Closing in favour of #1535. |
This is a work in progress. Nothing about this approach is set in stone.
This is a starting point for testing out the different potential approaches to OpenTelemetry support via the (hosted) collector on the Ruby integration.
It is in part a re-implementation of the work done in the Python integration, particularly of Python's experimental collector mode and Python's metric helpers.
What this does
collector_endpointconfiguration option.Appsignal::Logger, through OpenTelemetry.Appsignal::Transactioninto OpenTelemetry span trees and emits them through OpenTelemetry, testing each library and framework integration thoroughly.What this does not do (yet)
Non-decisions
The following are things that the current implementation does, but that are explicitly not set in stone:
enable_opentelemetry_httpemit a warning as well when used alongside collector mode, as that's likely not intended behaviour either.