Skip to content

Update tempo docs - #945

Closed
aantn wants to merge 26 commits into
masterfrom
update-tempo-docs
Closed

aantn wants to merge 26 commits into
masterfrom
update-tempo-docs

Conversation

@aantn

@aantn aantn commented Sep 6, 2025

Copy link
Copy Markdown
Collaborator

LLM generated - needs a little review, but seems correct

@aantn
aantn requested a review from Sheeproid September 6, 2025 17:18
@coderabbitai

coderabbitai Bot commented Sep 6, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Documentation for Grafana Tempo builtin toolset updated: added a configuration reference, proxy vs direct connection guidance, environment-variable templating, example configurations (multi-tenant and env-var based), and expanded capabilities list with new and removed operations. Examples were updated to use templated api_key and clarified optional headers/labels/healthcheck.

Changes

Cohort / File(s) Summary
Docs: Grafana Tempo toolset
docs/data-sources/builtin-toolsets/grafanatempo.md
Added Configuration Reference (url, api_key, grafana_datasource_uid, headers, external_url, healthcheck, labels) and default label mappings. Introduced Grafana proxy prerequisites and steps. Switched examples to env-var templating for api_key; added optional headers/labels/healthcheck notes. Updated direct connection URL to tempo.monitoring:3200. Added example configs (multi-tenant, env-var). Expanded capabilities: removed fetch_tempo_* legacy items; added comparative trace search, tag search, trace by ID, and metrics queries. Minor wording adjustments.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant H as Holmes CLI / Robusta
  participant G as Grafana (Proxy)
  participant T as Tempo

  Note over U,H: Proxy-based setup
  U->>H: Configure grafana_datasource_uid, api_key (env), headers
  H->>G: Query via Grafana datasource API
  G->>T: Forward trace/metrics query
  T-->>G: Results
  G-->>H: Response
  H-->>U: Display results/status
Loading
sequenceDiagram
  autonumber
  actor U as User
  participant H as Holmes CLI / Robusta
  participant T as Tempo

  Note over U,H: Direct connection
  U->>H: Configure url (e.g., tempo.monitoring:3200), optional healthcheck
  H->>T: Direct trace/metrics query
  T-->>H: Results
  H-->>U: Display results/status
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • small copy improvements #833 — Updates Grafana API healthcheck and messaging for grafana_datasource_uid and detects misconfigured direct Loki/Tempo URLs; aligns with this PR’s proxy/datasource UID docs.

Suggested reviewers

  • moshemorad
  • arikalon1
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-tempo-docs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/data-sources/builtin-toolsets/grafanatempo.md (1)

53-58: Prefer bearer token over basic auth for Grafana API UID lookup.

Examples earlier recommend service account tokens. Show a token-based curl to avoid encouraging basic auth.

-# List the Tempo data sources
-curl -s -u <username>:<password> http://localhost:3000/api/datasources | jq '.[] | select(.type == "tempo")'
+# List the Tempo data sources (service account token)
+# export GRAFANA_TOKEN=<your service account token>
+curl -s -H "Authorization: Bearer $GRAFANA_TOKEN" http://localhost:3000/api/datasources \
+  | jq '.[] | select(.type == "tempo")'
🧹 Nitpick comments (8)
docs/data-sources/builtin-toolsets/grafanatempo.md (8)

9-18: Clarify external_url purpose and healthcheck semantics; add examples.

Readers won’t know how these affect link generation and probing. Add a short note and example right after the table.

 | `labels` | No | Custom label mappings for Kubernetes resources | See below |
 
+> Note:
+> - `external_url` is used to generate deep links in answers (e.g., to Grafana Explore). If omitted, HolmesGPT may fall back to `url`.
+> - `healthcheck` is a path appended to `url` for readiness checks. Use `api/health` when proxying via Grafana; use `ready` or `live` when connecting directly to Tempo.
+
+Example:
+```yaml
+# optional
+external_url: https://acme-corp.grafana.net
+# proxy mode health check:
+healthcheck: "api/health"
+```

88-103: Include external_url and proxy healthcheck in the Grafana proxy example.

Makes link generation and health probing explicit for proxy mode.

       grafana/tempo:
         enabled: true
         config:
           api_key: "{{env.GRAFANA_API_KEY}}"  # Can use env variables
           url: <your grafana url> # e.g. https://acme-corp.grafana.net
           grafana_datasource_uid: <the UID of the tempo data source in Grafana>
+          # Optional: Deep links & health probe (proxy mode)
+          # external_url: <your grafana url>
+          # healthcheck: "api/health"
           # Optional: Add headers for authentication or other purposes
           # headers:
           #   Custom-Header: "value"

121-131: Mirror external_url and proxy healthcheck in Helm example.

Keeps CLI and Helm guidance aligned.

           grafana_datasource_uid: <the UID of the tempo data source in Grafana>
+          # Optional: Deep links & health probe (proxy mode)
+          # external_url: <your grafana url>
+          # healthcheck: "api/health"
           # Optional: Custom label mappings (defaults shown)
           # labels:
           #   pod: "k8s.pod.name"

148-156: Direct mode: clarify healthcheck path and optionally show external_url.

Avoid ambiguity on whether paths need a leading slash and how links are built.

Can you confirm whether the code expects a leading “/” for healthcheck (e.g., "/ready") or a bare segment ("ready")?

         config:
           url: http://tempo.monitoring:3200
           # Optional: API key for authentication
           # api_key: "{{env.TEMPO_API_KEY}}"
           # Optional: Headers for multi-tenancy or custom auth
           # headers:
           #   X-Scope-OrgID: "<tenant id>"
           # Optional: Custom health check endpoint
-          # healthcheck: "ready"  # or "live"
+          # healthcheck: "ready"  # or "live" (path appended to the URL)
+          # Optional: Deep links target (e.g., Grafana Explore)
+          # external_url: https://acme-corp.grafana.net

168-176: Replicate direct-mode notes in Helm example.

Keeps both examples consistent.

           # Optional: Headers for multi-tenancy or custom auth
           # headers:
           #   X-Scope-OrgID: "<tenant id>"
           # Optional: Custom health check endpoint
-          # healthcheck: "ready"  # or "live"
+          # healthcheck: "ready"  # or "live" (path appended to the URL)
+          # Optional: Deep links target (e.g., Grafana Explore)
+          # external_url: https://acme-corp.grafana.net

219-236: Multi-tenant example: add healthcheck and optional external_url.

Enhances completeness for real-world deployments.

     config:
       url: http://tempo.monitoring:3200
       api_key: "{{env.TEMPO_API_KEY}}"
+      # healthcheck: "ready"
+      # external_url: https://acme-corp.grafana.net
       headers:
         X-Scope-OrgID: "production"
       labels:

242-250: Confirm env-var interpolation behavior for Helm values.

Vanilla Helm doesn’t expand {{env.*}} inside values; this relies on Robusta’s tooling. Add a one-liner note or link, and validate this still works.

       api_key: "{{env.TEMPO_API_KEY}}"
       grafana_datasource_uid: "{{env.GRAFANA_TEMPO_DATASOURCE_UID}}"
+      # Note: Environment variable templating is handled by Holmes/Robusta tooling, not vanilla Helm.

256-263: Verify capabilities list matches implemented tools; add cross-reference.

Ensure removed tools aren’t referenced elsewhere and link to the toolset schema or code for discoverability.

Would you like me to scan the repo to confirm the tool names and surface any stale references?

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 402384b and 71b5249.

📒 Files selected for processing (1)
  • docs/data-sources/builtin-toolsets/grafanatempo.md (6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

In MkDocs content, add a blank line between a header or bold text and the following list so lists render correctly

Files:

  • docs/data-sources/builtin-toolsets/grafanatempo.md
🪛 LanguageTool
docs/data-sources/builtin-toolsets/grafanatempo.md

[grammar] ~9-~9: There might be a mistake here.
Context: ...eld | Required | Description | Default | |-------|----------|-------------|------...

(QB_NEW_EN)


[grammar] ~10-~10: There might be a mistake here.
Context: ...----|----------|-------------|---------| | url | Yes | Base URL for Tempo or Gr...

(QB_NEW_EN)


[grammar] ~11-~11: There might be a mistake here.
Context: ... URL for Tempo or Grafana instance | - | | api_key | No | API key for authentic...

(QB_NEW_EN)


[grammar] ~12-~12: There might be a mistake here.
Context: ... for authentication (Bearer token) | - | | grafana_datasource_uid | No | UID of...

(QB_NEW_EN)


[grammar] ~13-~13: There might be a mistake here.
Context: ...ce in Grafana (enables proxy mode) | - | | headers | No | Additional HTTP heade...

(QB_NEW_EN)


[grammar] ~14-~14: There might be a mistake here.
Context: ... headers (e.g., for multi-tenancy) | - | | external_url | No | External URL for...

(QB_NEW_EN)


[grammar] ~15-~15: There might be a mistake here.
Context: ... External URL for generating links | - | | healthcheck | No | Health check endp...

(QB_NEW_EN)


[grammar] ~16-~16: There might be a mistake here.
Context: ...| Health check endpoint path | "ready" | | labels | No | Custom label mappings ...

(QB_NEW_EN)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Pre-commit checks
  • GitHub Check: llm_evals
  • GitHub Check: Pre-commit checks
🔇 Additional comments (1)
docs/data-sources/builtin-toolsets/grafanatempo.md (1)

5-15: MkDocs spacing looks correct around headings and the table.

There’s a blank line after headers before the table; should render fine.

@github-actions

github-actions Bot commented Sep 6, 2025

Copy link
Copy Markdown
Contributor

Results of HolmesGPT evals

  • ask_holmes: 32/37 test cases were successful, 0 regressions, 2 skipped, 3 setup failures
Test suite Test case Status
ask 01_how_many_pods ✅
ask 02_what_is_wrong_with_pod ✅
ask 04_related_k8s_events ↪️
ask 05_image_version ✅
ask 09_crashpod ✅
ask 10_image_pull_backoff ✅
ask 110_k8s_events_image_pull ✅
ask 11_init_containers ✅
ask 13a_pending_node_selector_basic ✅
ask 14_pending_resources ✅
ask 15_failed_readiness_probe ✅
ask 17_oom_kill ✅
ask 18_crash_looping_v2 🚧
ask 19_detect_missing_app_details ✅
ask 20_long_log_file_search ✅
ask 24_misconfigured_pvc ✅
ask 24a_misconfigured_pvc_basic ✅
ask 28_permissions_error 🚧
ask 29_events_from_alert_manager ↪️
ask 39_failed_toolset ✅
ask 41_setup_argo ✅
ask 42_dns_issues_steps_new_tools ✅
ask 43_current_datetime_from_prompt ✅
ask 45_fetch_deployment_logs_simple ✅
ask 51_logs_summarize_errors ✅
ask 53_logs_find_term ✅
ask 54_not_truncated_when_getting_pods ✅
ask 59_label_based_counting ✅
ask 60_count_less_than 🚧
ask 61_exact_match_counting ✅
ask 63_fetch_error_logs_no_errors ✅
ask 79_configmap_mount_issue ✅
ask 83_secret_not_found ✅
ask 86_configmap_like_but_secret ✅
ask 93_calling_datadog[0] ✅
ask 93_calling_datadog[1] ✅
ask 93_calling_datadog[2] ✅

Legend

  • ✅ the test was successful
  • ↪️ the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🚧 the test had a setup failure (not a code regression)
  • 🔧 the test failed due to mock data issues (not a code regression)
  • ❌ the test failed and should be fixed before merging the PR

Base automatically changed from tempo-freefrom-toolset to master September 7, 2025 13:09
@aantn aantn closed this Sep 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants