feat(monitoring): emit ActiveSupport::Notifications for agent operations#330
Open
bexchauveto wants to merge 5 commits into
Open
feat(monitoring): emit ActiveSupport::Notifications for agent operations#330bexchauveto wants to merge 5 commits into
bexchauveto wants to merge 5 commits into
Conversation
Add a backend-agnostic observability seam so host apps can monitor what the agent does without any new dependency. A tiny helper (ForestAdminDatasourceToolkit::Monitoring) emits events under the "*.forest_admin" namespace; each carries a monotonic duration and an identifying payload (collection, record id, acting user id/rendering/project). Events: - request.forest_admin (agent route dispatch: route, collection, id, method) - computed_field, action, chart, hook, segment, search, operator_emulation, override, write (customizer decorators) Hooks are guarded to stay silent when none are registered; computed fields are instrumented at the batch call (one event per field, not per record). The gem never configures an SDK — that stays the host app's responsibility. Bug fixes surfaced while wiring identity into payloads: - Caller#project had no attr_reader (would NoMethodError) - SegmentCollectionDecorator#refine_filter discarded its caller and compute_segment used Kernel#caller (backtrace Array) as the context caller - write replacer invoked the user handler twice per field; collapsed to a single evaluation Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Autocorrected: %q{...} -> '...' in caller test doubles (Style/RedundantPercentQ,
Style/PercentLiteralDelimiters) and block.call -> yield in Hooks#instrument.
No behavior change; toolkit/customizer/agent suites green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hook.forest_admin carried only {operation, position}, so a subscriber
couldn't tell which collection or user triggered the hook. Thread the hook
context into Hooks#instrument and include collection + caller identity, matching
the other customizer events.
Adds a `name` delegation to RelaxedCollection so context.collection.name is
available (also useful to user customization code).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a `caller:` keyword to Monitoring.instrument that auto-merges the acting
user's non-PII identity, and convert every instrumentation site to it. The
call is now identical everywhere — collection decorators, the computed-field
utility class, the plain Hooks object, the datasource chart decorator, and the
agent route wrapper all use the same signature:
Monitoring.instrument(event, payload, caller:) { work }
This is the pattern a client follows for their own custom instrumentation, from
any context (no CollectionDecorator subclass required). Drops the per-site
`.merge(caller_payload(...))` boilerplate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 new issues
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| # Uniform entry point, callable from anywhere (decorator, utility class, plain | ||
| # object, or a client's custom handler). Pass `caller:` to auto-merge the acting | ||
| # user's non-PII identity (user_id/rendering_id/project) into the payload. | ||
| def self.instrument(event, payload = {}, caller: nil, &block) |
Ships the host-side monitoring as a config-driven convenience (Layer 2) so most
clients get it by flipping one flag instead of writing a subscriber. The agent
still only emits; this turns the notifications into structured log lines.
- config.monitoring = { enabled: true, ... } (disabled by default)
- STDOUT + JSON by default, tagged "source":"forest_admin" (auto-collected by
Datadog/CloudWatch/k8s, filterable when mixed); output: also takes a file
path (rotated) or a custom Logger
- baked in: X-Request-Id correlation middleware, hook filtering, PII-safe SQL
(binds omitted), 3-tier SQL summarisation (off/medium/full)
- ForestAdminRails::Monitoring.subscribe! kept app-free and unit-tested
- install! wired from an engine initializer (after load_config_initializers so
the middleware can still be inserted); no-op unless enabled
- install generator config template documents the option
Advanced backends (StatsD, Sentry) still use a code subscriber — a YAML value
can't call a backend SDK.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| write(logger, config, name: event.name, duration_ms: event.duration, | ||
| payload: event.payload.except(:exception, :exception_object), | ||
| error: event.payload[:exception_object]) | ||
| end |
| entry[:count] += 1 | ||
| entry[:ms] += event.duration | ||
| end | ||
| end |
| request_id: request_id }.merge(payload) | ||
| data[:error] = "#{error.class}: #{error.message}" if error | ||
| logger.info(JSON.generate(data.compact)) | ||
| end |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| # unit-testable without booting Rails. | ||
| def subscribe!(logger, options = {}) | ||
| config = DEFAULTS.merge(options.transform_keys(&:to_sym)) | ||
| ignore = config[:ignore].map(&:to_s) |
There was a problem hiding this comment.
🟠 High forest_admin_rails/monitoring.rb:69
subscribe! calls config[:ignore].map(&:to_s), which raises NoMethodError when a host app sets ignore: nil or ignore: 'hook' in its monitoring config. This crashes subscriber installation during boot instead of treating the option as empty or coercing a single value. Consider normalizing with Array(config[:ignore]).map(&:to_s).
Suggested change
| ignore = config[:ignore].map(&:to_s) | |
| ignore = Array(config[:ignore]).map(&:to_s) |
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_rails/lib/forest_admin_rails/monitoring.rb around line 69:
`subscribe!` calls `config[:ignore].map(&:to_s)`, which raises `NoMethodError` when a host app sets `ignore: nil` or `ignore: 'hook'` in its monitoring config. This crashes subscriber installation during boot instead of treating the option as empty or coercing a single value. Consider normalizing with `Array(config[:ignore]).map(&:to_s)`.
| end | ||
|
|
||
| def call(env) | ||
| Thread.current[:forest_request_id] = env['action_dispatch.request_id'] |
There was a problem hiding this comment.
🟡 Medium forest_admin_rails/monitoring.rb:44
The monitoring state stored in Thread.current (:forest_request_id, :forest_in_request, :forest_sql_acc) is fiber-local in Ruby 3.x, so when a request's execution crosses a fiber boundary the request id is silently lost and SQL aggregation/logging stops for the rest of the request. RequestTagger#call sets Thread.current[:forest_request_id] and the SQL subscriber gates on Thread.current[:forest_in_request], but neither value survives a fiber switch. Since the file already requires active_support/isolated_execution_state, consider using ActiveSupport::IsolatedExecutionState instead of Thread.current for all three keys so the state follows the request's logical execution scope.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_rails/lib/forest_admin_rails/monitoring.rb around line 44:
The monitoring state stored in `Thread.current` (`:forest_request_id`, `:forest_in_request`, `:forest_sql_acc`) is fiber-local in Ruby 3.x, so when a request's execution crosses a fiber boundary the request id is silently lost and SQL aggregation/logging stops for the rest of the request. `RequestTagger#call` sets `Thread.current[:forest_request_id]` and the SQL subscriber gates on `Thread.current[:forest_in_request]`, but neither value survives a fiber switch. Since the file already requires `active_support/isolated_execution_state`, consider using `ActiveSupport::IsolatedExecutionState` instead of `Thread.current` for all three keys so the state follows the request's logical execution scope.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Adds a backend-agnostic observability seam so host apps can monitor what the agent does — request latency, slow computed/smart fields, actions, charts, hooks — without any new dependency and without the gem owning an APM SDK.
The agent is a library mounted in a host Rails app; it must not configure OpenTelemetry, register exporters, or install signal handlers (that's the host's job). So instead of an SDK, it emits
ActiveSupport::Notificationsevents — already a dependency of every package, backend-agnostic, and bridgeable to OpenTelemetry later (opentelemetry-instrumentation-active_support) with zero further gem change.The seam — one uniform entry point
A single helper in
forest_admin_datasource_toolkit(the lowest common package), callable from anywhere — a decorator, a utility class, a plain object, or a client's custom handler:Emits
"#{event}.forest_admin"with the block's monotonic duration and a compacted payload. Thecaller:keyword auto-merges the acting user's non-PII identity (user_id,rendering_id,project). A host subscribes once (ActiveSupport::Notifications.subscribe(/\.forest_admin$/)) and routes to logs / StatsD / Datadog / Sentry — the gem chooses nothing. This is the exact same call a client uses for their own custom instrumentation.Events emitted
request.forest_adminadd_route)computed_field.forest_adminaction/chart/hook/segment/search/operator_emulation/override/write.forest_admin+ caller= non-PII acting user. Hooks are guarded (silent when none registered) and carrycollection+ caller so they correlate like the rest; computed fields are instrumented at the batch call (one event per field, not per record).Why not centralize onto the base
CollectionDecoratorThe customizer wraps every collection in a ~23-layer decorator stack; a single
listcascades through all layers. Base-class instrumentation would fire ~15–20 nested events per operation and duplicate therequest.forest_admintiming. Instead, generic ops are covered at the right layer (requestat the top,sql.active_record/Mongo at the bottom), semantic events are per-decorator, and the uniformMonitoring.instrumentcall is the opt-in extension point for custom code.Bugs fixed along the way
Caller#projecthad noattr_reader—caller.projectwouldNoMethodError(a verified test double caught it).SegmentCollectionDecorator#refine_filterdiscarded itscallerarg andcompute_segmentaccidentally used Ruby'sKernel#caller(a backtraceArray) as the context caller. Threaded the real caller through.namedelegation toRelaxedCollection(also useful to user customization code) so hook events can carry the collection name.Overhead
Measured (ActiveSupport 8.1, Ruby 3.3): ~330–450 ns per instrumented op with no subscriber (paid always), ~3.6 µs with a log subscriber. Per request: microseconds unmonitored, well under 1% with full monitoring on.
Tests
New/updated specs assert each event fires with the right payload. Full suites green: toolkit 463 / customizer 695 / agent 721, 0 failures. Rubocop clean.
Not in scope (deliberate)
No SDK bootstrap in the gem, no new dependency, no config toggle (subscriber presence is the switch), no baked-in metrics client, no base-decorator auto-instrumentation. The host-side reference integration (log file, request-id correlation, tiered SQL) lives in a separate app, not this gem.
🤖 Generated with Claude Code
Note
Emit ActiveSupport::Notifications for agent operations and add structured logging in Rails
ForestAdminDatasourceToolkit::Monitoringmodule that wrapsActiveSupport::Notifications.instrumentto emit*.forest_adminevents with standardized caller metadata (user_id, rendering_id, project).action.forest_admin,hook.forest_admin).ForestAdminRails::Monitoringwith a Rails engine initializer that, whenForestAdminRails.config.monitoring.enabledis true, installs a request correlation middleware and subscribes structured JSON or text logs to all*.forest_adminand SQL events.WriteReplaceCollectionDecorator#rewrite_keywhere user-defined write handlers were called twice; they are now called exactly once.Macroscope summarized c024cf4.