chore: updated docs for otel plugin#2712
chore: updated docs for otel plugin#2712sammaji wants to merge 1 commit into04-08-feat_multiple_otel_profilesfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated OpenTelemetry docs: switched OTLP HTTP examples to v1 endpoints, added profile-based Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant Redpanda as "Redpanda\n(kafka)"
participant Tempo as "Tempo\n(ingest.kafka)"
participant OTEL as "OTEL Collector"
participant Grafana as "Grafana / Tempo UI"
App->>Redpanda: publish traces/metrics (topic: tempo-ingest)
Redpanda->>Tempo: Kafka consumer reads topic
Tempo->>OTEL: forward ingested traces/metrics
OTEL->>Grafana: export/store traces/metrics
Grafana->>OTEL: query traces/metrics
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
🧪 Test Suite AvailableThis PR can be tested by a repository admin. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/features/observability/otel.mdx (1)
191-247: Clarify thatconfig.profiles[]is Gateway v1.5+ config (not Go SDKotel.Config).This page mixes SDK and Gateway setup. A short note near the v1.5+ tabs would prevent users from applying
profiles[]to SDK initialization code.💡 Suggested wording
<Tab title="config.json (v1.5+)"> For Gateway mode, configure via `config.json`: +> Note: `profiles` is a Gateway config schema (v1.5+). Go SDK initialization still uses the flat `otel.Config` fields.Based on learnings: "When reviewing documentation PRs in a Graphite-managed stack, first check related or previous PRs in the stack for feature implementations before flagging documentation as incorrect or unsupported."
Also applies to: 859-937
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/observability/otel.mdx` around lines 191 - 247, Add a short clarifying note next to the "config.json (v1.5+)" tab (and similarly in the other v1.5+ tabs around the same section) that explicitly states that config.profiles[] is the Gateway v1.5+ plugin configuration and NOT the Go SDK otel.Config or SDK initialization code; update the wording to recommend using the SDK's otel.Config when initializing instrumentation in application code and reserve profiles[] examples for Gateway config only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/observability/otel.mdx`:
- Around line 278-289: The “This launches” post-snippet service list is missing
the redpanda service entry; update the list under the “This launches” section to
include a short bullet/line for the redpanda service (matching the service name
"redpanda" used in the docker-compose snippet and describing it as the
Kafka-compatible broker started by that container) so the runtime topology
reflects the added container.
- Around line 327-328: The prometheus service in the compose snippet contains a
duplicate depends_on mapping; remove the second depends_on entry under the
prometheus service so there is only one depends_on key (merge any additional
entries into the existing depends_on if needed) to avoid duplicate YAML mapping
keys; locate the prometheus service block and deduplicate the depends_on
mappings.
---
Nitpick comments:
In `@docs/features/observability/otel.mdx`:
- Around line 191-247: Add a short clarifying note next to the "config.json
(v1.5+)" tab (and similarly in the other v1.5+ tabs around the same section)
that explicitly states that config.profiles[] is the Gateway v1.5+ plugin
configuration and NOT the Go SDK otel.Config or SDK initialization code; update
the wording to recommend using the SDK's otel.Config when initializing
instrumentation in application code and reserve profiles[] examples for Gateway
config only.
🪄 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: c171c92e-89cd-4d67-a53d-fbff142a6498
📒 Files selected for processing (1)
docs/features/observability/otel.mdx
Confidence Score: 3/5The v1.5+ config tabs and URL fixes are good, but the Kafka ingestion path is wired into Tempo without being fed by any component in the stack — users following the docs will end up with a non-functional Redpanda integration. One P1 finding: the Kafka/block_builder configuration in Tempo is unreachable because the OTel Collector has no Kafka exporter. This leaves documented infrastructure silently broken. The rest of the changes (new config tabs, URL corrections, Prometheus flags) are clean. docs/features/observability/otel.mdx — specifically the Tempo and OTel Collector configs in the Docker Compose section Important Files Changed
Reviews (6): Last reviewed commit: "chore: updated docs for otel plugin" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - **[Built-in Observability](./default)** - Local logging for development | ||
| - **[Maxim Plugin](./maxim)** - Advanced LLM evaluation and monitoring | ||
| - **[Telemetry](../telemetry)** - Prometheus metrics and dashboards | ||
| - **[Telemetry](../telemetry)** - Prometheus metrics and dashboards |
There was a problem hiding this comment.
The file is missing a trailing newline, which is flagged by most linters (no-newline-at-end-of-file) and can cause noisy diffs in future PRs.
| - **[Telemetry](../telemetry)** - Prometheus metrics and dashboards | |
| - **[Telemetry](../telemetry)** - Prometheus metrics and dashboards |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
sammaji
force-pushed
the
04-14-chore_updated_docs_for_otel_plugin
branch
from
7e9db67 to
c1b37c4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/observability/otel.mdx`:
- Around line 341-343: The docs expose Grafana on host port "4000:3000" (see
ports mapping and GF_FEATURE_TOGGLES_ENABLE: traceqlEditor) but still instruct
users to visit http://localhost:3000; update the user-facing URL(s) to
http://localhost:4000 (or make the ports mapping and the example URL consistent)
and search for other occurrences of "http://localhost:3000" in this doc to
change them so the exposed host port and documented access URL match.
🪄 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: 88417b0b-c08d-42f9-919d-cf32f8c5b46b
📒 Files selected for processing (1)
docs/features/observability/otel.mdx
sammaji
force-pushed
the
04-14-chore_updated_docs_for_otel_plugin
branch
from
c1b37c4 to
e898d9b
Compare
sammaji
force-pushed
the
04-14-chore_updated_docs_for_otel_plugin
branch
from
e898d9b to
1a70113
Compare
| { | ||
| "service_name": "bifrost", | ||
| "enabled": true, | ||
| "collector_url": "http://localhost:4318", |
There was a problem hiding this comment.
/v1/traces path in HTTP collector_url
The HTTP collector_url here is missing the required /v1/traces path. The bifrost HTTP client sends traces via http.Post(c.endpoint, ...) — it uses the URL exactly as-is with no automatic path appending (see plugins/otel/http.go line 78). Without the path, the OTel Collector will return a 404. Every other HTTP example in this file — the existing "HTTP Protocol" tab (http://otel-collector:4318/v1/traces) and the new "HTTP Protocol (v1.5+)" metrics tab (http://otel-collector:4318/v1/traces) — includes the full path.
| "collector_url": "http://localhost:4318", | |
| "collector_url": "http://localhost:4318/v1/traces", |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/observability/otel.mdx`:
- Around line 192-249: The docs show a nested "profiles" array but the otel
plugin only supports a flat config; update the MDX examples to match the current
Config struct and ValidateConfig behavior in plugins/otel/main.go by replacing
any "config.profiles[]" examples with the flat top-level fields (service_name,
collector_url, trace_type, protocol, insecure, tls_ca_cert, headers, etc.) for
both traces and metrics sections (also address the repeated sections around
lines 868–937). Reference the Config type and ValidateConfig function to ensure
every documented field maps directly to the plugin's supported fields and remove
or gate any "profiles" examples until a code change adds profile support.
🪄 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: e5fe4989-880d-4b62-becb-3c9a7eb55dd6
📒 Files selected for processing (1)
docs/features/observability/otel.mdx
| <Tab title="config.json (v1.5+)"> | ||
|
|
||
| For Gateway mode, configure via `config.json`: | ||
|
|
||
| ```json | ||
| { | ||
| "plugins": [ | ||
| { | ||
| "enabled": true, | ||
| "name": "otel", | ||
| "config": { | ||
| "profiles": [ | ||
| { | ||
| "service_name": "bifrost", | ||
| "enabled": true, | ||
| "collector_url": "http://localhost:4318", | ||
| "trace_type": "genai_extension", | ||
| "protocol": "http", | ||
| "headers": { | ||
| "Authorization": "env.OTEL_API_KEY" | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ``` | ||
|
|
||
| If you need to connect to an OTEL collector that requires TLS, configure `tls_ca_cert` and set insecure mode to `false`: | ||
|
|
||
| ```json | ||
| { | ||
| "plugins": [ | ||
| { | ||
| "enabled": true, | ||
| "name": "otel", | ||
| "config": { | ||
| "profiles": [ | ||
| { | ||
| "service_name": "bifrost", | ||
| "enabled": true, | ||
| "collector_url": "localhost:4317", | ||
| "trace_type": "genai_extension", | ||
| "protocol": "grpc", | ||
| "insecure": false, | ||
| "tls_ca_cert": "/path/to/your/ca.cert", | ||
| "headers": { | ||
| "Authorization": "env.OTEL_API_KEY" | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| ``` | ||
| </Tab> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== OTEL config schema in current branch =="
rg -n 'type Config struct|json:"profiles"|Profiles|service_name|collector_url|trace_type|protocol' plugins/otel/main.go -C2
echo
echo "== OTEL validation requirements (top-level keys) =="
rg -n 'collector url is required|trace type is required|protocol is required|ValidateConfig' plugins/otel/main.go -C2
echo
echo "== Docs references to profiles =="
rg -n '"profiles"|config.json \(v1\.5\+\)' docs/features/observability/otel.mdx -C1
echo
echo "== PR stack metadata for this Graphite-managed PR =="
gh pr view 2712 --repo maximhq/bifrost --json number,title,headRefName,baseRefName,body,urlRepository: maximhq/bifrost
Length of output: 5070
config.profiles[] examples describe a config structure that the current plugin does not support.
The documentation in the v1.5+ section shows nested config.profiles[] configuration, but plugins/otel/main.go defines the Config struct with flat top-level fields only (service_name, collector_url, trace_type, protocol, etc.). The ValidateConfig function validates these top-level fields directly and does not recognize a profiles field. Users who copy these documented examples will fail validation.
This PR updates documentation without a corresponding implementation change to add profile support. Before merging, either:
- Gate these sections behind a separate feature branch/implementation PR, or
- Revert the examples to use the flat config structure that the plugin currently accepts
Example using current supported schema
{
"plugins": [
{
"enabled": true,
"name": "otel",
"config": {
"service_name": "bifrost",
"collector_url": "http://localhost:4318",
"trace_type": "genai_extension",
"protocol": "http",
"headers": {
"Authorization": "env.OTEL_API_KEY"
}
}
}
]
}Also applies to lines 868–937 (metrics configuration sections).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/features/observability/otel.mdx` around lines 192 - 249, The docs show a
nested "profiles" array but the otel plugin only supports a flat config; update
the MDX examples to match the current Config struct and ValidateConfig behavior
in plugins/otel/main.go by replacing any "config.profiles[]" examples with the
flat top-level fields (service_name, collector_url, trace_type, protocol,
insecure, tls_ca_cert, headers, etc.) for both traces and metrics sections (also
address the repeated sections around lines 868–937). Reference the Config type
and ValidateConfig function to ensure every documented field maps directly to
the plugin's supported fields and remove or gate any "profiles" examples until a
code change adds profile support.
1a70113 to
56d60ec
Compare
sammaji
changed the base branch from
graphite-base/2712
to
04-08-feat_multiple_otel_profiles
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/observability/otel.mdx`:
- Line 167: The docs currently state that using tls_ca_cert requires setting
insecure to false, but runtime shows tls_ca_cert simply takes precedence and
insecure only defaults when omitted; update the wording around tls_ca_cert and
insecure to reflect actual behavior: explain that tls_ca_cert supplies a CA for
TLS verification, that specifying tls_ca_cert does not automatically force
insecure=false, and that users should explicitly set insecure: false to enforce
verification (or omit insecure to use the default behavior). Reference the
tls_ca_cert and insecure configuration keys and the TLS/collector setup logic
(the otel plugin TLS parsing code paths) when making the doc change.
🪄 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: 03e79983-dbf2-468a-b2fe-b133b78168f7
📒 Files selected for processing (1)
docs/features/observability/otel.mdx
sammaji
force-pushed
the
04-14-chore_updated_docs_for_otel_plugin
branch
from
56d60ec to
ceb8c4d
Compare
| ingest: | ||
| kafka: | ||
| address: redpanda:9092 | ||
| topic: tempo-ingest | ||
|
|
||
| block_builder: | ||
| consume_cycle_duration: 30s |
There was a problem hiding this comment.
ingest.kafka and block_builder are configured in Tempo, but the OTel Collector's pipeline still exports traces directly to tempo:4317 via otlp/tempo. No component in this stack writes to the tempo-ingest Kafka topic on Redpanda, so the block builder will never consume any data and traces will continue to flow through the standard OTLP path — making Redpanda an unused service.
To make the Kafka path actually work, the OTel Collector config (or a new sidecar) needs a kafka exporter writing to redpanda:9092 on the tempo-ingest topic, and the otlp/tempo exporter should be removed from the traces pipeline. Otherwise, the ingest.kafka/block_builder blocks and the Redpanda service can be removed and the docs simplified back to a direct-ingest setup.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/features/observability/otel.mdx (1)
192-195: Clarify field scope for v1.5+ profile config.Now that
config.profiles[]is documented, add a one-liner in the v1.5+ tabs noting that required fields (collector_url,protocol,trace_type, metrics fields, etc.) are defined per profile entry, not atconfigroot. This will prevent schema misreads.Suggested doc tweak
<Tab title="config.json (v1.5+)"> For Gateway mode, configure via `config.json`: +In v1.5+, required OTel fields are configured per profile under `config.profiles[]`.Also applies to: 860-863
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/observability/otel.mdx` around lines 192 - 195, Add a one-line clarification in the "config.json (v1.5+)" tab stating that telemetry configuration fields (e.g., collector_url, protocol, trace_type, metrics fields) are defined per profile under config.profiles[] and not at the config root; update the same wording in the corresponding v1.5+ tab near the lines referenced (also apply the same line to the section around 860-863) so readers understand the required fields belong to each profile entry rather than to config root.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/features/observability/otel.mdx`:
- Around line 192-195: Add a one-line clarification in the "config.json (v1.5+)"
tab stating that telemetry configuration fields (e.g., collector_url, protocol,
trace_type, metrics fields) are defined per profile under config.profiles[] and
not at the config root; update the same wording in the corresponding v1.5+ tab
near the lines referenced (also apply the same line to the section around
860-863) so readers understand the required fields belong to each profile entry
rather than to config root.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 04b92d20-dc10-446e-b017-823c3ec42181
📒 Files selected for processing (1)
docs/features/observability/otel.mdx
1 participant
Summary
Adds comprehensive configuration examples for OpenTelemetry (OTel) plugin version 1.5+ and updates the observability stack with Redpanda integration for improved trace ingestion performance.
Changes
config.json (v1.5+)with examples for both HTTP and gRPC protocolsType of change
Affected areas
How to test
Validate the documentation changes by reviewing the configuration examples:
New configuration options documented:
profilesarray structure for v1.5+protocolfield (http/grpc)metrics_enabledandmetrics_endpointfor push-based metricsmetrics_push_intervalfor controlling export frequencyScreenshots/Recordings
N/A - Documentation changes only
Breaking changes
Related issues
N/A
Security considerations
The configuration examples include proper TLS certificate handling and environment variable references for API keys, maintaining security best practices.
Checklist
docs/contributing/README.mdand followed the guidelines