Skip to content

feat(tempo): added grafana tempo integration#2646

Open
aniketio-ctrl wants to merge 5 commits into
Tracer-Cloud:mainfrom
aniketio-ctrl:main
Open

feat(tempo): added grafana tempo integration#2646
aniketio-ctrl wants to merge 5 commits into
Tracer-Cloud:mainfrom
aniketio-ctrl:main

Conversation

@aniketio-ctrl
Copy link
Copy Markdown

@aniketio-ctrl aniketio-ctrl commented May 28, 2026

Fixes #
#2563

Describe the changes you have made in this PR -

Adds Grafana Tempo as a first-class standalone integration (not via the Grafana datasource proxy), following the SigNoz pattern. Introduces a TempoClient, a shared OTLP/JSON parser, a query_tempo tool, full integration registry wiring, CLI setup, and tests.

Demo/Screenshot for feature changes and bug fixes -


Code Understanding and AI Usage

Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?

  • No, I wrote all the code myself
  • Yes, I used AI assistance (continue below)

If you used AI assistance:

  • I have reviewed every single line of the AI-generated code
  • I can explain the purpose and logic of each function/component I added
  • I have tested edge cases and understand how the code handles them
  • I have modified the AI output to follow this project's coding standards and conventions

Explain your implementation approach:


Checklist before requesting a review

  • I have added proper PR title and linked to the issue
  • I have performed a self-review of my code
  • I can explain the purpose of every function, class, and logic block I added
  • I understand why my changes work and have tested them thoroughly
  • I have considered potential edge cases and how my code handles them
  • If it is a core feature, I have added thorough tests
  • My code follows the project's style guidelines and conventions

Note: Please check Allow edits from maintainers if you would like us to assist in the PR.

@github-actions
Copy link
Copy Markdown
Contributor

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

@aniketio-ctrl
Copy link
Copy Markdown
Author

@greptile review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR adds Grafana Tempo as a first-class standalone integration that talks directly to the Tempo HTTP API (no Grafana Cloud proxy required), using its own TEMPO_URL/auth. It also extracts the shared OTLP trace parser into app/services/otlp_trace.py so both the new standalone client and the existing Grafana Cloud mixin can reuse it.

  • New TempoClient in app/services/tempo/client.py with _build_traceql (TraceQL query builder with key-validation and value-escaping), search_traces, get_trace_by_id, list_services, and list_span_names; wired up as the query_tempo agent tool.
  • Full integration lifecycle added: TempoConfig model, validate_tempo_config (hits /api/search/tags), _verify_tempo, _setup_tempo CLI flow, _classify_service_instance catalog entry, and tempo_available_or_backend availability guard that correctly gates on connection_verified.
  • Shared OTLP parser (app/services/otlp_trace.py) replaces inline parsing in both the new client and the Grafana Cloud Tempo mixin, removing duplicate code.

Confidence Score: 5/5

Safe to merge; the integration is self-contained, read-only, and all previously flagged correctness issues have been addressed in this revision.

The tag-key injection path now validates keys against _VALID_TAG_KEY_RE before interpolation, the duration precision issue is fixed with float(), and _tempo_is_available correctly gates on connection_verified. The only remaining gap is the CLI setup not prompting for basic-auth credentials, which doesn't affect runtime correctness.

app/integrations/cli.py (_setup_tempo is missing username/password prompts for basic-auth users)

Important Files Changed

Filename Overview
app/services/tempo/client.py New standalone TempoClient; duration precision fix (float) and tag-key injection fix (regex validation) are both present in this revision.
app/integrations/tempo.py New TempoConfig model, validation, and helper functions; URL-only is_configured, auth-header logic, and env-var loading are all consistent.
app/tools/TempoTool/init.py query_tempo tool now correctly delegates availability check to tempo_available_or_backend, which requires connection_verified; previous bypass concern is resolved.
app/tools/utils/availability.py Added tempo_available_or_backend gating on connection_verified
app/integrations/cli.py _setup_tempo added but only prompts for bearer token; basic-auth (username/password) fields supported by TempoConfig are not exposed in the CLI setup flow.
app/services/grafana/tempo.py Refactored _get_trace_details to use shared parse_otlp_trace; behavior is unchanged, inline OTLP parsing removed.
app/services/otlp_trace.py Shared OTLP parsing extracted from Grafana mixin; _duration_ms uses round(float, 4) correctly, extract_span_attributes handles all common OTLP value kinds.
app/integrations/_catalog_impl.py Tempo config classification block follows the same pattern as signoz; exception handling and is_configured guard are both present.

Sequence Diagram

sequenceDiagram
    participant Agent
    participant TempoTool as query_tempo tool
    participant TempoClient
    participant TempoAPI as Tempo HTTP API

    Agent->>TempoTool: call(action, filters, sources)
    TempoTool->>TempoTool: _tempo_is_available(sources) → tempo_available_or_backend
    alt connection_verified or _backend
        TempoTool->>TempoClient: TempoClient(TempoConfig)
        alt "action = search"
            TempoClient->>TempoClient: _build_traceql(service, tags…)
            TempoClient->>TempoAPI: "GET /api/search?q=…"
            TempoAPI-->>TempoClient: "{traces:[…]}"
            TempoClient-->>TempoTool: "{available:true, traces:[…]}"
        else "action = get_trace"
            TempoClient->>TempoAPI: "GET /api/traces/{id}"
            TempoAPI-->>TempoClient: OTLP/JSON
            TempoClient->>TempoClient: parse_otlp_trace(payload)
            TempoClient-->>TempoTool: "{available:true, spans:[…]}"
        else "action = list_services / list_span_names"
            TempoClient->>TempoAPI: "GET /api/v2/search/tag/{tag}/values"
            TempoAPI-->>TempoClient: "{tagValues:[…]}"
            TempoClient-->>TempoTool: "{available:true, services:[…]}"
        end
        TempoTool-->>Agent: result dict
    else not available
        TempoTool-->>Agent: "{available:false, error:…}"
    end
Loading

Reviews (4): Last reviewed commit: "feat(tempo): pr review comments" | Re-trigger Greptile

Comment thread app/services/tempo/client.py
Comment thread app/services/tempo/client.py
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 28, 2026

Greptile Summary

This PR adds Grafana Tempo as a first-class standalone integration that talks directly to the Tempo HTTP API (not through a Grafana datasource proxy), following the same patterns as the existing SigNoz integration.

  • New TempoClient in app/services/tempo/client.py covers trace-by-ID fetch, TraceQL search, and tag-value listing; a shared app/services/otlp_trace.py module is extracted to de-duplicate OTLP/JSON parsing between this client and the existing Grafana Cloud TempoMixin.
  • Full integration scaffold — config model, env-var loading, CLI setup wizard, verification adapter, registry entry, EffectiveIntegrations field, and EvidenceSource literal — mirrors the SigNoz pattern throughout.
  • query_tempo tool uses the project's single-entrypoint action-dispatch pattern with fixture-backend injection for synthetic tests; broad test coverage accompanies the feature.

Confidence Score: 3/5

The feature is well-structured and largely safe, but tag keys in TraceQL expressions are not sanitized, meaning a manipulated LLM call with a crafted key could produce a broken or injected query sent to Tempo.

The new _build_traceql method escapes attribute values but interpolates tag keys directly into the TraceQL string. While Tempo would likely reject a malformed expression, the absence of key validation leaves an unguarded input path through the tool. The round(int(...), 4) issue is minor but indicates float-precision handling was not carefully considered. Everything else — config model, auth header construction, OTLP parsing, tool dispatch, and test coverage — is solid and follows established project patterns.

app/services/tempo/client.py — specifically _build_traceql (tag key sanitization) and _parse_search_traces (duration type handling)

Important Files Changed

Filename Overview
app/services/tempo/client.py New standalone Tempo HTTP client; tag keys in TraceQL construction are not sanitized (potential injection), and round(int(...), 4) is a no-op that silently truncates float durations.
app/integrations/tempo.py New integration config/validation module; clean pydantic model, proper auth header construction, and env-var loading — no issues found.
app/tools/TempoTool/init.py Single action-based tool wrapper; availability check and backend injection pattern match the project's SigNoz pattern.
app/services/otlp_trace.py New shared OTLP/JSON parser; correctly extracts string/int/bool/double attribute types and enriches spans with service_name, duration_ms, and status fields — good refactor.
app/services/grafana/tempo.py Refactored Grafana Tempo mixin to delegate to the shared OTLP parser; the return shape is now richer (additive change) and _extract_span_attributes delegates correctly.
app/integrations/_catalog_impl.py Adds Tempo to _classify_service_instance and load_env_integrations following the same pattern as SigNoz — no issues.
tests/services/test_tempo_client.py Good unit test coverage for all TempoClient methods including error paths, TraceQL construction, and v1/v2 tag-value formats.
tests/synthetic/test_tempo_scenario.py Synthetic end-to-end scenario using a fixture backend; validates tool dispatch wiring and alert-source mapping.

Sequence Diagram

sequenceDiagram
    participant Agent as Investigation Agent
    participant Tool as query_tempo (TempoTool)
    participant Client as TempoClient
    participant Tempo as Tempo HTTP API

    Agent->>Tool: call(action, service, trace_id, tags, ...)
    alt tempo_backend injected (test)
        Tool->>Tool: _dispatch(tempo_backend, ...)
    else real config from _kwargs
        Tool->>Tool: TempoConfig.model_validate(_kwargs)
        Tool->>Client: TempoClient(config)
        alt "action == get_trace"
            Client->>Tempo: "GET /api/traces/{id}"
            Tempo-->>Client: OTLP/JSON
            Client->>Client: parse_otlp_trace()
        else "action == search"
            Client->>Client: _build_traceql(service, span_name, duration, tags)
            Client->>Tempo: "GET /api/search?q={traceql}"
            Tempo-->>Client: trace summaries
        else "action == list_services / list_span_names"
            Client->>Tempo: "GET /api/v2/search/tag/{tag}/values"
            Tempo-->>Client: tag values
        end
        Client-->>Tool: result dict
    end
    Tool-->>Agent: "{source, action, available, ...}"
Loading

Comments Outside Diff (1)

  1. app/services/tempo/client.py, line 762-772 (link)

    P1 Unsanitized tag keys in TraceQL expression

    Tag keys from the tags dict are interpolated directly into the TraceQL string — only the values are escaped via _escape_traceql_value. A key like foo" } || { resource.service.name = "evil would produce a syntactically broken or potentially injected query. While Tempo will likely reject the malformed query, it can cause noisy errors and is a footgun if TraceQL's parser is more permissive than expected.

    Keys should be validated to contain only characters legal in a TraceQL attribute selector (letters, digits, _, -, .) before being embedded in the expression.

Reviews (2): Last reviewed commit: "feat(tempo): added grafana tempo integra..." | Re-trigger Greptile

Comment thread app/services/tempo/client.py
@aniketio-ctrl
Copy link
Copy Markdown
Author

Video of opensre giving RCA via fetching from tempo :

Screen.Recording.2026-05-28.at.10.14.32.PM.1.mp4

@aniketio-ctrl
Copy link
Copy Markdown
Author

@greptile review

@Davidson3556
Copy link
Copy Markdown
Contributor

sorry@aniketio-ctrl this needs to be closed. not assigned to you. you need to be assigned first before you work on any issues

@muddlebee @Devesh36

@aniketio-ctrl
Copy link
Copy Markdown
Author

sorry@aniketio-ctrl this needs to be closed. not assigned to you. you need to be assigned first before you work on any issues

@muddlebee @Devesh36

But it was assigned to me , #2563 (comment)
@Davidson3556

@Davidson3556
Copy link
Copy Markdown
Contributor

Apologies I will review now

@aniketio-ctrl
Copy link
Copy Markdown
Author

Apologies I will review now

Cool, let me know if you need any help.

@aniketio-ctrl
Copy link
Copy Markdown
Author

Opensre verify integrations tempo SS ->
Screenshot 2026-05-29 at 6 40 33 PM

Updated demo video ->

Screen.Recording.2026-05-29.at.7.32.11.PM.mov

@muddlebee
Copy link
Copy Markdown
Collaborator

muddlebee commented May 29, 2026

need to see the demo for entire flow from opensre onboard the integration should work here -> all the env needs to be pasted here, then all of it here https://www.opensre.com/docs/integrations-overview#local-integrations
basically the flow should work from CLI, including env setups etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants