Repository navigation
Conversation
altruby
left a comment
There was a problem hiding this comment.
Thank you very much for your contribution as always :)
This looks good, but I think we can make a few improvements.
What do you think? Cheers.
| @@ -0,0 +1,242 @@ | |||
| # frozen_string_literal: true | |||
|
|
|||
| module LLM | |||
There was a problem hiding this comment.
There is an unwritten rule in llm.rb that we open - at most - two levels of nesting.
So for example - this should be:
class LLM::Tracer
class Multi < self
end
end| # tracer, so the remaining tracers still run. An | ||
| # {LLM::Interrupt LLM::Interrupt} travels up the stack, like | ||
| # everywhere else in the gem. | ||
| def safely(tracer) |
There was a problem hiding this comment.
I think we can drop this method.
There are two reasons or scenarios to think about, and we cover both of them without this code.
Scenario 1
The first scenario is where Multi raises an error. In that case Multi cannot forward the method call to the tracer(s) it wraps, and we never reach them. Since Multi inherits LLM::Tracer::Rescue - like all classes who inherit from LLM::Tracer - it means when a Multi callback does raise an error, we already catch it via the Rescue module it has inherited, and we would print something like this:
an llm.rb tracer has crashed
[tracer] LLM::Tracer::Multi
[class ] NotImplementedError
# ...
Scenario 2
The second scenario is where the Multi tracer does not raise an error but one of the tracers that it wraps does. This is also covered by the Rescue module, and it covers every tracer under Multi's supervision. In this case, we would print something like:
an llm.rb tracer has crashed
[tracer] TracerBeingWrapped
[class ] NotImplementedError
# ...
And the exception never reaches the Multi tracer. Does that make sense?
| self | ||
| end | ||
|
|
||
| ## |
There was a problem hiding this comment.
The spans and flush! methods are unique to the Telemetry tracer as far as I know.
We shouldn't implement them here. I think we should consider something like:
class Multi < self
##
# Returns the tracer(s) under the supervision of Multi
# @return [Array<LLM::Tracer>]
attr_reader :tracers
endThen if you need to access spans, or flush, you can do:
t = multi.tracers.grep(LLM::Tracer::Telemetry).first
t.spans
t.flush!
Fixes #273
Adds
LLM::Tracer::Multi, a tracer that fans out every tracingcallback to a set of tracers, so a request can (for example) be
logged to a file and exported to OpenTelemetry at the same time:
Design notes:
on_request_start/on_tool_startare trackedper tracer in an
LLM::Tracer::Multi::Spanswrapper, and eachtracer is handed back the span it returned when the request or
tool finishes, errors, or is interrupted.
error is reported the way
LLM::Tracer::Rescuereports a crashedtracer, and the remaining tracers still run.
LLM::Interruptstill travels up the stack.
start_trace/stop_trace,set_finish_metadata_proc, andmerge_extrafan out;spansaggregates every tracer's spans;flush!fans out.set tracer: :multi_tracerform from the issue already works:agent options resolve a symbol through a (possibly private) method
via
LLM.resolve_option, so no agent-side change was needed.Tests: new
spec/tracer/multi_spec.rb(15 examples, all green).Full
spec/tracer+spec/agent_spec.rbrun: 310 examples,0 failures.