refactor: changes attribute keys in OTEL plugin to follow semantic conventions - #3732
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR expands Bifrost tracing to emit OpenTelemetry GenAI spec-compliant attributes alongside legacy attributes for backward compatibility. New OTel conversion helpers and attribute schema definitions enable core request handling, MCP tool spans, and request/response attribute population to emit both spec-style ( ChangesOTel Tracing Attribute Alignment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@core/bifrost.go`:
- Around line 5271-5274: The key-selection span is setting the provider
attribute using string(providerKey) which mismatches the canonical
representation used elsewhere; update the call that sets
schemas.AttrProviderName in the key-selection flow (where
keyTracer.SetAttribute(keyHandle, schemas.AttrProviderName, string(providerKey))
is used) to use the canonical helper schemas.OTelProviderName(providerKey)
instead so the same attribute key (schemas.AttrProviderName) emits the same
mapped value across spans (reference symbols: keyTracer.SetAttribute, keyHandle,
providerKey, schemas.AttrProviderName, schemas.OTelProviderName).
In `@core/mcp/pluginpipeline.go`:
- Around line 78-90: The defer currently always calls tracer.EndSpan(spanHandle,
schemas.SpanStatusOk, "") which hides failures; change it to inspect the
named-return finalError and end the span with an error status/message when
finalError != nil (e.g., use schemas.SpanStatusError and finalError.Error())
while preserving the existing tool-result attribute logic (finalResponse, req,
schemas.AttrToolCallResult, tracer.SetAttribute) before calling tracer.EndSpan;
ensure spanHandle and tracer are still nil-checked as before.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 788d971c-dc5d-4220-ada7-06a498039413
📒 Files selected for processing (6)
core/bifrost.gocore/mcp/pluginpipeline.gocore/schemas/otelconv.gocore/schemas/trace.goframework/tracing/llmspan.goplugins/otel/converter.go
Confidence Score: 5/5Safe to merge — all changes are additive attribute emissions with no modification to request routing, provider logic, or auth paths. Every previously emitted attribute is preserved verbatim; the new bifrost.* and spec-aligned keys are purely additive. The named-return refactor in pluginpipeline.go is straightforward and the deferred span status logic is correct. The only gap is that chat completions' N parameter still doesn't surface AttrChoiceCount, but that is a pre-existing omission rather than a regression. framework/tracing/llmspan.go — PopulateChatRequestAttributes is the one place where the new AttrChoiceCount key was not applied despite the chat params struct supporting N. Important Files Changed
Reviews (4): Last reviewed commit: "refactor: changes attribute keys in OTEL..." | Re-trigger Greptile |
3ae04af to
62c69d8
Compare
ad9565a to
e1bee15
Compare
e1bee15 to
bfcf10a
Compare
62c69d8 to
930e60e
Compare
Merge activity
|
The base branch was changed.
bfcf10a to
b92a081
Compare
…nventions (#3732) ## Summary Aligns Bifrost's tracing attribute emissions with the OpenTelemetry GenAI semantic conventions spec. Bifrost-internal concepts (routing, governance, retry counters, etc.) were previously emitted under the `gen_ai.*` namespace, which pollutes the OTel spec namespace. This PR introduces a parallel `bifrost.*` namespace as the canonical home for those attributes and adds spec-compliant mappings for provider names, operation names, token usage keys, tool execution attributes, and more. Legacy `gen_ai.*` emissions are retained in parallel (tagged `// legacy:`) to avoid breaking existing dashboards, with a clear migration path to drop them once consumers have moved over. ## Changes - Added `core/schemas/otelconv.go` with `OTelOperationName` and `OTelProviderName` helpers that map Bifrost-internal types to OTel GenAI spec values (e.g. `Bedrock` → `aws.bedrock`, `ChatCompletionRequest` → `chat`). - Introduced a full set of `bifrost.*` attribute constants (`AttrBifrostProviderName`, `AttrBifrostVirtualKeyID`, `AttrBifrostRetries`, etc.) as the canonical namespace for Bifrost-internal span attributes. - Added new OTel spec-aligned attribute constants: `AttrOperationName`, `AttrChoiceCount`, `AttrEmbeddingsDimensionCount`, `AttrEncodingFormats`, `AttrUsageCacheReadInputTokens`, `AttrUsageCacheCreationInputTokens`, `AttrErrorTypeSpec`, and tool execution attributes (`AttrToolName`, `AttrToolCallID`, `AttrToolCallArguments`, `AttrToolCallResult`, `AttrToolType`). - Updated `executeRequestWithRetries` in `bifrost.go` to emit both legacy and canonical attributes, use `OTelProviderName`/`OTelOperationName`, and format the root span name as `"{operation} {model}"` per the GenAI semconv. - Updated `PopulateRequestAttributes`, `PopulateErrorAttributes`, `PopulateContextAttributes`, and all response/request attribute helpers in `llmspan.go` to emit both legacy and spec-aligned keys in parallel. - Fixed `AttrStopSequences` to emit a proper `[]string` slice instead of a comma-joined string; the joined form is preserved under `AttrBifrostStopSequencesJoined` for back-compat. - Added `gen_ai.usage.input_tokens` / `gen_ai.usage.output_tokens` alongside the deprecated `prompt_tokens` / `completion_tokens` keys across chat, text completion, and embedding response helpers. - Added OTel GenAI tool execution attributes to MCP execute-tool spans in `pluginpipeline.go`, including tool name, call ID, arguments, and result (captured via named returns). - Updated the OTel plugin converter to emit `bifrost.request.id` alongside the legacy `gen_ai.request_id`. ## Type of change - [ ] Bug fix - [ ] Feature - [x] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [ ] Providers/Integrations - [x] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/... ./framework/tracing/... ./plugins/otel/... ``` Validate that spans emitted by `executeRequestWithRetries` contain both the legacy `gen_ai.*` attributes and the new `bifrost.*` / spec-aligned counterparts. Confirm MCP execute-tool spans include `gen_ai.tool.name`, `gen_ai.tool.call.id`, `gen_ai.tool.call.arguments`, and `gen_ai.tool.call.result`. Verify `gen_ai.provider.name` now emits the spec-canonical form (e.g. `aws.bedrock`) while `bifrost.provider.name` retains the short Bifrost name. ## Breaking changes - [ ] Yes - [x] No All previously emitted attributes are still present. New attributes are additive. The only behavioral difference is that `gen_ai.provider.name` now emits the OTel canonical form rather than the Bifrost short name; the short name is preserved under `bifrost.provider.name`. ## Security considerations No auth, secrets, PII, or sandboxing changes. Span attributes may now include tool call arguments and results on MCP tool spans — operators should ensure their observability backend's data retention policies are appropriate for that data. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
## ✨ Features
- **Azure v1 API Migration** — Migrated Azure provider to the v1 API:
removed the `api-version` query parameter and the
`/openai/deployments/{model}/...` URL pattern in favor of
`/openai/v1/{operation}`; the `api_version` field has been dropped from
`AzureKeyConfig` (#3661, #3756)
- **EnvVar Support for OTEL & Prometheus Configs** — `CollectorURL`,
`MetricsEndpoint`, headers, push gateway URL, and basic auth credentials
can now be sourced from environment variables (e.g.,
`env.OTEL_COLLECTOR_URL`); added a new `ConfigMarshallerPlugin`
interface that lets plugins control storage/redaction round-trips
(#3651)
- **OTel Extra Header Forwarding** — `x-bf-eh-*` extra headers forwarded
to upstream providers are now also emitted on the request span under
`gen_ai.request.extra_header.*` for end-to-end tracing (#3730)
- **OTel Semantic Conventions** — Aligned OTel attribute keys with the
OpenTelemetry GenAI spec (canonical `gen_ai.*` and new `bifrost.*`
attributes); legacy attributes are retained in parallel to avoid
breaking existing dashboards (#3732)
- **VK Quota with Provider Configs** — `GetVirtualKeyQuotaByValue` and
the `getVirtualKeyQuota` HTTP response now include `provider_configs`
with their budgets and rate limits (#3721)
- **MCP Temp Token Non-Auth Toggle** — Added
`mcp_enable_temp_token_auth` client config flag to gate short-lived MCP
token minting for non-authenticated users (#3720)
- **Responses Stream in JSON Parser** — `jsonparser` plugin now handles
OpenAI Responses API streaming (`ResponsesStreamRequest`) in addition to
chat completions (#3749)
- **Session API Rework** — Logout now calls both the password-based
session logout and OAuth logout endpoints and resets all RTK Query cache
state (#3698)
## 🐞 Fixed
- **Streaming Latency for Observability** — Deferred root span
termination to the trace completer callback for streaming requests so
request latency is no longer inflated by header-flush time (#3762)
- **Stream Cancellation Race** — Set `BifrostContextKeyConnectionClosed`
before closing the stream and short-circuit `idleTimeoutReader.Read`
when the connection is already closed to avoid panics and hangs on
cancellation (#3733)
- **Bedrock Cache Points** — Strip cache points from Bedrock requests
for models that do not support prompt caching (e.g., GLM, Llama) to
avoid Converse API errors (#3754)
- **Bedrock Empty Text Blocks** — Skip empty/nil text blocks during
Bedrock response conversion to avoid invalid messages (#3747)
- **Bedrock Reasoning + Tools** — Preserve reasoning content blocks on
assistant turns that also contain tool calls in the Bedrock chat
converter (#3690)
- **Bedrock Search Content & Video** — Restored search content and video
parts that were being dropped from Bedrock-native passthrough requests
(#3729)
- **Structured Output Stop Reason** — Fixed an incorrect `tool_calls`
finish reason when structured output is combined with extended-thinking
tools (#3685)
- **Gemini Tool Schema Passthrough** — Forward full tool parameter
schemas via `parametersJsonSchema` instead of the lossy `parameters`
form; corrected tool response role to `user`; resolved structured output
+ tools conflict (#3761)
- **Anthropic Stop Reason & Tool Versions** — Normalized stop reason
mapping (`end_turn` to `stop`, `tool_use` to `tool_calls`, `max_tokens`
to `length`) and upgraded `text_editor_20250124`/`str_replace_editor` to
`text_editor_20250728` for computer-use tools (#3761)
- **Azure Endpoint Redaction** — Fixed a panic when
`AzureKeyConfig.Endpoint` is a literal value rather than an env
reference (#3761)
- **Auth Middleware Path Match** — Match temp-token auth middleware
whitelist against the request path only, not the full URI with query
parameters (#3737)
- **Governance Blocked Models UI** — Restored the missing Blocked Models
create/edit UI in the VK provider config sheet (#3750)
- **Logging Plugin Cleanup Drain** — Fixed a shutdown race where
`batchWriter` could drop in-flight log entries; `Cleanup` now drains
both the recovered batch and remaining queue within a 30-second budget
(#3717)
- **Model Rankings Empty Entries** — Excluded entries with empty `model`
values from model rankings matview queries so blank rows no longer
surface in the UI (#3758)
- **User Filter Duplicates** — Recreated `mv_filter_users` matview to
require non-empty `user_name`, eliminating duplicate filter dropdown
entries (#3764)
- **User Filter Display Name** — Use `user_name` instead of `user_id` as
the display label for users in logging filters (#3691)
- **Large Numeric ID Precision** — Preserve large numeric IDs in URL
search params by skipping JSON parsing for plain strings (#3692)
## 🔧 Refactors & Chores
- **Error Propagation for GetAvailable\* APIs** — `GetAvailable*`
methods on `LoggerPlugin`/`LogManager` now return wrapped errors instead
of silently logging and returning empty slices (#3759)
- **Governance Blocklist Matching** — Use `slices.Contains` for VK
blocked-model matching for clearer code with identical semantics (#3727)
- **Exported `ResolvePeriod`** — Renamed `resolvePeriod` to
`ResolvePeriod` so external packages can reuse the period parsing
(#3763)
## 📚 Docs
- **OTEL Env Var Documentation** — Documented `env.VAR_NAME` support for
`collector_url`, `metrics_endpoint`, and headers in OTEL/Prometheus
plugin docs
- **OTEL OSS Features & Examples** — Added OTEL documentation to the OSS
features list with usage examples (#3731)
- **Anthropic Auth Recommendation** — Recommend `ANTHROPIC_AUTH_TOKEN`
over `ANTHROPIC_CUSTOM_HEADERS` for Claude Code authentication (#3686)
## Summary
This PR releases Bifrost OSS `v1.5.5` and Enterprise `v1.4.4`, bumping all module pins from `v1.5.12`/`v1.3.12` to `v1.5.13`/`v1.3.13` across core, framework, and all plugins. It also hardens the Docker manifest shell scripts, expands CI egress allowlists, and updates documentation to reflect the new SCIM-based user provisioning feature.
## Changes
- **Module version bumps**: All `go.mod`/`go.sum` files updated from `core v1.5.12` → `v1.5.13`, `framework v1.3.12` → `v1.3.13`, and all plugin versions incremented accordingly (`compat`, `governance`, `jsonparser`, `logging`, `maxim`, `mocker`, `otel`, `prompts`, `semanticcache`, `telemetry`).
- **Docker manifest scripts**: Added `#!/usr/bin/env bash` shebang and `set -euo pipefail` to `create-docker-manifest.sh` and `create-docker-manifest-ubi9.sh`; quoted all variable expansions and switched `jq -r` to `jq -er` to fail on null digests.
- **CI egress allowlist**: Added `production.cloudfront.docker.com:443` to Docker-related job allowlists, and added `_https._tcp.dl.google.com:443` and `motd.ubuntu.com:443` to the Ubuntu package job allowlist.
- **Changelog files**: Cleared per-module `changelog.md` files (content moved into the new versioned docs). Added `docs/changelogs/v1.5.5.mdx` and `docs/changelogs/ent-v1.4.4.mdx` with full release notes, and registered both in `docs/docs.json`.
- **Documentation**: Replaced the SSO Integration link with a User Provisioning (SCIM) link in both `README.md` and `transports/README.md`.
- **Enterprise v1.4.4 highlights** (documented): Kafka and Google Cloud Pub/Sub observability sinks, chunked streaming with a 100 MB inter-node message ceiling, BigQuery custom labels via env vars using the new `ConfigMarshallerPlugin` interface, temporary access token expiry extensions, and a multi-node cluster integration harness.
- **OSS v1.5.5 highlights** (documented): Azure v1 API migration, env-var support for OTel/Prometheus configs, OTel extra-header forwarding and semantic-convention alignment, virtual key quota including provider configs, Responses API streaming in `jsonparser`, and a batch of Bedrock, Gemini, Anthropic, Azure, and logging plugin fixes.
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [x] Documentation
- [x] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [x] Plugins
- [ ] UI (React)
- [x] Docs
## How to test
```sh
# Core/Transports
go version
go test ./...
# Verify Docker manifest scripts exit on error
bash -n .github/workflows/scripts/create-docker-manifest.sh
bash -n .github/workflows/scripts/create-docker-manifest-ubi9.sh
```
Validate that the new changelog pages (`changelogs/v1.5.5` and `changelogs/ent-v1.4.4`) render correctly in the docs site.
## Screenshots/Recordings
N/A
## Breaking changes
- [x] Yes
- [ ] No
The Azure provider no longer accepts `api_version` in `AzureKeyConfig` and has migrated to the `/openai/v1/{operation}` URL pattern. See the [v1.4.0 Migration Guide](https://docs.getbifrost.ai/enterprise/migration-guides/v1.4.0) for full details.
## Related issues
#3661, #3756, #3651, #3730, #3732, #3754, #3747, #3690, #3729, #3685, #3733, #3761, #3735, #3721, #3720, #3749, #3698, #3762, #3750, #3727, #3717, #3759, #3758, #3764, #3691, #3692, #3737, #3763
## Security considerations
- The `ConfigMarshallerPlugin` interface redacts secrets (OTel collector URLs, Prometheus push gateway credentials, BigQuery labels) at config storage time and rehydrates them at load time, preventing plaintext secret persistence.
- Docker manifest scripts now use `set -euo pipefail`, preventing silent failures that could result in malformed or missing image manifests being pushed.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable

Summary
Aligns Bifrost's tracing attribute emissions with the OpenTelemetry GenAI semantic conventions spec. Bifrost-internal concepts (routing, governance, retry counters, etc.) were previously emitted under the
gen_ai.*namespace, which pollutes the OTel spec namespace. This PR introduces a parallelbifrost.*namespace as the canonical home for those attributes and adds spec-compliant mappings for provider names, operation names, token usage keys, tool execution attributes, and more.Legacy
gen_ai.*emissions are retained in parallel (tagged// legacy:) to avoid breaking existing dashboards, with a clear migration path to drop them once consumers have moved over.Changes
core/schemas/otelconv.gowithOTelOperationNameandOTelProviderNamehelpers that map Bifrost-internal types to OTel GenAI spec values (e.g.Bedrock→aws.bedrock,ChatCompletionRequest→chat).bifrost.*attribute constants (AttrBifrostProviderName,AttrBifrostVirtualKeyID,AttrBifrostRetries, etc.) as the canonical namespace for Bifrost-internal span attributes.AttrOperationName,AttrChoiceCount,AttrEmbeddingsDimensionCount,AttrEncodingFormats,AttrUsageCacheReadInputTokens,AttrUsageCacheCreationInputTokens,AttrErrorTypeSpec, and tool execution attributes (AttrToolName,AttrToolCallID,AttrToolCallArguments,AttrToolCallResult,AttrToolType).executeRequestWithRetriesinbifrost.goto emit both legacy and canonical attributes, useOTelProviderName/OTelOperationName, and format the root span name as"{operation} {model}"per the GenAI semconv.PopulateRequestAttributes,PopulateErrorAttributes,PopulateContextAttributes, and all response/request attribute helpers inllmspan.goto emit both legacy and spec-aligned keys in parallel.AttrStopSequencesto emit a proper[]stringslice instead of a comma-joined string; the joined form is preserved underAttrBifrostStopSequencesJoinedfor back-compat.gen_ai.usage.input_tokens/gen_ai.usage.output_tokensalongside the deprecatedprompt_tokens/completion_tokenskeys across chat, text completion, and embedding response helpers.pluginpipeline.go, including tool name, call ID, arguments, and result (captured via named returns).bifrost.request.idalongside the legacygen_ai.request_id.Type of change
Affected areas
How to test
go test ./core/... ./framework/tracing/... ./plugins/otel/...Validate that spans emitted by
executeRequestWithRetriescontain both the legacygen_ai.*attributes and the newbifrost.*/ spec-aligned counterparts. Confirm MCP execute-tool spans includegen_ai.tool.name,gen_ai.tool.call.id,gen_ai.tool.call.arguments, andgen_ai.tool.call.result. Verifygen_ai.provider.namenow emits the spec-canonical form (e.g.aws.bedrock) whilebifrost.provider.nameretains the short Bifrost name.Breaking changes
All previously emitted attributes are still present. New attributes are additive. The only behavioral difference is that
gen_ai.provider.namenow emits the OTel canonical form rather than the Bifrost short name; the short name is preserved underbifrost.provider.name.Security considerations
No auth, secrets, PII, or sandboxing changes. Span attributes may now include tool call arguments and results on MCP tool spans — operators should ensure their observability backend's data retention policies are appropriate for that data.
Checklist
docs/contributing/README.mdand followed the guidelines