From 075d41a698cceca121199929d96f5f5454aa07c4 Mon Sep 17 00:00:00 2001 From: Brent Rager Date: Tue, 18 Aug 2026 19:28:28 -0400 Subject: [PATCH] otel: make Go/Python/TS/.NET tool spans survive ingest, and stop the zero cost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four non-Rust engines had every defect Rust had, unchanged. Prod is Rust-only so nothing customer-facing was affected, but anyone dogfooding `th operator serve --lang go` got the same silent data loss. **Tool spans were being discarded at ingest.** The OTLP ingest builds a span's attribute set from the resource attrs plus THAT span's own, with no inheritance from the parent, and its LLM-event gate keys on `gen_ai.system`. None of the four set it on the tool span — so those spans never became rows, exactly as Rust's didn't for their entire existence. They now carry their own `gen_ai.system`, `gen_ai.operation.name`, `gen_ai.conversation.id` and (where the engine has one) `smooai.org_id`. `gen_ai.operation.name` is spelled exactly `chat` / `tool`. The ingest takes the attribute verbatim when present and only derives it from the span name as a fallback, so `execute_tool` would land in the column and match nothing. **Cost reaches the span for the first time in all four**, as `gen_ai.usage.cost_usd` when positive and `smooai.gen_ai.cost_unavailable` = `unpriced` otherwise. Never a bare 0: the gateway answers 0 for a model it has no price for, so a zero means "not measured", never "free". Two engine-specific bugs found while wiring this, both fixed: - **.NET published `input_tokens = 0`.** It guarded only on `sawUsage`, which a usage chunk carrying null counts sets while both totals resolve to 0 via `?? 0`. The other three guard on the counts. Now so does .NET. - **.NET was the one engine with a literal `new TurnUsage(0, ...)`** on the cost fallback path — its own XML doc admitted "0 means 'nothing priced it', not 'free'" while nothing downstream could tell. The span no longer carries that zero. Per instruction, the fabrication was NOT ported for parity's sake: parity means the same honest contract — absent where the value is unknown — not the same lies. Each engine's telemetry test asserts the new contract, and each fails without the change: Go `span missing attribute "gen_ai.system"`, Python on the same attribute, TS `FAIL (1)`, .NET `StreamingTurnEmitsGenAiSpansWithModelAndToolArgs [FAIL]`. Go 2 packages ok · Python 364 passed · TypeScript 357 passed / 38 files · .NET 575 passed across 5 assemblies. go vet and tsc --noEmit clean. `python/server/uv.lock` picks up a 1-line correction: the lock recorded the local package at 1.52.3 while pyproject says 1.54.1, i.e. it was stale on main. Not in this PR: usage/cost provenance and `gen_ai.response.id`, which need the same engine support the Rust core got in 1.10.0. Recorded in PARITY-STATUS.md as the one remaining Rust-only row, and in th-73c8b5. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LkCz96UfUxcai5RU4LwPnG --- PARITY-STATUS.md | 18 +++++ dotnet/server/src/Telemetry.cs | 40 ++++++++++ dotnet/server/src/TurnRunner.cs | 41 ++++++++-- dotnet/server/tests/TelemetryTests.cs | 20 +++++ go/server/telemetry.go | 28 +++++++ go/server/telemetry_test.go | 28 +++++++ go/server/turn_runner.go | 40 ++++++++-- .../src/smooth_operator_server/telemetry.py | 26 +++++++ .../src/smooth_operator_server/turn_runner.py | 58 +++++++++++---- python/server/tests/test_telemetry.py | 22 ++++++ python/server/uv.lock | 2 +- typescript/server/src/telemetry.ts | 74 ++++++++++++++++++- typescript/server/src/turnRunner.ts | 14 ++-- typescript/server/test/telemetry.test.ts | 24 ++++++ 14 files changed, 397 insertions(+), 38 deletions(-) diff --git a/PARITY-STATUS.md b/PARITY-STATUS.md index d6e90278..8081871c 100644 --- a/PARITY-STATUS.md +++ b/PARITY-STATUS.md @@ -70,6 +70,9 @@ All five servers carry the transport core: frame dispatch, per-turn engine, sess | Shared scenario conformance corpus | ✅ | ✅ | ✅ | ✅ | ✅ | | Postgres conversation store | ✅ | ✅ | ✅ | ✅ | ✅ | | Server `gen_ai.*` OTel telemetry (chat + tool spans · redacted tool args · env-gated OTLP) | ✅ | ✅ | ✅ | ✅ | ✅ | +| └ ingest-joinable spans (`gen_ai.operation.name` · self-identifying tool spans) [^ingest] | ✅ | ✅ | ✅ | ✅ | ✅ | +| └ cost on the span (`gen_ai.usage.cost_usd` / `smooai.gen_ai.cost_unavailable`) | ✅ | ✅ | ✅ | ✅ | ✅ | +| └ usage/cost provenance + `gen_ai.response.id` [^provenance] | ✅ | — | — | — | — | | Second storage backend (DynamoDB + S3 Vectors) | ✅ | — | — | — | — | | Persistent checkpoint / knowledge / ACL-knowledge stores [^knowledge] | ✅ | ✅ | ◐ | ◐ | ◐ | | Deep ingestion + ACL surface | ✅ | ✅ | ◐ | ◐ | ◐ | @@ -77,6 +80,21 @@ All five servers carry the transport core: frame dispatch, per-turn engine, sess | Backplane `publish` (event fan-out) | ✅ | — | ✅ | ✅ | — | | **Cross-pod backplane (Redis / NATS)** | ✅ | — | — | — | — | +[^ingest]: The api-prime OTLP ingest builds a span's attribute set from the resource + attrs plus **that span's own**, with no inheritance from the parent. A tool span + without its own `gen_ai.system` therefore fails the ingest's LLM-event gate and is + **discarded** — Rust's were, for their entire existence (zero rows with + `operation_name = 'tool'`, all time). `gen_ai.operation.name` must be literally + `"chat"` / `"tool"`: the ingest takes the attribute verbatim when present and only + derives it from the span name as a fallback, so any other spelling lands in the + column and matches nothing. + +[^provenance]: Rust-only because it needs engine support that exists only in the Rust + core (1.10.0): `usage_estimated` / `cost_estimated` on `AgentEvent::Completed`, plus + capturing the gateway's `chatcmpl-…` response id. Until the other four cores carry + the same fields, those engines cannot distinguish a measured token count from an + estimated one, and have no join key to `LiteLLM_SpendLogs`. Tracked in th-73c8b5. + [^knowledge]: The durable **knowledge + ACL-knowledge** Postgres stores now ship in **Go, TypeScript and Python** too ([PRs #442, #443, #444](https://github.com/SmooAI/smooth-operator/pulls)) — on the shared `knowledge_vectors` table + pgvector — alongside Rust and .NET (which already had them, e.g. Python's `postgres_knowledge.py`: `PostgresVectorKnowledge` / `PostgresAclKnowledge`). These three cells stay **◐ rather than ✅** for two honest reasons: the persistent **checkpoint** store is still pending in Go/TS/Python, and the **TS + Python** knowledge stores are shipped and contract-tested but **not yet wired into the live dispatcher** (a sync-engine vs async-pg bridge). The **second storage backend** (DynamoDB + S3-Vectors, the row above) remains Rust-only. **The operational consequence:** only the **Rust** server scales past one replica today. Go, TypeScript and Python run an in-memory backplane — correct for a single process, silently wrong the moment you run two pods, because an event published on pod A never reaches a socket held by pod B. C# has no backplane surface at all. diff --git a/dotnet/server/src/Telemetry.cs b/dotnet/server/src/Telemetry.cs index 0faa9b9c..4a58b8ef 100644 --- a/dotnet/server/src/Telemetry.cs +++ b/dotnet/server/src/Telemetry.cs @@ -40,6 +40,46 @@ public static class Telemetry public const string GenAiToolArguments = "gen_ai.tool.call.arguments"; public const string GenAiAgentName = "gen_ai.agent.name"; + /// + /// gen_ai.operation.name — the operation a span represents. + /// + /// The api-prime OTLP ingest takes this attribute VERBATIM when present and only derives it + /// from the span name as a fallback, and its queries filter on operation_name = 'tool'. + /// So the values must be exactly / — + /// a spelling like execute_tool would land in the column and match nothing. + /// + public const string GenAiOperationName = "gen_ai.operation.name"; + + /// + /// gen_ai.usage.cost_usd — the turn's cost in USD. + /// + /// Recorded ONLY when positive. A zero is ambiguous: the gateway answers 0 for a model it has + /// no price for, and local pricing returns the free tier for anything it does not recognise, so + /// a zero means "not measured", never "free". Exporting it would render a paid turn as a + /// confident $0.00. + /// + public const string GenAiUsageCostUsd = "gen_ai.usage.cost_usd"; + + /// smooai.gen_ai.cost_unavailable — why is + /// absent. Set INSTEAD of the cost, never alongside it. Same attribute name and values across + /// every engine so a consumer never special-cases per language. + public const string CostUnavailable = "smooai.gen_ai.cost_unavailable"; + + /// value: no price could be established. + public const string CostUnavailableUnpriced = "unpriced"; + + /// smooai.org_id — the owning org, matching every other engine. + /// ponytail: declared here but never set — the .NET server has no org concept at all + /// (no orgId anywhere in dotnet/server/src), so wiring it is a plumbing change + /// through FrameDispatcher, not a span tag. Set it here once that exists. + public const string SmooaiOrgId = "smooai.org_id"; + + /// value on a span. + public const string OperationChat = "chat"; + + /// value on a span. + public const string OperationTool = "tool"; + /// Span name for the per-turn GenAI chat span (gen_ai.chat). public const string SpanChat = "gen_ai.chat"; diff --git a/dotnet/server/src/TurnRunner.cs b/dotnet/server/src/TurnRunner.cs index 80a35c83..9c7c5ece 100644 --- a/dotnet/server/src/TurnRunner.cs +++ b/dotnet/server/src/TurnRunner.cs @@ -146,13 +146,20 @@ private static string ConfiguredModel() => /// Emit a gen_ai.tool child span (parented to the ambient turn span) for one tool /// call, carrying the tool name and its redacted JSON arguments. No-op when nothing is sampling /// (StartActivity returns null). - private static void EmitToolSpan(FunctionCallContent call) + private static void EmitToolSpan(FunctionCallContent call, string conversationId) { using var toolSpan = Telemetry.Source.StartActivity(Telemetry.SpanTool); if (toolSpan is null) { return; } + // The OTLP ingest builds a span's attributes from the resource attrs plus THAT span's own, + // with no inheritance from the parent — so a child repeats its identifiers or it cannot be + // joined. Omitting gen_ai.system is worse than losing the join: the ingest's LLM-event gate + // keys on it, so bare tool spans are DISCARDED. Rust's were, for their entire existence. + toolSpan.SetTag(Telemetry.GenAiSystem, Telemetry.SystemName); + toolSpan.SetTag(Telemetry.GenAiOperationName, Telemetry.OperationTool); + toolSpan.SetTag(Telemetry.GenAiConversationId, conversationId); toolSpan.SetTag(Telemetry.GenAiToolName, call.Name); var args = call.Arguments is null ? "{}" : JsonSerializer.Serialize(call.Arguments); toolSpan.SetTag(Telemetry.GenAiToolArguments, Telemetry.RedactToolArguments(args)); @@ -226,6 +233,7 @@ public async Task RunAsync(string conversationId, string requestId, // host (env-gated on OTEL_EXPORTER_OTLP_ENDPOINT) or a test's ActivityListener. using var turnActivity = Telemetry.Source.StartActivity(Telemetry.SpanChat); turnActivity?.SetTag(Telemetry.GenAiSystem, Telemetry.SystemName); + turnActivity?.SetTag(Telemetry.GenAiOperationName, Telemetry.OperationChat); turnActivity?.SetTag(Telemetry.GenAiRequestModel, ConfiguredModel()); turnActivity?.SetTag(Telemetry.GenAiConversationId, conversationId); turnActivity?.SetTag(Telemetry.GenAiAgentName, Telemetry.AgentName); @@ -480,7 +488,7 @@ public async Task RunAsync(string conversationId, string requestId, toolNames[call.CallId] = call.Name; // `gen_ai.tool` child span (nests under the turn span), mirroring the Rust // runner emitting one gen_ai.tool span per tool call with redacted args. - EmitToolSpan(call); + EmitToolSpan(call, conversationId); // DEFER a confirmation-gated tool's toolCall chunk: it is emitted from the // gate AFTER write_confirmation_required, so the wire order matches the // canonical (Rust) server. Non-gated tools emit their chunk inline as before. @@ -516,12 +524,31 @@ public async Task RunAsync(string conversationId, string requestId, _interactionPark?.Clear(sessionId); } - // Record token usage on the turn span (omitted when the engine reported none, per the GenAI - // conventions), mirroring the Rust runner's turn_span.record of the usage fields. - if (turnActivity is not null && sawUsage) + // Token counts and cost on the turn span, recording only what was actually measured. + // + // `sawUsage` alone was NOT enough: a usage chunk carrying null counts sets it while both + // totals resolve to 0 via `?? 0`, so this published `input_tokens = 0` on a grounded turn. + // Every other engine guards on the counts themselves. Absent is honest; 0 is a lie. + if (turnActivity is not null) { - turnActivity.SetTag(Telemetry.GenAiUsageInputTokens, promptTokens); - turnActivity.SetTag(Telemetry.GenAiUsageOutputTokens, completionTokens); + if (sawUsage && (promptTokens > 0 || completionTokens > 0)) + { + turnActivity.SetTag(Telemetry.GenAiUsageInputTokens, promptTokens); + turnActivity.SetTag(Telemetry.GenAiUsageOutputTokens, completionTokens); + } + + // Cost is judged separately from the counts: the gateway reports it on an HTTP header + // while usage arrives on an SSE chunk, so either can turn up without the other. A + // non-positive cost becomes an explicit "unpriced" marker, never a confident $0.00. + var costUsd = TurnUsageFrom(agent, sawUsage, promptTokens, completionTokens)?.CostUsd ?? 0; + if (costUsd > 0 && double.IsFinite(costUsd)) + { + turnActivity.SetTag(Telemetry.GenAiUsageCostUsd, costUsd); + } + else + { + turnActivity.SetTag(Telemetry.CostUnavailable, Telemetry.CostUnavailableUnpriced); + } } // 5. Persist the outbound reply. diff --git a/dotnet/server/tests/TelemetryTests.cs b/dotnet/server/tests/TelemetryTests.cs index de6d60d1..39180194 100644 --- a/dotnet/server/tests/TelemetryTests.cs +++ b/dotnet/server/tests/TelemetryTests.cs @@ -121,6 +121,26 @@ public async Task StreamingTurnEmitsGenAiSpansWithModelAndToolArgs() var args = toolSpan.GetTagItem(Telemetry.GenAiToolArguments) as string ?? string.Empty; Assert.Contains("return policy refund window", args); Assert.Equal(chatSpan.Id, toolSpan.ParentId); + + // Being a child is NOT enough. The OTLP ingest builds a span's attributes from + // the resource attrs plus THAT span's own, with no parent inheritance, so the + // tool span repeats the identifiers itself — and without gen_ai.system it fails + // the ingest's LLM-event gate outright and is discarded, which is what happened + // to Rust's tool spans for their entire existence. + Assert.Equal(Telemetry.SystemName, toolSpan.GetTagItem(Telemetry.GenAiSystem)); + Assert.Equal(Telemetry.OperationTool, toolSpan.GetTagItem(Telemetry.GenAiOperationName)); + Assert.Equal(conversationId, toolSpan.GetTagItem(Telemetry.GenAiConversationId)); + + // Must be exactly "chat"/"tool" — the ingest takes the attribute verbatim when + // present and its queries filter on operation_name = 'tool'. + Assert.Equal(Telemetry.OperationChat, chatSpan.GetTagItem(Telemetry.GenAiOperationName)); + + // Cost: exactly one of the two is ever set. This scripted turn is unpriced, so + // the marker must be there INSTEAD of a $0.00 — a missing price must never read + // as free. Before this, .NET was the one engine that shipped a literal + // `new TurnUsage(0, ...)` on the fallback path. + Assert.Null(chatSpan.GetTagItem(Telemetry.GenAiUsageCostUsd)); + Assert.Equal(Telemetry.CostUnavailableUnpriced, chatSpan.GetTagItem(Telemetry.CostUnavailable)); } [Fact] diff --git a/go/server/telemetry.go b/go/server/telemetry.go index f210fe3c..1b2552f8 100644 --- a/go/server/telemetry.go +++ b/go/server/telemetry.go @@ -42,6 +42,34 @@ const ( // SmooaiOrgID is `smooai.org_id` — the owning org. Matches the monorepo TS chat // handler's attribute exactly so the observability studio groups Rust + Go turns by org. SmooaiOrgID = "smooai.org_id" + // GenAIOperationName is `gen_ai.operation.name` — the operation a span represents. + // + // The api-prime OTLP ingest takes this attribute VERBATIM when present and only + // derives it from the span name as a fallback, and its queries filter on + // `operation_name = 'tool'`. So the values must be exactly OperationChat / + // OperationTool — a spelling like "execute_tool" would land in the column and + // match nothing. + GenAIOperationName = "gen_ai.operation.name" + // GenAIUsageCostUSD is `gen_ai.usage.cost_usd` — the turn's cost in USD. + // + // Recorded ONLY when positive. A zero is ambiguous: the gateway answers 0 for a + // model it has no price for, and local pricing returns the free tier for anything + // it does not recognise, so a zero means "not measured", never "free". Exporting + // it would render a paid turn as a confident $0.00. + GenAIUsageCostUSD = "gen_ai.usage.cost_usd" + // CostUnavailable is `smooai.gen_ai.cost_unavailable` — why GenAIUsageCostUSD is + // absent. Set INSTEAD of the cost, never alongside it. Same attribute name and + // values across every engine so a consumer never special-cases per language. + CostUnavailable = "smooai.gen_ai.cost_unavailable" + // CostUnavailableUnpriced is the CostUnavailable value for "no price could be + // established for this model". + CostUnavailableUnpriced = "unpriced" +) + +// OperationChat / OperationTool are the GenAIOperationName values. +const ( + OperationChat = "chat" + OperationTool = "tool" ) // SystemName is emitted for GenAISystem and used as the tracer + service name. diff --git a/go/server/telemetry_test.go b/go/server/telemetry_test.go index f2c5d139..1ebcacbe 100644 --- a/go/server/telemetry_test.go +++ b/go/server/telemetry_test.go @@ -112,6 +112,34 @@ func TestStreamingTurnEmitsGenAISpans(t *testing.T) { t.Errorf("gen_ai.tool span should be a child of gen_ai.chat; parent=%s chat=%s", tool.Parent.SpanID(), chat.SpanContext.SpanID()) } + + // Being a child is NOT enough. The OTLP ingest builds a span's attributes from the + // resource attrs plus THAT span's own, with no parent inheritance, so the tool span + // repeats the identifiers itself — and without gen_ai.system it fails the ingest's + // LLM-event gate outright and is discarded, which is what happened to Rust's tool + // spans for their entire existence (zero rows with operation_name='tool', all time). + assertAttr(t, tool.Attributes, GenAISystem, SystemName) + assertAttr(t, tool.Attributes, GenAIOperationName, OperationTool) + assertAttr(t, tool.Attributes, GenAIConversationID, session.ConversationID) + assertAttr(t, tool.Attributes, SmooaiOrgID, "org-telemetry") + + // Must be exactly "chat"/"tool" — the ingest takes the attribute verbatim when + // present and its queries filter on operation_name = 'tool'. + assertAttr(t, chat.Attributes, GenAIOperationName, OperationChat) + + // Cost: exactly one of the two is ever set. The mock turn IS priced (local + // ModelPricing knows openai/gpt-4o), so the cost lands and the marker must not — + // a zero must never be exported as a real cost, and a real cost must never carry + // an "unavailable" marker beside it. + cost, hasCost := attr(chat.Attributes, GenAIUsageCostUSD) + if !hasCost { + t.Errorf("a priced turn must record %s; got attrs %+v", GenAIUsageCostUSD, chat.Attributes) + } else if cost == "0" || cost == "0.000000" { + t.Errorf("%s must never be exported as zero — that means unpriced, not free", GenAIUsageCostUSD) + } + if _, ok := attr(chat.Attributes, CostUnavailable); ok { + t.Errorf("%s must not be set alongside a real cost", CostUnavailable) + } } func assertAttr(t *testing.T, kvs []attribute.KeyValue, key, want string) { diff --git a/go/server/turn_runner.go b/go/server/turn_runner.go index 4d1d0e0d..bbd3a535 100644 --- a/go/server/turn_runner.go +++ b/go/server/turn_runner.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "math" "os" "strings" "sync/atomic" @@ -200,6 +201,7 @@ func (r *TurnRunner) Run(ctx context.Context, sessionID, conversationID, request tr := otel.Tracer(SystemName) ctx, turnSpan := tr.Start(ctx, SpanChat, oteltrace.WithAttributes( attribute.String(GenAISystem, SystemName), + attribute.String(GenAIOperationName, OperationChat), attribute.String(GenAIRequestModel, r.spanModel()), attribute.String(GenAIConversationID, conversationID), attribute.String(GenAIAgentName, AgentName), @@ -444,19 +446,43 @@ consume: // GenAI conventions) and emit one `gen_ai.tool` child span per tool call — carrying // the redacted arguments, measured latency, and an ERROR status on failure. Mirrors the // Rust runner's post-turn span emission. - if usage != nil && (usage.PromptTokens > 0 || usage.CompletionTokens > 0) { - turnSpan.SetAttributes( - attribute.Int(GenAIUsageInputTokens, usage.PromptTokens), - attribute.Int(GenAIUsageOutputTokens, usage.CompletionTokens), - ) + if usage != nil { + // Counts are omitted when the engine reported none: absent is honest, 0 is a + // lie (a grounded turn always consumes prompt tokens). + if usage.PromptTokens > 0 || usage.CompletionTokens > 0 { + turnSpan.SetAttributes( + attribute.Int(GenAIUsageInputTokens, usage.PromptTokens), + attribute.Int(GenAIUsageOutputTokens, usage.CompletionTokens), + ) + } + // Cost is judged separately from the counts: the gateway reports it on an HTTP + // header while usage arrives on an SSE chunk, so either can turn up without the + // other. A non-positive cost becomes an explicit "unpriced" marker, never $0.00. + if usage.CostUSD > 0 && !math.IsInf(usage.CostUSD, 0) && !math.IsNaN(usage.CostUSD) { + turnSpan.SetAttributes(attribute.Float64(GenAIUsageCostUSD, usage.CostUSD)) + } else { + turnSpan.SetAttributes(attribute.String(CostUnavailable, CostUnavailableUnpriced)) + } } for _, rec := range toolRecords { - _, toolSpan := tr.Start(ctx, SpanTool, oteltrace.WithAttributes( + // The OTLP ingest builds a span's attributes from the resource attrs plus THAT + // span's own, with no inheritance from the parent — so a child repeats its + // identifiers or it cannot be joined. Omitting gen_ai.system is worse than + // losing the join: the ingest's LLM-event gate keys on it, so bare tool spans + // are DISCARDED. Rust's were, for their entire existence. + toolAttrs := []attribute.KeyValue{ + attribute.String(GenAISystem, SystemName), + attribute.String(GenAIOperationName, OperationTool), + attribute.String(GenAIConversationID, conversationID), attribute.String(GenAIToolName, rec.name), attribute.String(GenAIToolArguments, redactToolArguments(rec.arguments)), attribute.Int64("duration_ms", rec.durationMs), attribute.Bool("is_error", rec.isError), - )) + } + if r.orgID != "" { + toolAttrs = append(toolAttrs, attribute.String(SmooaiOrgID, r.orgID)) + } + _, toolSpan := tr.Start(ctx, SpanTool, oteltrace.WithAttributes(toolAttrs...)) if rec.isError { toolSpan.SetStatus(codes.Error, rec.errText) } diff --git a/python/server/src/smooth_operator_server/telemetry.py b/python/server/src/smooth_operator_server/telemetry.py index c3ac9fc8..dee941a4 100644 --- a/python/server/src/smooth_operator_server/telemetry.py +++ b/python/server/src/smooth_operator_server/telemetry.py @@ -56,6 +56,27 @@ #: ``smooai.org_id`` — the owning org. Matches the monorepo TS chat handler so the #: observability studio groups Python + Rust + TS turns by org. SMOOAI_ORG_ID = "smooai.org_id" +#: ``gen_ai.operation.name`` — the operation a span represents. +#: +#: The api-prime OTLP ingest takes this attribute VERBATIM when present and only +#: derives it from the span name as a fallback, and its queries filter on +#: ``operation_name = 'tool'``. So the values must be exactly :data:`OPERATION_CHAT` +#: / :data:`OPERATION_TOOL` — a spelling like ``execute_tool`` would land in the +#: column and match nothing. +GEN_AI_OPERATION_NAME = "gen_ai.operation.name" +#: ``gen_ai.usage.cost_usd`` — the turn's cost in USD. +#: +#: Recorded ONLY when positive. A zero is ambiguous: the gateway answers 0 for a +#: model it has no price for, and local pricing returns the free tier for anything +#: it does not recognise, so a zero means "not measured", never "free". Exporting +#: it would render a paid turn as a confident $0.00. +GEN_AI_USAGE_COST_USD = "gen_ai.usage.cost_usd" +#: ``smooai.gen_ai.cost_unavailable`` — why :data:`GEN_AI_USAGE_COST_USD` is absent. +#: Set INSTEAD of the cost, never alongside it. Same attribute name and values +#: across every engine so a consumer never special-cases per language. +COST_UNAVAILABLE = "smooai.gen_ai.cost_unavailable" +#: :data:`COST_UNAVAILABLE` value: no price could be established for the model. +COST_UNAVAILABLE_UNPRICED = "unpriced" #: The value emitted for :data:`GEN_AI_SYSTEM` — identifies these traces. SYSTEM_NAME = "smooth-operator" @@ -68,6 +89,11 @@ #: Span name for a per-tool-call child span (``gen_ai.tool``). SPAN_TOOL = "gen_ai.tool" +#: :data:`GEN_AI_OPERATION_NAME` value on a :data:`SPAN_CHAT` span. +OPERATION_CHAT = "chat" +#: :data:`GEN_AI_OPERATION_NAME` value on a :data:`SPAN_TOOL` span. +OPERATION_TOOL = "tool" + #: Env var that switches :func:`init_telemetry` to a real OTLP exporter. OTLP_ENDPOINT_ENV = "OTEL_EXPORTER_OTLP_ENDPOINT" diff --git a/python/server/src/smooth_operator_server/turn_runner.py b/python/server/src/smooth_operator_server/turn_runner.py index c072a863..a87f8a37 100644 --- a/python/server/src/smooth_operator_server/turn_runner.py +++ b/python/server/src/smooth_operator_server/turn_runner.py @@ -16,6 +16,7 @@ import contextlib import json import logging +import math import os from dataclasses import dataclass, field, fields from typing import Any, Callable @@ -498,6 +499,7 @@ async def _gate(req: HumanApprovalRequest) -> HumanApprovalResponse: # (zero cost) until `init_telemetry` installs a provider. span_attrs: dict[str, Any] = { telemetry.GEN_AI_SYSTEM: telemetry.SYSTEM_NAME, + telemetry.GEN_AI_OPERATION_NAME: telemetry.OPERATION_CHAT, telemetry.GEN_AI_REQUEST_MODEL: self._model or DEFAULT_MODEL, telemetry.GEN_AI_CONVERSATION_ID: conversation_id, telemetry.GEN_AI_AGENT_NAME: telemetry.AGENT_NAME, @@ -526,7 +528,7 @@ async def _gate(req: HumanApprovalRequest) -> HumanApprovalResponse: # name + redacted arguments (mirrors the Rust runner's per-tool # child span). Emitted for gated tools too — the span is # independent of the deferred wire chunk below. - _emit_tool_span(event) + _emit_tool_span(event, conversation_id, self._org_id) # DEFER a confirmation-gated tool's toolCall chunk: it is emitted # from the gate AFTER `write_confirmation_required`, so the wire # order matches the reference (Rust) server. Non-gated tools emit @@ -545,13 +547,12 @@ async def _gate(req: HumanApprovalRequest) -> HumanApprovalResponse: "promptTokens": event.response.usage.prompt_tokens, "completionTokens": event.response.usage.completion_tokens, } - # Record token usage on the turn span (omitted when the engine - # reported none, per the GenAI conventions). - prompt = event.response.usage.prompt_tokens - completion = event.response.usage.completion_tokens - if prompt or completion: - turn_span.set_attribute(telemetry.GEN_AI_USAGE_INPUT_TOKENS, prompt) - turn_span.set_attribute(telemetry.GEN_AI_USAGE_OUTPUT_TOKENS, completion) + _record_turn_usage( + turn_span, + event.response.usage.prompt_tokens, + event.response.usage.completion_tokens, + event.response.cost_usd, + ) finally: # Turn over: the preamble window is closed for good. Cancel and reap the # task so a still-in-flight preamble can neither emit late nor linger as @@ -750,18 +751,47 @@ def _truncate(value: str, max_chars: int) -> str: return value if len(value) <= max_chars else value[:max_chars] -def _emit_tool_span(event: ToolCallEvent) -> None: +def _record_turn_usage(turn_span: Any, prompt: int, completion: int, cost_usd: float) -> None: + """Record the turn's token counts and cost — only the parts actually measured. + + Counts are omitted when the engine reported none: absent is honest, ``0`` is a lie + (a grounded turn always consumes prompt tokens). Cost is judged separately, because + the gateway reports it on an HTTP header while usage arrives on an SSE chunk — either + can turn up without the other. A non-positive cost becomes an explicit ``unpriced`` + marker rather than a confident ``$0.00``.""" + if prompt or completion: + turn_span.set_attribute(telemetry.GEN_AI_USAGE_INPUT_TOKENS, prompt) + turn_span.set_attribute(telemetry.GEN_AI_USAGE_OUTPUT_TOKENS, completion) + if cost_usd > 0 and math.isfinite(cost_usd): + turn_span.set_attribute(telemetry.GEN_AI_USAGE_COST_USD, cost_usd) + else: + turn_span.set_attribute(telemetry.COST_UNAVAILABLE, telemetry.COST_UNAVAILABLE_UNPRICED) + + +def _emit_tool_span(event: ToolCallEvent, conversation_id: str, org_id: str | None) -> None: """Emit a ``gen_ai.tool`` child span for a tool call, carrying the tool name and the redacted JSON arguments (mirrors the Rust runner's per-tool child span). The span opens as a child of the current ``gen_ai.chat`` turn span and closes immediately — a marker, not a duration measured around execution (the engine owns - tool execution). No-op cost until a tracer provider is installed.""" + tool execution). No-op cost until a tracer provider is installed. + + It repeats the turn's identifiers rather than relying on the parent: the OTLP ingest + builds a span's attributes from the resource attrs plus THAT span's own, with no + parent inheritance. Omitting ``gen_ai.system`` is worse than losing the join — the + ingest's LLM-event gate keys on it, so bare tool spans are DISCARDED. Rust's were, + for their entire existence.""" + attributes: dict[str, Any] = { + telemetry.GEN_AI_SYSTEM: telemetry.SYSTEM_NAME, + telemetry.GEN_AI_OPERATION_NAME: telemetry.OPERATION_TOOL, + telemetry.GEN_AI_CONVERSATION_ID: conversation_id, + telemetry.GEN_AI_TOOL_NAME: event.name, + telemetry.GEN_AI_TOOL_ARGUMENTS: telemetry.redact_tool_arguments(event.arguments or ""), + } + if org_id: + attributes[telemetry.SMOOAI_ORG_ID] = org_id with telemetry.tracer().start_as_current_span( telemetry.SPAN_TOOL, - attributes={ - telemetry.GEN_AI_TOOL_NAME: event.name, - telemetry.GEN_AI_TOOL_ARGUMENTS: telemetry.redact_tool_arguments(event.arguments or ""), - }, + attributes=attributes, ): pass diff --git a/python/server/tests/test_telemetry.py b/python/server/tests/test_telemetry.py index b2057247..0e7599ad 100644 --- a/python/server/tests/test_telemetry.py +++ b/python/server/tests/test_telemetry.py @@ -92,6 +92,28 @@ async def test_turn_emits_gen_ai_spans_with_org_and_tool_args(span_exporter: InM assert "return policy refund window" in args, f"tool args should carry the query; got {args!r}" assert tool.parent is not None and tool.parent.span_id == chat.context.span_id + # Being a child is NOT enough. The OTLP ingest builds a span's attributes from the + # resource attrs plus THAT span's own, with no parent inheritance, so the tool span + # repeats the identifiers itself — and without gen_ai.system it fails the ingest's + # LLM-event gate outright and is discarded, which is what happened to Rust's tool + # spans for their entire existence (zero rows with operation_name='tool', all time). + assert tool.attributes[telemetry.GEN_AI_SYSTEM] == telemetry.SYSTEM_NAME + assert tool.attributes[telemetry.GEN_AI_OPERATION_NAME] == telemetry.OPERATION_TOOL + assert tool.attributes[telemetry.GEN_AI_CONVERSATION_ID] == "conv-otel-srv" + assert tool.attributes[telemetry.SMOOAI_ORG_ID] == "org-telemetry" + + # Must be exactly "chat"/"tool" — the ingest takes the attribute verbatim when + # present and its queries filter on operation_name = 'tool'. + assert chat.attributes[telemetry.GEN_AI_OPERATION_NAME] == telemetry.OPERATION_CHAT + + # Cost: exactly one of the two is ever set, and a zero is never exported as a real + # cost (it means "unpriced", not "free"). + if telemetry.GEN_AI_USAGE_COST_USD in chat.attributes: + assert chat.attributes[telemetry.GEN_AI_USAGE_COST_USD] > 0 + assert telemetry.COST_UNAVAILABLE not in chat.attributes + else: + assert chat.attributes[telemetry.COST_UNAVAILABLE] == telemetry.COST_UNAVAILABLE_UNPRICED + def test_redact_tool_arguments_scrubs_secret_named_keys() -> None: out = telemetry.redact_tool_arguments('{"query":"weather","api_key":"sk-live-123"}') diff --git a/python/server/uv.lock b/python/server/uv.lock index 6ee689b6..7a86a627 100644 --- a/python/server/uv.lock +++ b/python/server/uv.lock @@ -846,7 +846,7 @@ wheels = [ [[package]] name = "smooai-smooth-operator-server" -version = "1.52.3" +version = "1.54.1" source = { editable = "." } dependencies = [ { name = "opentelemetry-api" }, diff --git a/typescript/server/src/telemetry.ts b/typescript/server/src/telemetry.ts index e6d1c133..14ee9169 100644 --- a/typescript/server/src/telemetry.ts +++ b/typescript/server/src/telemetry.ts @@ -26,6 +26,33 @@ export const GEN_AI_TOOL_NAME = 'gen_ai.tool.name'; export const GEN_AI_TOOL_ARGUMENTS = 'gen_ai.tool.call.arguments'; export const GEN_AI_AGENT_NAME = 'gen_ai.agent.name'; export const SMOOAI_ORG_ID = 'smooai.org_id'; +/** + * `gen_ai.operation.name` — the operation a span represents. + * + * The api-prime OTLP ingest takes this attribute VERBATIM when present and only + * derives it from the span name as a fallback, and its queries filter on + * `operation_name = 'tool'`. So the values must be exactly {@link OPERATION_CHAT} + * / {@link OPERATION_TOOL} — a spelling like `execute_tool` would land in the + * column and match nothing. + */ +export const GEN_AI_OPERATION_NAME = 'gen_ai.operation.name'; +/** + * `gen_ai.usage.cost_usd` — the turn's cost in USD. + * + * Recorded ONLY when positive. A zero is ambiguous: the gateway answers `0` for + * a model it has no price for, and local pricing returns the free tier for + * anything it doesn't recognise, so a zero means "not measured", never "free". + * Exporting it would render a paid turn as a confident $0.00. + */ +export const GEN_AI_USAGE_COST_USD = 'gen_ai.usage.cost_usd'; +/** + * `smooai.gen_ai.cost_unavailable` — why {@link GEN_AI_USAGE_COST_USD} is absent. + * Set INSTEAD of the cost, never alongside it. Same attribute name and values + * across every engine so a consumer never special-cases per language. + */ +export const COST_UNAVAILABLE = 'smooai.gen_ai.cost_unavailable'; +/** {@link COST_UNAVAILABLE} value: no price could be established for the model. */ +export const COST_UNAVAILABLE_UNPRICED = 'unpriced'; /** `gen_ai.system` value identifying the polyglot operator to the studio. */ export const SYSTEM_NAME = 'smooth-operator'; @@ -37,6 +64,11 @@ export const SPAN_CHAT = 'gen_ai.chat'; /** Span name for a per-tool-call child span (`gen_ai.tool`). */ export const SPAN_TOOL = 'gen_ai.tool'; +/** {@link GEN_AI_OPERATION_NAME} value on a {@link SPAN_CHAT} span. */ +export const OPERATION_CHAT = 'chat'; +/** {@link GEN_AI_OPERATION_NAME} value on a {@link SPAN_TOOL} span. */ +export const OPERATION_TOOL = 'tool'; + /** Instrumentation-scope name the tracer is fetched under. */ export const TRACER_NAME = SYSTEM_NAME; @@ -108,6 +140,7 @@ export function getTracer(): Tracer { export function startTurnSpan(model: string, conversationId: string, orgId: string | undefined): Span { const attributes: Record = { [GEN_AI_SYSTEM]: SYSTEM_NAME, + [GEN_AI_OPERATION_NAME]: OPERATION_CHAT, [GEN_AI_REQUEST_MODEL]: model, [GEN_AI_CONVERSATION_ID]: conversationId, [GEN_AI_AGENT_NAME]: AGENT_NAME, @@ -116,16 +149,55 @@ export function startTurnSpan(model: string, conversationId: string, orgId: stri return getTracer().startSpan(SPAN_CHAT, { attributes }); } +/** + * Record the turn's token counts and cost on the turn span — only the parts that + * were actually measured. + * + * Counts are omitted entirely when the engine reported none: absent is honest, + * `0` is a lie (a grounded turn always consumes prompt tokens). Cost is judged + * separately, because the gateway reports it on an HTTP header while usage comes + * on an SSE chunk — either can arrive without the other. A non-positive cost + * becomes {@link COST_UNAVAILABLE} rather than a `$0.00`. + */ +export function recordTurnUsage(turnSpan: Span, usage: { promptTokens: number; completionTokens: number; costUsd: number } | undefined): void { + if (!usage) return; + if (usage.promptTokens > 0 || usage.completionTokens > 0) { + turnSpan.setAttribute(GEN_AI_USAGE_INPUT_TOKENS, usage.promptTokens); + turnSpan.setAttribute(GEN_AI_USAGE_OUTPUT_TOKENS, usage.completionTokens); + } + if (usage.costUsd > 0 && Number.isFinite(usage.costUsd)) { + turnSpan.setAttribute(GEN_AI_USAGE_COST_USD, usage.costUsd); + } else { + turnSpan.setAttribute(COST_UNAVAILABLE, COST_UNAVAILABLE_UNPRICED); + } +} + /** * Emit a child `gen_ai.tool` span for one tool call under `turnSpan`, carrying the * tool name and redacted arguments. `durationMs`, when known, is recorded too. */ -export function recordToolSpan(turnSpan: Span, toolName: string, argumentsJson: string, durationMs?: number): void { +export function recordToolSpan( + turnSpan: Span, + toolName: string, + argumentsJson: string, + conversationId: string, + orgId?: string, + durationMs?: number, +): void { const ctx = trace.setSpan(context.active(), turnSpan); + // The OTLP ingest builds a span's attributes from the resource attrs plus + // THAT span's own, with no inheritance from the parent — so a child repeats + // its identifiers or it cannot be joined. Omitting `gen_ai.system` is worse + // than losing the join: the ingest's LLM-event gate keys on it, so bare tool + // spans are DISCARDED. Rust's were, for their entire existence. const attributes: Record = { + [GEN_AI_SYSTEM]: SYSTEM_NAME, + [GEN_AI_OPERATION_NAME]: OPERATION_TOOL, + [GEN_AI_CONVERSATION_ID]: conversationId, [GEN_AI_TOOL_NAME]: toolName, [GEN_AI_TOOL_ARGUMENTS]: redactToolArguments(argumentsJson), }; + if (orgId) attributes[SMOOAI_ORG_ID] = orgId; if (durationMs !== undefined) attributes.duration_ms = durationMs; getTracer().startSpan(SPAN_TOOL, { attributes }, ctx).end(); } diff --git a/typescript/server/src/turnRunner.ts b/typescript/server/src/turnRunner.ts index ade45e50..59f2c8bb 100644 --- a/typescript/server/src/turnRunner.ts +++ b/typescript/server/src/turnRunner.ts @@ -20,7 +20,7 @@ import type { ModelCeilingResolver } from './modelCeiling.js'; import * as protocol from './protocol.js'; import type { Citation, Frame, TurnUsage } from './protocol.js'; import type { SessionStore } from './sessionStore.js'; -import { GEN_AI_USAGE_INPUT_TOKENS, GEN_AI_USAGE_OUTPUT_TOKENS, recordToolSpan, startTurnSpan } from './telemetry.js'; +import { recordToolSpan, recordTurnUsage, startTurnSpan } from './telemetry.js'; import { withUserImages, type UserImage } from './toolContext.js'; import { advanceStep, judgeStep, resolveCurrentStep, type ConversationWorkflow } from './workflow.js'; @@ -425,7 +425,7 @@ export class TurnRunner { // Emit a `gen_ai.tool` child span for every tool call the engine makes — // BEFORE the gated `continue` below, so a confirmation-gated tool is // traced too (parity with the Rust runner collecting all tool records). - if (event.type === 'tool_call') recordToolSpan(turnSpan, event.name, event.arguments); + if (event.type === 'tool_call') recordToolSpan(turnSpan, event.name, event.arguments, conversationId, this.orgId); // DEFER a confirmation-gated tool's toolCall chunk: it is emitted from // the gate AFTER `write_confirmation_required`, so the wire order matches // the reference (Rust) server. Non-gated tools emit their chunk inline. @@ -445,12 +445,10 @@ export class TurnRunner { }; } } - // Record token usage on the turn span, omitting a turn that reported none - // (per the GenAI conventions), matching the Rust runner. - if (usage && (usage.promptTokens > 0 || usage.completionTokens > 0)) { - turnSpan.setAttribute(GEN_AI_USAGE_INPUT_TOKENS, usage.promptTokens); - turnSpan.setAttribute(GEN_AI_USAGE_OUTPUT_TOKENS, usage.completionTokens); - } + // Token counts (omitted when the engine reported none) plus cost and, + // when there is none, an explicit "unpriced" marker. One helper owns + // that policy so it can't drift from the other engines. + recordTurnUsage(turnSpan, usage); } finally { turnSpan.end(); // Turn over: drop any lingering pending confirmation so a stale entry can't diff --git a/typescript/server/test/telemetry.test.ts b/typescript/server/test/telemetry.test.ts index fdb575e1..01c29c42 100644 --- a/typescript/server/test/telemetry.test.ts +++ b/typescript/server/test/telemetry.test.ts @@ -64,6 +64,30 @@ describe('TurnRunner GenAI OTel spans', () => { // The tool span is a child of the turn span (same trace). expect(toolSpan!.spanContext().traceId).toBe(chatSpan!.spanContext().traceId); expect(toolSpan!.parentSpanContext?.spanId).toBe(chatSpan!.spanContext().spanId); + + // (3) Being a child is NOT enough. The OTLP ingest builds a span's attributes + // from the resource attrs plus THAT span's own, with no parent inheritance, so + // the tool span repeats the identifiers itself — and without gen_ai.system it + // fails the ingest's LLM-event gate outright and is discarded, which is what + // happened to Rust's tool spans for their entire existence. + expect(attr(toolSpan!, 'gen_ai.system')).toBe('smooth-operator'); + expect(attr(toolSpan!, 'gen_ai.operation.name')).toBe('tool'); + expect(attr(toolSpan!, 'gen_ai.conversation.id')).toBe(session.conversationId); + expect(attr(toolSpan!, 'smooai.org_id')).toBe('org-telemetry'); + + // Must be exactly 'chat'/'tool' — the ingest takes the attribute verbatim when + // present and its queries filter on operation_name = 'tool'. + expect(attr(chatSpan!, 'gen_ai.operation.name')).toBe('chat'); + + // (4) Cost: exactly one of the two is ever set, and a zero is never exported as + // a real cost (it means "unpriced", not "free"). + const cost = attr(chatSpan!, 'gen_ai.usage.cost_usd'); + if (cost === undefined) { + expect(attr(chatSpan!, 'smooai.gen_ai.cost_unavailable')).toBe('unpriced'); + } else { + expect(Number(cost)).toBeGreaterThan(0); + expect(attr(chatSpan!, 'smooai.gen_ai.cost_unavailable')).toBeUndefined(); + } }); });