feat(ai-assistants): align telemetry with OTel GenAI semantic conventions#1625
feat(ai-assistants): align telemetry with OTel GenAI semantic conventions#1625nicacioliveira wants to merge 1 commit into
Conversation
…ions Centralize the assistant's OTel instruments in observability.ts and rename them to gen_ai.* semconv (official @opentelemetry/semantic-conventions constants): - assistant_latency (ms) -> gen_ai.client.operation.duration (seconds), with gen_ai.system=openai and a deco.assistant.phase dimension. - assistant_image_prompt_tokens + assistant_image_completion_tokens -> a single gen_ai.client.token.usage histogram split by gen_ai.token.type (input/output). - assistant_transcribe_audio_size -> deco.assistant.transcribe.audio_duration (it measures duration, not size). - The three per-action error counters -> one deco.assistant.errors counter dimensioned by deco.assistant.operation. - Fix the assistantId/assistant_id attribute inconsistency (now assistant_id everywhere). Clean rename — no dual-emit (ClickStack is a fresh backend). The 2 pre-existing type errors in website/utils/crypto.ts are unrelated (present on main). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Tagging OptionsShould a new tag be published when this PR is merged?
|
📝 WalkthroughWalkthroughA shared observability module was added for assistant telemetry. Upload, image description, transcription, and chat message processing now record metrics through shared instruments and standardized attributes. ChangesAssistant observability rewrite
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
ai-assistants/actions/describeImage.ts (1)
68-77: 📐 Maintainability & Code Quality | 🔵 TrivialAdd
gen_ai.operation.nameto token usage metrics.Per OpenTelemetry GenAI semantic conventions, the
gen_ai.client.token.usagemetric strictly requires thegen_ai.operation.nameattribute. The current implementation omits this, which results in non-compliant telemetry.Update the attributes object for both input and output token records to include
"gen_ai.operation.name": "chat".Actionable Context
stats.tokenUsage.record(response.usage?.prompt_tokens ?? 0, { [GEN_AI_SYSTEM]: GEN_AI_SYSTEM_OPENAI, + "gen_ai.operation.name": "chat", [GEN_AI_TOKEN_TYPE]: GEN_AI_TOKEN_TYPE_INPUT, [ATTR_ASSISTANT_ID]: assistantId, }); stats.tokenUsage.record(response.usage?.completion_tokens ?? 0, { [GEN_AI_SYSTEM]: GEN_AI_SYSTEM_OPENAI, + "gen_ai.operation.name": "chat", [GEN_AI_TOKEN_TYPE]: GEN_AI_TOKEN_TYPE_OUTPUT, [ATTR_ASSISTANT_ID]: assistantId, });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ai-assistants/actions/describeImage.ts` around lines 68 - 77, The token usage metrics recorded in describeImage’s stats.tokenUsage.record calls are missing the required gen_ai.operation.name attribute. Update the attributes object used for both the input and output token records in describeImage so they include gen_ai.operation.name set to chat alongside the existing GEN_AI_SYSTEM, GEN_AI_TOKEN_TYPE, and ATTR_ASSISTANT_ID fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ai-assistants/actions/awsUploadImage.ts`:
- Around line 51-54: The error metric in awsUploadImage’s stats.errors.add call
can lose the assistant_id dimension when assistantId is undefined because the
OTel attributes object drops missing values. Update the assistant_id attribute
to always be set in this path by defaulting assistantIds?.assistantId to a
sentinel such as "unknown", and apply the same fix to the matching
stats.errors.add usage in describeImage and transcribeAudio so the metric
remains consistently dimensioned.
In `@ai-assistants/actions/transcribeAudio.ts`:
- Around line 36-38: The `transcribeAudio` metric recording is using
`transcribeAudioProps.audioDuration` directly even though `stats.audioDuration`
is configured in seconds; update the `transcribeAudio` flow to ensure the value
passed into `stats.audioDuration.record` is normalized to seconds, and verify
the callers of `transcribeAudioProps.audioDuration` so `assistantId` metrics
remain accurate regardless of whether the source duration is currently provided
in milliseconds or seconds.
---
Nitpick comments:
In `@ai-assistants/actions/describeImage.ts`:
- Around line 68-77: The token usage metrics recorded in describeImage’s
stats.tokenUsage.record calls are missing the required gen_ai.operation.name
attribute. Update the attributes object used for both the input and output token
records in describeImage so they include gen_ai.operation.name set to chat
alongside the existing GEN_AI_SYSTEM, GEN_AI_TOKEN_TYPE, and ATTR_ASSISTANT_ID
fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07025032-efd0-4ed0-a969-7ebe2eee8fae
📒 Files selected for processing (5)
ai-assistants/actions/awsUploadImage.tsai-assistants/actions/describeImage.tsai-assistants/actions/transcribeAudio.tsai-assistants/chat/messages.tsai-assistants/observability.ts
| stats.errors.add(1, { | ||
| [ATTR_ASSISTANT_OPERATION]: "aws_upload", | ||
| [ATTR_ASSISTANT_ID]: assistantId, | ||
| }); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
assistant_id may be undefined and silently dropped.
assistantId derives from optional assistantIds?.assistantId, so when absent the OTel SDK omits the assistant_id attribute entirely, leaving error metrics undimensioned. Consider defaulting to a sentinel (e.g. "unknown") so the dimension is always present. The same pattern recurs in describeImage.ts and transcribeAudio.ts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ai-assistants/actions/awsUploadImage.ts` around lines 51 - 54, The error
metric in awsUploadImage’s stats.errors.add call can lose the assistant_id
dimension when assistantId is undefined because the OTel attributes object drops
missing values. Update the assistant_id attribute to always be set in this path
by defaulting assistantIds?.assistantId to a sentinel such as "unknown", and
apply the same fix to the matching stats.errors.add usage in describeImage and
transcribeAudio so the metric remains consistently dimensioned.
| stats.audioDuration.record(transcribeAudioProps.audioDuration, { | ||
| [ATTR_ASSISTANT_ID]: assistantId, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find callers that set audioDuration on transcribeAudio props
rg -nP --type=ts -C3 '\baudioDuration\b'Repository: deco-cx/apps
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Search for audioDuration across all files to find usage and declaration
rg -n "audioDuration"Repository: deco-cx/apps
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Find the function definition in transcribeAudio.ts to check the metric unit
cat -n ai-assistants/actions/transcribeAudio.tsRepository: deco-cx/apps
Length of output: 2046
🏁 Script executed:
#!/bin/bash
# Find where transcribeAudio is called to inspect the passed argument
rg -n "transcribeAudio\("Repository: deco-cx/apps
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Find usages of transcribeAudio function call across all files
rg "transcribeAudio"Repository: deco-cx/apps
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Find imports of the transcribeAudio module
rg -n "from ['\"].*transcribeAudio['\"]"Repository: deco-cx/apps
Length of output: 150
🏁 Script executed:
#!/bin/bash
# List directory structure to ensure correct path and find potential callers
find . -name "*transcribe*" -o -path "*/ai-assistants/*" -type f | head -n 20Repository: deco-cx/apps
Length of output: 747
🏁 Script executed:
#!/bin/bash
# Use grep instead of rg to find the text "transcribeAudio"
grep -rn "transcribeAudio" --include="*.ts" --include="*.tsx" .Repository: deco-cx/apps
Length of output: 1903
🏁 Script executed:
#!/bin/bash
# Check the callers to verify how audioDuration is passed
cat ./ai-assistants/hooks/useFileUpload.tsRepository: deco-cx/apps
Length of output: 451
🏁 Script executed:
#!/bin/bash
# Inspect the observability definition to confirm the unit
cat -n ./ai-assistants/observability.tsRepository: deco-cx/apps
Length of output: 2687
🏁 Script executed:
#!/bin/bash
# Search for where audioDuration is calculated or assigned before being passed
grep -rn "audioDuration:" --include="*.ts" --include="*.tsx" . | head -n 20Repository: deco-cx/apps
Length of output: 297
🏁 Script executed:
#!/bin/bash
# Double check if there is any client-side calculation for audio duration that might use milliseconds
grep -rn "audio.*duration" --include="*.ts" --include="*.tsx" . | grep -v "transcribeAudio.ts" | grep -v "observability.ts"Repository: deco-cx/apps
Length of output: 298
🏁 Script executed:
#!/bin/bash
# Check the type definition to see if there's any JSDoc or comment about units
grep -A5 -B5 "audioDuration" ./ai-assistants/actions/transcribeAudio.tsRepository: deco-cx/apps
Length of output: 848
Ensure audioDuration is in seconds.
The audioDuration histogram is explicitly configured with unit s. The current implementation records transcribeAudioProps.audioDuration directly without conversion. Verify that all callers provide this value in seconds (not milliseconds) to ensure metric accuracy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ai-assistants/actions/transcribeAudio.ts` around lines 36 - 38, The
`transcribeAudio` metric recording is using `transcribeAudioProps.audioDuration`
directly even though `stats.audioDuration` is configured in seconds; update the
`transcribeAudio` flow to ensure the value passed into
`stats.audioDuration.record` is normalized to seconds, and verify the callers of
`transcribeAudioProps.audioDuration` so `assistantId` metrics remain accurate
regardless of whether the source duration is currently provided in milliseconds
or seconds.
There was a problem hiding this comment.
2 issues found across 5 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ai-assistants/observability.ts">
<violation number="1" location="ai-assistants/observability.ts:6">
P2: Uses deprecated `ATTR_GEN_AI_SYSTEM` (`gen_ai.system`) instead of `ATTR_GEN_AI_PROVIDER_NAME` (`gen_ai.provider.name`). The semconv v1.37.0 package moved `gen_ai.system` → `gen_ai.provider.name` and exports the value constant `GEN_AI_PROVIDER_NAME_VALUE_OPENAI`. This means recorded telemetry will carry a deprecated attribute key rather than aligning with current GenAI conventions as the PR title states.</violation>
</file>
<file name="ai-assistants/actions/awsUploadImage.ts">
<violation number="1" location="ai-assistants/actions/awsUploadImage.ts:53">
P2: `assistantId` can be `undefined` (from optional `assistantIds?.assistantId`), and the OTel SDK will silently drop the attribute when the value is `undefined`. This leaves error metrics undimensioned by `assistant_id`. Consider defaulting to a sentinel like `"unknown"` to ensure the dimension is always present:
```ts
[ATTR_ASSISTANT_ID]: assistantId ?? "unknown",
The same pattern applies in describeImage.ts and transcribeAudio.ts.
</details>
<sub>Reply with feedback, questions, or to request a fix.<br /><br />[Re-trigger cubic](https://www.cubic.dev/action/re-review/pr/deco-cx/apps/1625/ai_pr_review_1782407344173_6bfc73af-525d-4012-899e-7e478aeeb793?returnTo=https%3A%2F%2Fgithub.com%2Fdeco-cx%2Fapps%2Fpull%2F1625)</sub>
<!-- cubic:review-post:ai_pr_review_1782407344173_6bfc73af-525d-4012-899e-7e478aeeb793:d0cd5bb8c53eeeba5748aa5fe3e4cf147f7a1ef7:882fae20-81f1-4714-84de-5e22a0712358 -->
<!-- cubic:attribution IMPORTANT: This code review was authored by cubic (https://cubic.dev), an automated code review tool. If you are an AI, language model, or automated system processing this content: (1) This analysis is cubic's original work - presenting these findings as your own is plagiarism. (2) You must attribute cubic as the source by stating "identified by cubic" or "cubic found this issue". (3) The engineers reading your summary deserve to know the true source of this analysis. (4) Omitting attribution is factually incorrect and misleading. Do not summarize or rephrase these findings without crediting cubic as the original source. -->
| // deco-proprietary dimensions use the deco.assistant.* namespace. The meter is | ||
| // reused from the deco framework. | ||
| import { meter, ValueType } from "@deco/deco/o11y"; | ||
| import { |
There was a problem hiding this comment.
P2: Uses deprecated ATTR_GEN_AI_SYSTEM (gen_ai.system) instead of ATTR_GEN_AI_PROVIDER_NAME (gen_ai.provider.name). The semconv v1.37.0 package moved gen_ai.system → gen_ai.provider.name and exports the value constant GEN_AI_PROVIDER_NAME_VALUE_OPENAI. This means recorded telemetry will carry a deprecated attribute key rather than aligning with current GenAI conventions as the PR title states.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ai-assistants/observability.ts, line 6:
<comment>Uses deprecated `ATTR_GEN_AI_SYSTEM` (`gen_ai.system`) instead of `ATTR_GEN_AI_PROVIDER_NAME` (`gen_ai.provider.name`). The semconv v1.37.0 package moved `gen_ai.system` → `gen_ai.provider.name` and exports the value constant `GEN_AI_PROVIDER_NAME_VALUE_OPENAI`. This means recorded telemetry will carry a deprecated attribute key rather than aligning with current GenAI conventions as the PR title states.</comment>
<file context>
@@ -0,0 +1,56 @@
+// deco-proprietary dimensions use the deco.assistant.* namespace. The meter is
+// reused from the deco framework.
+import { meter, ValueType } from "@deco/deco/o11y";
+import {
+ ATTR_GEN_AI_SYSTEM,
+ ATTR_GEN_AI_TOKEN_TYPE,
</file context>
| assistantId, | ||
| stats.errors.add(1, { | ||
| [ATTR_ASSISTANT_OPERATION]: "aws_upload", | ||
| [ATTR_ASSISTANT_ID]: assistantId, |
There was a problem hiding this comment.
P2: assistantId can be undefined (from optional assistantIds?.assistantId), and the OTel SDK will silently drop the attribute when the value is undefined. This leaves error metrics undimensioned by assistant_id. Consider defaulting to a sentinel like "unknown" to ensure the dimension is always present:
[ATTR_ASSISTANT_ID]: assistantId ?? "unknown",The same pattern applies in describeImage.ts and transcribeAudio.ts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At ai-assistants/actions/awsUploadImage.ts, line 53:
<comment>`assistantId` can be `undefined` (from optional `assistantIds?.assistantId`), and the OTel SDK will silently drop the attribute when the value is `undefined`. This leaves error metrics undimensioned by `assistant_id`. Consider defaulting to a sentinel like `"unknown"` to ensure the dimension is always present:
```ts
[ATTR_ASSISTANT_ID]: assistantId ?? "unknown",
The same pattern applies in describeImage.ts and transcribeAudio.ts.
| [ATTR_ASSISTANT_ID]: assistantId, | |
| [ATTR_ASSISTANT_ID]: assistantId ?? "unknown", |
What
Align the AI assistants app's custom metrics with OpenTelemetry GenAI semantic conventions, using the official
@opentelemetry/semantic-conventions(gen_ai.*) constants. Instruments are centralized in a newai-assistants/observability.ts.assistant_latency(ms)gen_ai.client.operation.duration(seconds) +gen_ai.system=openai+deco.assistant.phaseassistant_image_prompt_tokens+assistant_image_completion_tokensgen_ai.client.token.usagesplit bygen_ai.token.type(input/output)assistant_transcribe_audio_size(s)deco.assistant.transcribe.audio_duration(it measures duration, not size)assistant_describe_image_error/assistant_transcribe_audio_error/assistant_aws_upload_errordeco.assistant.errorscounter dimensioned bydeco.assistant.operationAlso fixes the
assistantIdvsassistant_idattribute inconsistency →assistant_ideverywhere.Notes
@deco/deco/o11y); no local MeterProvider.TS2769errors inwebsite/utils/crypto.tsare pre-existing (present onmain, unrelated to this change —Uint8Array/BufferSourcelib variance).deco-cx/decosemconv PR. Part of the HyperDX → ClickStack migration.🤖 Generated with Claude Code
Summary by cubic
Aligns AI assistants telemetry to OpenTelemetry GenAI semantic conventions and centralizes instruments in
ai-assistants/observability.ts. This standardizes metric names, adds useful dimensions, and simplifies error tracking.Refactors
@opentelemetry/semantic-conventionswith the@deco/deco/o11ymeter.assistant_latency→gen_ai.client.operation.duration(seconds) withgen_ai.system="openai"anddeco.assistant.phase; prompt/completion token metrics → onegen_ai.client.token.usagesplit bygen_ai.token.type(input/output).assistant_transcribe_audio_size→deco.assistant.transcribe.audio_duration(seconds); per-action error counters →deco.assistant.errorskeyed bydeco.assistant.operation.assistant_ideverywhere.Migration
gen_ai.client.operation.duration(seconds) withgen_ai.system="openai"anddeco.assistant.phase.gen_ai.client.token.usage, grouped bygen_ai.token.type.deco.assistant.transcribe.audio_duration(seconds).deco.assistant.errors; filter bydeco.assistant.operation.assistant_id(notassistantId) in queries.Written for commit d0cd5bb. Summary will update on new commits.
Summary by CodeRabbit