Skip to content

Add a private HTTP API for trace redaction - #7886

Open
carles-grafana wants to merge 1 commit into
grafana:mainfrom
carles-grafana:feat/private-redaction-api
Open

carles-grafana wants to merge 1 commit into
grafana:mainfrom
carles-grafana:feat/private-redaction-api

Conversation

@carles-grafana

@carles-grafana carles-grafana commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Adds a disabled-by-default POST /api/redactions endpoint to the query frontend. It accepts explicit trace IDs for one tenant and submits them to the existing backend scheduler for asynchronous redaction. The response contains the batch ID and number of jobs created; it does not wait for redaction to finish.

The endpoint has only two settings under query_frontend.redaction: enabled and backend_scheduler_address. It reuses the existing top-level backend_scheduler_client.grpc_client_config for TLS and other transport settings, rather than defining a second client configuration.

This is a private administrator-facing endpoint, not a public API. Keep the route behind administrator-only access controls and off the public gateway allowlist.

Relationship to secret detection

Split from #7885. This PR contains only the redaction endpoint, configuration, application wiring, tests, and documentation. It does not depend on secret detection; both PRs target main independently.

Validation

  • go test -timeout=5m ./modules/frontend ./cmd/tempo/app ./pkg/api
  • make chlog-validate
  • make generate-manifest
  • Standalone Tempo startup smoke check: the redaction module honors the shared TLS settings and fails on a missing configured CA file.
  • Removed per-endpoint transport configuration is rejected rather than silently ignored.

Checklist

  • Tests updated
  • Documentation added
  • Changelog entry added

Accept trace IDs at POST /api/redactions and submit them to the existing backend scheduler for asynchronous redaction. Reuse the shared backend_scheduler_client transport settings, keep the endpoint disabled by default, and document that it must stay behind administrator-only access controls.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6cae93e. Configure here.

if err != nil {
writeRedactionError(w, http.StatusBadRequest, "trace_ids must contain canonical 32-character hexadecimal trace IDs")
return
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Trace ID parsing rejects valid IDs

Medium Severity

The redaction handler only accepts exactly 32-character hex strings and does not use HexStringToTraceID. Tempo search returns IDs via TraceIDToHexString, which strips leading zeros, so many valid IDs from Tempo itself (all 64-bit IDs, and about one in sixteen 128-bit IDs) are rejected. The CLI already pads these correctly.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6cae93e. Configure here.

Copilot AI 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.

🟡 Changes recommended

The endpoint lacks an explicit administrator-only boundary, and scheduler submission lacks a bounded request timeout.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a disabled-by-default POST /api/redactions endpoint that validates trace IDs and asynchronously submits redaction jobs to the backend scheduler.

Changes:

  • Adds redaction configuration and scheduler client wiring.
  • Reuses shared transport settings.
  • Adds tests, documentation, generated references, and a changelog entry.
File summaries
File Summary
pkg/api/http.go Adds the redaction API route.
modules/frontend/redaction_handler.go Validates requests and submits redaction batches; submission lacks a bounded deadline.
modules/frontend/redaction_handler_test.go Tests validation, submission, responses, and errors.
modules/frontend/docs/config-reference.md Updates the configuration reference.
modules/frontend/config.go Adds redaction settings.
docs/sources/tempo/configuration/manifest.md Updates the configuration manifest.
docs/sources/tempo/configuration/_index.md Documents redaction configuration.
cmd/tempo/app/modules.go Wires the client and route; the route lacks explicit administrator-only protection and a bounded timeout.
.chloggen/private-redaction-api.yaml Adds the changelog entry.
Review details

Suppressed comments (2)

cmd/tempo/app/modules.go:484

  • MEDIUM: This route is wrapped only with authentication, while the query-frontend /api handlers receive Frontend.APITimeout in initQueryFrontend. SubmitRedaction is a synchronous gRPC submission, so an unresponsive scheduler can leave this HTTP request waiting without that configured bound. Could the existing API timeout (or another bounded submission context) be applied here and map expiry to a gateway-timeout response?
		Handler(t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(handler.Submit)))

modules/frontend/redaction_handler.go:100

  • [MEDIUM] Could this handler apply Frontend.APITimeout (or a dedicated bounded deadline) before invoking the remote scheduler? Unlike the other /api frontend handlers, this route is wrapped only with HTTPAuthMiddleware, so the gRPC submission inherits an HTTP context with no deadline; if the scheduler stays unavailable or connecting, requests can remain open indefinitely and accumulate. The redaction work is asynchronous after acceptance, but the submission RPC itself still needs a bound.
	response, err := h.client.SubmitRedaction(ctx, &tempopb.SubmitRedactionRequest{
		TraceIds: traceIDs,
		Mode:     tempopb.RedactionMode_REDACTION_MODE_APPLY,
	})
  • Files reviewed: 9/9 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cmd/tempo/app/modules.go
Comment on lines +481 to +484
t.Server.HTTPRouter().
Methods(http.MethodPost).
Path(addHTTPAPIPrefix(&t.cfg, api.PathRedactions)).
Handler(t.HTTPAuthMiddleware.Wrap(http.HandlerFunc(handler.Submit)))
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.

2 participants