Skip to content

Use standard OTel attributes for on_tool_interrupt telemetry - #275

Merged
altruby merged 2 commits into
r-uby-dev:mainfrom
azmi2409:otel-interrupt-standard-attrs
Oct 11, 2026
Merged

altruby merged 2 commits into
r-uby-dev:mainfrom
azmi2409:otel-interrupt-standard-attrs

Conversation

@azmi2409

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #259.

gen_ai.tool.interrupt is not part of the OpenTelemetry standard (as altruby noted post-merge on #259), so this replaces the custom span event in on_tool_interrupt with the approach he suggested — the same pattern used by on_tool_error and on_interrupt:

attributes = {"error.type" => "LLM::Interrupt"}
attributes.each { span.set_attribute(_1, _2) }
span.status = ::OpenTelemetry::Trace::Status.error("tool interrupted")
span.add_event("gen_ai.tool.finish")
span.tap(&:finish)

The public API (on_tool_interrupt(ex:, span:)) is unchanged.

Tests

  • Updated spec/tracer/telemetry_spec.rb to assert the new behavior: error.type is recorded as "LLM::Interrupt", the span status is an error, and the standard gen_ai.tool.finish event is emitted (instead of gen_ai.tool.interrupt).
  • bundle exec rspec spec/tracer/telemetry_spec.rb: 26 examples, 0 failures.
  • Full suite has 43 pre-existing failures (console specs, unrelated); verified identical on pristine upstream before my change.

Follow-up to r-uby-dev#259. Replaces the non-standard `gen_ai.tool.interrupt`
span event with the pattern maintainer altruby suggested:
`error.type` => "LLM::Interrupt", error status, and the standard
`gen_ai.tool.finish` event — mirroring how `on_tool_error` and
`on_interrupt` handle it.
Comment thread lib/llm/tracer/telemetry.rb Outdated
@altruby

altruby commented Oct 11, 2026

Copy link
Copy Markdown
Member

@azmi2409
Looks good to me :) Thanks a lot. I left one comment for you to check out.

@altruby altruby added this to the v16.1.0 milestone Oct 11, 2026
@altruby
altruby merged commit c89c839 into r-uby-dev:main Oct 11, 2026
6 checks passed
@altruby

altruby commented Oct 11, 2026

Copy link
Copy Markdown
Member

@azmi2409

Full suite has 43 pre-existing failures (console specs, unrelated); verified identical on pristine upstream before my change.

Sorry. I just saw this now.
Are you on Windows by any chance? Could we skip those tests on Windows?

@azmi2409

Copy link
Copy Markdown
Contributor Author

Not on Windows here — Linux. And a correction on my PR-body note: those failures weren't console specs. They were the alibaba provider specs failing locally because DASHSCOPE_API_HOST wasn't set (CI sets it to token-plan.ap-southeast-1.maas.aliyuncs.com); the VCR cassettes are recorded against that host. With the env var set, the full suite is green here — 1924 examples, 0 failures — and spec/console passes standalone (114 examples, 0 failures). So there's nothing Windows-specific in the suite that I can see, and CI is Linux-only anyway.

If you're seeing console-spec failures on your Windows machine, my guess would be the curses dev-dependency — it's effectively uninstallable on Windows, which would make require "llm/console" raise and take down every console spec at load. If you can paste the actual error you're getting, I can look at whether a Gem.win_platform? skip (or a graceful fallback when curses is missing) is the right call rather than skipping blind.

@altruby

altruby commented Oct 11, 2026

Copy link
Copy Markdown
Member

And a correction on my PR-body note: those failures weren't console specs. They were the alibaba provider specs failing locally because DASHSCOPE_API_HOST wasn't set (CI sets it to token-plan.ap-southeast-1.maas.aliyuncs.com);

Ah, right.

I think we could add DASHSCOPE_API_HOST=token-plan.ap-southeast-1.maas.aliyuncs.com to the .env.sample file. At least then if you run cp .env.sample .env you don't have to worry about setting that variable. I'm not sure how easy it is to bootstrap this project from zero. There are a lot of development dependencies to install and some aren't trivial (for example the pg gem).

my guess would be the curses dev-dependency — it's effectively uninstallable on Windows, which would make require "llm/console" raise and take down every console spec at load

Yeah. I think the curses dependency is broken, and the fork specs also won't work. I don't have a Windows machine to test on, but I created #276 so we could try to run Windows on CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants