Code review consensus from Tobi+Sandi: the empty-block call
'Opencode::Instrumentation.instrument(name, payload) { }' at fire-
and-forget call sites in opencode-rails is API smell. Tobi: 'two
named verbs are clearer than one verb with a vestigial block.'
Sandi: 'a method with a block parameter that's optional but expected
empty in some call sites is doing two things.'
Two emission shapes now:
.instrument(name, payload) { ... } # block; duration measured
.notify(name, payload) # fire-and-forget; no block
Both flow through the same adapter. The adapter still always
receives a block argument (some adapters key on it, e.g. AS::
Notifications.instrument requires a block) — .notify passes an
empty {}. Adapter return value is ignored for .notify (it returns
nil); .instrument continues to pass through the block's return.
Three new tests in smoke_test.rb:
- no-op when no adapter set
- forwards to adapter + verifies block presence + verifies that
.notify returns nil (not the adapter's return)
- works without a block at the call site
Also: switched gemspec metadata URLs from Gitea to GitHub. The gem
will eventually publish from github.com/ajaynomics/opencode-ruby —
the metadata now reflects that. (No actual GitHub remote push yet;
that's the user's manual step.)
15 tests pass, 32 assertions, 0 failures.
77 lines
2.9 KiB
Ruby
77 lines
2.9 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
module Opencode
|
|
# Pluggable instrumentation adapter. opencode-ruby ships zero
|
|
# dependencies on Rails or any specific instrumentation library. Users
|
|
# plug in their own emitter:
|
|
#
|
|
# # ActiveSupport::Notifications (Rails apps):
|
|
# Opencode::Instrumentation.adapter = ->(name, payload, &block) {
|
|
# ActiveSupport::Notifications.instrument(name, payload, &block)
|
|
# }
|
|
#
|
|
# # stdout (debugging, non-Rails scripts):
|
|
# Opencode::Instrumentation.adapter = ->(name, payload, &block) {
|
|
# puts "[#{name}] #{payload.inspect}"
|
|
# block.call
|
|
# }
|
|
#
|
|
# When no adapter is set (default), instrumentation is a no-op pass-
|
|
# through that yields the block and returns its value. The Client emits
|
|
# events for HTTP requests, SSE stream lifecycle, and recovery paths.
|
|
#
|
|
# Event names the Client emits:
|
|
#
|
|
# - opencode.request — every HTTP request to OpenCode server
|
|
#
|
|
# If you wire a real adapter, the payload hash carries `:method` and
|
|
# `:path` for opencode.request. Other events may add fields in future
|
|
# versions; treat the payload as forward-compatible.
|
|
#
|
|
# Two emission shapes:
|
|
#
|
|
# .instrument(name, payload) { ... } — wrap a block; the duration
|
|
# of the block becomes part
|
|
# of the event (when the
|
|
# adapter is ActiveSupport::
|
|
# Notifications-shaped).
|
|
#
|
|
# .notify(name, payload) — fire-and-forget; no block,
|
|
# no duration. Use for
|
|
# point-in-time observations
|
|
# (e.g. "this artifact was
|
|
# dropped").
|
|
module Instrumentation
|
|
class << self
|
|
attr_accessor :adapter
|
|
end
|
|
|
|
# Yields the block, optionally routed through the adapter if one is
|
|
# set. Always returns the block's return value (so call sites can
|
|
# wrap their work transparently).
|
|
def self.instrument(name, payload = {})
|
|
return yield unless adapter
|
|
|
|
adapter.call(name, payload) { yield }
|
|
end
|
|
|
|
# Fire-and-forget event. No block, no return value (the adapter's
|
|
# return is ignored). Use for point-in-time observations where
|
|
# duration doesn't apply — apply_patch.artifacts_dropped,
|
|
# session.recreated, etc.
|
|
#
|
|
# Implementation: invokes the same adapter as #instrument but with
|
|
# an empty block. Hosts that adapt to ActiveSupport::Notifications
|
|
# will see a zero-duration event; hosts that adapt to a structured-
|
|
# event API (Rails.event.notify, OpenTelemetry span events) can
|
|
# detect the empty-block convention if they need to. Most hosts
|
|
# don't need to care.
|
|
def self.notify(name, payload = {})
|
|
return unless adapter
|
|
|
|
adapter.call(name, payload) { }
|
|
nil
|
|
end
|
|
end
|
|
end
|