Skip to content

feat(#2862): Level 2 distributed tracing — OTLP export to a backend - #3903

Merged
ralphbean merged 13 commits into
fullsend-ai:mainfrom
dhshah13:feat/2862-level2-otlp
Jul 10, 2026
Merged

feat(#2862): Level 2 distributed tracing — OTLP export to a backend#3903
ralphbean merged 13 commits into
fullsend-ai:mainfrom
dhshah13:feat/2862-level2-otlp

Conversation

@dhshah13

@dhshah13 dhshah13 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Closes #2862. Implements Level 2 of ADR 0050 — OTLP export of the metadata spans fullsend already records at Level 1. The export-path approach it takes relates to #2780 (below).

What this does

When OTEL_EXPORTER_OTLP_TRACES_ENDPOINT (or OTEL_EXPORTER_OTLP_ENDPOINT) is set, every run exports its Level 1 span tree via OTLP/HTTP to the configured backend, in addition to the local L1 files. Unset → inert, byte-for-byte the L1 path. Still metadata-only (no prompt/completion content — that's L3).

Approach — export path (relates to #2780)

Adopt the OpenTelemetry Go SDK + otlptracehttp rather than hand-rolled OTLP. Export is a replay of the finalized L1 artifacts at run close (otlp.ExportRunDir), not a live TracerProvider: L1 span IDs/timestamps are reproduced byte-for-byte via tracetest.SpanStub, so the wire spans are identical to the L1 file. Fail-open by construction — a bounded 5s flush from context.Background(), capped retries, and the export runs in a post-Finalize defer whose error only warns and never touches the run's exit code.

Commits

  • build(deps): OTel SDK + otlptracehttp (http/protobuf only)
  • feat(#2862): gen_ai.* identity attrs at the L1 source
  • feat(#2862): internal/telemetry/otlp exporter package
  • feat(#2862): runner wiring (export in the Finalize defer)
  • chore(hack): telemetry-replay dev tool (replay a captured run to any OTLP backend)
  • docs(tracing): operator guide
  • ci(triage): OTEL passthrough so orgs can enable L2 via the reusable workflow

Validation

  • Exporter unit coverage (in-repo, reviewer-verifiable): go test ./internal/telemetry/otlp/... — real in-process OTLP/HTTP protobuf sink; span-identity fidelity (byte-equal ids, ns-exact times); fail-open matrix (TCP black-hole, hanging server, refused, DNS .invalid, 4xx, 503→retry→delivered); gating + kill switches (OTEL_SDK_DISABLED, OTEL_TRACES_EXPORTER=none, malformed-endpoint refusal, grpc refusal). otlp pkg ~95%.
  • Live end-to-end against the pilot's managed MLflow: a CI triage run's artifact exported over the native /v1/traces wire (Bearer + experiment-scoped) → trace ingested with state OK, full run → sandbox_create, agent tree, gen_ai.usage.*, tool_calls, model. Backend is an adopter decision per ADR 0050 — proven against MLflow ≥3.6 (local) and the managed instance.
  • Fail-open negative control: production exporter at a proxy-blocked endpoint → fast fail, run unaffected, L1 files intact.

Deliberate deviations (disclosed)

  1. Also accepts the signal-specific OTEL_EXPORTER_OTLP_TRACES_ENDPOINT alongside the ADR-named generic var (OTel-spec precedence; documented).
  2. "Non-blocking flush" is a bounded 5s synchronous flush, not fire-and-forget — a background goroutine would be killed at process exit and lose spans.
  3. Keeps the ADR's literal gen_ai.system key; semconv ≥1.30 renamed it to gen_ai.provider.name → tracked as a follow-up.

Enabling export (next steps — env-var contract)

L2 is opt-in per environment. Merging ships the capability; a run captures only when its environment is configured:

  • OTEL_EXPORTER_OTLP_TRACES_ENDPOINT — e.g. https://<backend>/v1/traces
  • OTEL_EXPORTER_OTLP_TRACES_HEADERS — backend auth + routing (for MLflow: Authorization=Bearer%20<token>,x-mlflow-experiment-id=<id>)
  • OTEL_EXPORTER_OTLP_CERTIFICATE — optional PEM for a private CA (e.g. an internal MLflow)

Kill switches honored: OTEL_SDK_DISABLED=true, OTEL_TRACES_EXPORTER=none.

  • Local runs → export the vars; every run exports at Finalize.
  • Org CI → set the org's Actions vars/secret. v0 is a fixed tag, so orgs pick up the reusable-workflow passthrough only after a release moves it (post-merge).

Out of scope (deliberate)

  • Content capture (prompts/completions/tool output) — Level 3, governance-gated.
  • Populating more MLflow columns — trace name, session, request/response, source/user. The native export emits the metadata spans + gen_ai.* attributes MLflow already renders (tokens, cost, model, tool_calls); richer trace-level columns need an L1 run-context capture plus a vendor-coupling decision, so they're out of scope here — a separate feature request will be opened after this PR lands.
  • Content-capture hardening; backend/infra deployment.

Follow-up: gen_ai.systemgen_ai.provider.name semconv rename.

dhshah13 added 7 commits July 8, 2026 13:30
Adds go.opentelemetry.io/otel, otel/sdk, otel/trace, and the
otlptracehttp exporter for ADR 0050 Level 2 trace export. Only the
HTTP/protobuf exporter is imported — no gRPC exporter; grpc appears in
the module graph transitively via the OTLP proto definitions.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
ADR 0050 and the tracing guide name gen_ai.operation.name and
gen_ai.agent.name on run spans; Level 1 never emitted them. Add them at
the source — the recorder's root span_start — so the local file and the
Level 2 export stay two views of one truth. The bare "agent" key stays
for existing consumers of the Level 1 schema.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
New internal/telemetry/otlp package implementing ADR 0050 Level 2 as a
replay of the Level 1 artifacts: run-telemetry.jsonl and
run-summary.json are parsed back into span snapshots (identical ids,
nanosecond-exact timestamps) and sent through the OTel SDK's OTLP/HTTP
exporter in one bounded export at run close. Export is therefore, by
construction, the same trace the local files record — and a future
'fullsend telemetry push' can replay any captured run directory.

Gating and safety:
- Inert unless OTEL_EXPORTER_OTLP_(TRACES_)ENDPOINT is set; honors
  OTEL_SDK_DISABLED and OTEL_TRACES_EXPORTER=none.
- Endpoint values are pre-validated: the SDK silently falls back to
  localhost:4318 on malformed input, fullsend refuses instead.
- Only http/protobuf; a grpc protocol setting is refused loudly.
- Hard 5s wall-clock budget over construction, send (capped retries),
  and shutdown, derived from context.Background() so traces of failed
  or cancelled runs still flush. Fail-open: errors are returned for a
  single warning line and never affect the run.
- An inbound-unsampled trace (W3C flags -00) is not exported; Level 1
  files are always written.
- Header/TLS/timeout/compression config is delegated entirely to the
  exporter's standard env handling, matching the published guide.

tracetest.SpanStub is the SDK's only public ReadOnlySpan constructor
(the interface has an unexported method); the replay design needs exact
id/timestamp control, which a live TracerProvider cannot guarantee.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
…nto the runner

Export runs inside the existing Finalize defer, after the Level 1
artifacts are complete on disk. Agent iteration spans gain the
ADR-named gen_ai.operation.name/gen_ai.agent.name at span start via the
agentSpanStartAttrs helper; sandbox_create carries create_agent.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
Replays a captured run directory through the production Level 2 export
path against any OTLP backend — the validation workhorse for fullsend-ai#2862 and
the prototype for a future 'fullsend telemetry push'.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
The span-structure section described a hierarchy that was never
emitted; it now matches the real spans (run, sandbox_create, agent)
and the attribute tables match what Level 1 records. Adds the Level 2
operational contract (bounded export at run close, sampled-flag
behavior, http/protobuf only, endpoint validation, kill switches,
private CAs), an MLflow >= 3.6 example incl. the experiment-id header
and Basic-auth percent-encoding, and marks Level 3 content capture as
planned rather than available. The env-var contract section is
unchanged — the implementation conforms to it.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
…kflow

Orgs consume the reusable workflows, whose job env they cannot set, so
there was no way to reach the fullsend process with the standard OTEL
export configuration. Pass it through from the caller's context: the
endpoint and resource attributes as Actions variables, the headers as
an optional secret (they may carry backend auth). Unset values leave
the exporter inert, per the Level 2 gate.

Triage only for now — the remaining reusable workflows follow once the
pattern is validated in the rehearsal org.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
@dhshah13
dhshah13 requested a review from a team as a code owner July 9, 2026 15:52
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

1 similar comment
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Add Level 2 tracing: replay L1 run artifacts via OTLP/HTTP export

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Export finalized Level 1 telemetry artifacts to an OTLP/HTTP backend when endpoint env vars are
 set.
• Preserve span identity/timestamps by replaying recorded L1 events into OTel span snapshots.
• Add operator + CI workflow configuration for opt-in export, plus a replay dev tool.
Diagram

graph TD
  A["GHA workflow env"] --> B["CLI runner"] --> C["Telemetry recorder (L1)"] --> D[("Run artifacts")]
  D --> E["OTLP replay exporter"] --> F["OTel SDK exporter"] --> G{{"OTLP backend"}}
  H["telemetry-replay tool"] --> E

  subgraph Legend
    direction LR
    _svc["Component"] ~~~ _fs[("Artifacts")] ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Live TracerProvider + span processors
  • ➕ Streams spans during execution (near-real-time visibility)
  • ➕ Leverages standard OTel instrumentation patterns
  • ➖ Harder to guarantee byte-for-byte identity with L1 artifacts
  • ➖ Higher risk of impacting runtime (network on hot path, shutdown ordering)
  • ➖ More complexity around sampling, parent adoption, and failure isolation
2. Hand-rolled OTLP/HTTP protobuf export
  • ➕ Smaller dependency surface if done minimally
  • ➕ Tighter control over retries/timeouts without SDK abstractions
  • ➖ High maintenance burden vs. spec/SDK changes
  • ➖ Easy to get protocol details wrong (headers parsing, gzip, retry semantics, resource/env handling)
  • ➖ Would duplicate SDK capabilities already validated by OTel
3. Export via local OpenTelemetry Collector sidecar/agent only
  • ➕ Keeps CLI simpler; collector handles retries/auth/backpressure
  • ➕ Central place to manage routing/transforms
  • ➖ Still needs a producer/export path in the CLI (stdout OTLP/log-based)
  • ➖ Doesn’t satisfy the replay-from-artifacts requirement as directly
  • ➖ Adds operational dependency even for local runs

Recommendation: Keep the PR’s replay-at-finalize approach using the OTel Go SDK. It cleanly enforces “fail-open” semantics, preserves span IDs/timestamps from the Level 1 source of truth, and enables offline replays (useful for debugging and CI artifact export) without introducing mid-run network coupling.

Files changed (13) +1627 / -38

Enhancement (5) +505 / -4
main.goAdd telemetry-replay dev tool to export a captured run dir via OTLP +56/-0

Add telemetry-replay dev tool to export a captured run dir via OTLP

• Adds a small CLI that replays an existing run directory through internal/telemetry/otlp.ExportRunDir using standard OTEL_EXPORTER_OTLP_* env vars. Prints the exported trace_id (from run-summary.json) to help operators locate the trace in the backend.

hack/telemetry-replay/main.go

run.goWire OTLP export into run finalization and add GenAI start attrs +24/-3

Wire OTLP export into run finalization and add GenAI start attrs

• Invokes otlp.ExportRunDir in the finalize defer so export happens only after Level 1 artifacts are finalized and never affects the run outcome. Adds GenAI semantic-convention identity on sandbox_create and agent span starts via a helper for consistent attribute emission.

internal/cli/run.go

otlp.goImplement bounded, fail-open OTLP/HTTP export entrypoint +129/-0

Implement bounded, fail-open OTLP/HTTP export entrypoint

• Adds Enabled() and ExportRunDir() with env-var gating, kill switches, protocol enforcement (http/protobuf only), endpoint pre-validation to avoid SDK localhost fallback, and a hard export timeout. Uses an OTel otlptracehttp exporter with CLI-appropriate retry limits and returns errors for caller warning only.

internal/telemetry/otlp/otlp.go

replay.goParse L1 artifacts and replay them into OTel ReadOnlySpan snapshots +289/-0

Parse L1 artifacts and replay them into OTel ReadOnlySpan snapshots

• Implements reading run-summary.json for sampling and parsing run-telemetry.jsonl with number fidelity (UseNumber) and deterministic ordering. Builds tracetest.SpanStub snapshots with exact IDs/timestamps, merges start/end attrs plus work item id, maps attribute types safely, and constructs OTel resources honoring standard env overrides.

internal/telemetry/otlp/replay.go

recorder.goAdd GenAI semantic-convention identity to the root run span_start +7/-1

Add GenAI semantic-convention identity to the root run span_start

• Extends the root span_start attributes to include gen_ai.operation.name and gen_ai.agent.name per ADR 0050, while retaining the legacy 'agent' key for existing Level 1 consumers.

internal/telemetry/recorder.go

Tests (4) +953 / -0
telemetry_run_test.goAdd unit test for agent span start GenAI identity attributes +7/-0

Add unit test for agent span start GenAI identity attributes

• Introduces a focused test ensuring agentSpanStartAttrs emits iteration plus gen_ai.operation.name and gen_ai.agent.name as expected.

internal/cli/telemetry_run_test.go

otlp_test.goEnd-to-end OTLP/HTTP exporter tests with in-process protobuf sink +693/-0

End-to-end OTLP/HTTP exporter tests with in-process protobuf sink

• Adds comprehensive tests validating env precedence, header propagation/decoding, gzip compression, resource/env behavior, sampling suppression, span identity fidelity (IDs + ns timestamps), SpanKind/parent behavior, status mapping, and multiple fail-open network pathologies with time bounds and retry delivery on 503.

internal/telemetry/otlp/otlp_test.go

replay_test.goTest replay robustness against partial/malformed artifacts and attr edge cases +235/-0

Test replay robustness against partial/malformed artifacts and attr edge cases

• Adds tests for missing files, missing summary (crash/unfinished runs), unpaired starts, malformed lines, invalid IDs/parents/timestamps, duplicate starts, unexpected attribute shapes degrading to strings, and large integer fidelity through JSON parsing/export.

internal/telemetry/otlp/replay_test.go

recorder_test.goAdd regression test for GenAI identity attrs on root span +18/-0

Add regression test for GenAI identity attrs on root span

• Verifies the recorder’s first span_start line contains gen_ai.operation.name and gen_ai.agent.name and preserves the existing 'agent' attribute for Level 1 schema compatibility.

internal/telemetry/recorder_test.go

Documentation (1) +93 / -30
distributed-tracing.mdDocument Level 2 OTLP export behavior and MLflow configuration +93/-30

Document Level 2 OTLP export behavior and MLflow configuration

• Expands the tracing guide with export timing, sampling behavior, protocol constraints, validation rules, kill switches, and TLS CA guidance. Adds an MLflow OTLP/HTTP example (including headers/auth encoding) and recommends resource attribute conventions for org-wide trace organization.

docs/guides/infrastructure/distributed-tracing.md

Other (3) +76 / -4
reusable-triage.ymlPass through OTEL env/secrets for opt-in Level 2 export in triage runs +12/-0

Pass through OTEL env/secrets for opt-in Level 2 export in triage runs

• Adds optional workflow inputs and environment wiring for OTEL_EXPORTER_OTLP_TRACES_ENDPOINT/HEADERS and OTEL_RESOURCE_ATTRIBUTES. This enables organizations to opt into OTLP export via Actions variables/secrets without changing run behavior when unset.

.github/workflows/reusable-triage.yml

go.modAdd OpenTelemetry SDK and OTLP/HTTP exporter dependencies +21/-2

Add OpenTelemetry SDK and OTLP/HTTP exporter dependencies

• Introduces go.opentelemetry.io/otel modules (sdk, trace, otlptracehttp) and OTLP proto packages required for OTLP export and protobuf-based unit tests.

go.mod

go.sumRecord checksums for new OpenTelemetry and transitive dependencies +43/-2

Record checksums for new OpenTelemetry and transitive dependencies

• Updates go.sum to reflect the newly added OTel modules and transitive requirements (e.g., grpc-gateway, genproto, x/net).

go.sum

@qodo-code-review

qodo-code-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (2)

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider


Action required

1. Procedures not numbered steps ✓ Resolved 📜 Skill insight ✧ Quality
Description
The guide presents procedural instructions as prose/comments in code blocks rather than numbered
(ordered) steps. This reduces scanability and violates the required procedure formatting standard.
Code

docs/guides/infrastructure/distributed-tracing.md[R218-220]

## Local development

Run an agent locally with traces going to a local backend:
Relevance

⭐⭐⭐ High

Numbered-step procedure formatting explicitly requested and accepted in docs reviews (PR #2663, PR
#2277).

PR-#2663
PR-#2277

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062079 requires procedural content to be in numbered steps. The `Local
development` procedure is presented as a single code block with comments instead of an ordered list.

docs/guides/infrastructure/distributed-tracing.md[218-234]
Skill: writing-user-docs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Procedural sections should use numbered steps, not prose paragraphs or comment-only sequences.

## Issue Context
The `Local development` section includes multiple actions (start backend, set env var, run command, open UI) presented as a commented code block rather than an ordered list.

## Fix Focus Areas
- docs/guides/infrastructure/distributed-tracing.md[218-234]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. No Prerequisites section ✓ Resolved 📜 Skill insight ✧ Quality
Description
docs/guides/infrastructure/distributed-tracing.md contains procedural commands/configuration
examples but does not include a clearly labeled Prerequisites section before those steps. This
violates the guide structure requirement and can lead to operators missing required setup.
Code

↗ docs/guides/infrastructure/distributed-tracing.md

If the endpoint is unreachable, the CLI continues normally — local files are
Relevance

⭐⭐⭐ High

Team repeatedly accepted adding explicit “Prerequisites” sections in guides (e.g., PR #2663, PR
#2277).

PR-#2663
PR-#2277

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062078 requires each guide to include a prerequisites section before procedural
steps. The guide proceeds directly into OTLP enablement commands without any Prerequisites
section.

docs/guides/infrastructure/distributed-tracing.md[22-33]
Skill: writing-user-docs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The distributed tracing guide includes procedures (environment variables, workflow config, local dev commands) but lacks a `## Prerequisites` section.

## Issue Context
The guide begins with configuration and command snippets under sections like `## Enabling OTLP export (Level 2)` without stating required access/inputs first.

## Fix Focus Areas
- docs/guides/infrastructure/distributed-tracing.md[8-47]
- docs/guides/infrastructure/distributed-tracing.md[22-33]
- docs/guides/infrastructure/distributed-tracing.md[185-234]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Endpoint host not validated ✓ Resolved 🐞 Bug ≡ Correctness
Description
otlp.ExportRunDir intends to reject malformed endpoints to avoid unexpected exporter behavior, but
it only checks URL parse + scheme and does not require a host. Endpoints like http:// will pass
validation even though they are not usable OTLP targets, defeating the intended “refuse malformed
endpoint” guardrail.
Code

internal/telemetry/otlp/otlp.go[R86-91]

+	// Pre-validate: on a malformed endpoint value the SDK reports to its
+	// global error handler and silently falls back to localhost:4318 — a
+	// typo would spray spans at localhost. Refuse instead.
+	if u, err := url.Parse(endpoint); err != nil || (u.Scheme != "http" && u.Scheme != "https") {
+		return fmt.Errorf("OTLP endpoint %q is not an http(s) URL; export skipped", endpoint)
+	}
Relevance

⭐⭐ Medium

URL validation hardening accepted elsewhere, but no direct precedent for requiring URL host
specifically.

PR-#1982

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The current pre-validation checks only that parsing succeeded and that the scheme is http/https, but
does not verify the URL has a host component, so an authority-less URL will not be rejected by this
guard.

internal/telemetry/otlp/otlp.go[86-91]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ExportRunDir` pre-validates the OTLP endpoint using `url.Parse` and scheme checks only. This allows authority-less URLs (e.g. `http://`) to pass validation even though they are not valid endpoints.

### Issue Context
The code comment explicitly states malformed endpoints should be refused to avoid silent fallback behavior; therefore validation should ensure the parsed URL is an absolute http(s) URL with an authority/host.

### Fix Focus Areas
- internal/telemetry/otlp/otlp.go[75-91]

### Implementation notes
- Extend validation to require `u.Host != ""` (and optionally reject other non-absolute forms like `u.Opaque != ""` or `!u.IsAbs()` if you want to be extra strict).
- Add/extend a unit test to cover a case like `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT=http://` being rejected and producing zero network activity.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View more (1)
4. Planned callout lacks issue link 📜 Skill insight ≡ Correctness
Description
The new Level 3 note uses the > **Planned:** callout format but does not include a link to a
tracking issue. This violates the planned-feature documentation requirement.
Code

docs/guides/infrastructure/distributed-tracing.md[R93-96]

+> **Planned:** Level 3 content capture is not yet implemented. This section
+> documents the contract decided in
+> [ADR 0050](../../ADRs/0050-distributed-tracing-instrumentation.md).
+
Relevance

⭐⭐ Medium

No historical evidence found enforcing “Planned” callouts to include issue links.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062082 requires planned features to use the > **Planned:** callout and include
an issue link. The callout is present but lacks any issue reference/link.

docs/guides/infrastructure/distributed-tracing.md[91-96]
Skill: writing-user-docs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Planned features must be documented using a `> **Planned:**` callout that includes a link to the relevant issue.

## Issue Context
The guide states Level 3 content capture is planned, but links only to ADR 0050 and not to an issue tracking the work.

## Fix Focus Areas
- docs/guides/infrastructure/distributed-tracing.md[91-96]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

5. Protocol error message misleading ✓ Resolved 🐞 Bug ◔ Observability
Description
When OTEL_EXPORTER_OTLP_TRACES_PROTOCOL is set to an unsupported value, ExportRunDir returns an
error message that always names OTEL_EXPORTER_OTLP_PROTOCOL, making debugging configuration issues
harder. This is especially confusing because protocolFromEnv gives precedence to the
traces-specific variable.
Code

internal/telemetry/otlp/otlp.go[R95-97]

+	if p := protocolFromEnv(); p != "" && p != "http/protobuf" {
+		return fmt.Errorf("OTEL_EXPORTER_OTLP_PROTOCOL %q is not supported (only http/protobuf); export skipped", p)
+	}
Relevance

⭐⭐ Medium

No clear precedent on error messages naming the exact env var source/precedence.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The error string hard-codes OTEL_EXPORTER_OTLP_PROTOCOL, while the protocol value may have come
from the traces-specific variable due to precedence in protocolFromEnv.

internal/telemetry/otlp/otlp.go[95-97]
internal/telemetry/otlp/otlp.go[122-129]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`protocolFromEnv()` prefers `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL`, but the error message for an unsupported protocol always references `OTEL_EXPORTER_OTLP_PROTOCOL`.

### Issue Context
This is a diagnostics/operability issue: the export is correctly skipped, but the warning message can point operators to the wrong variable.

### Fix Focus Areas
- internal/telemetry/otlp/otlp.go[95-129]

### Implementation notes
- Either:
 - Make `protocolFromEnv` return `(value, sourceVarName)` and use `sourceVarName` in the error, or
 - Mention both variables in the error (e.g. "OTEL_EXPORTER_OTLP_(TRACES_)PROTOCOL ...").

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

6. Guide under infrastructure/ path 📜 Skill insight ⌂ Architecture
Description
The modified guide is located at docs/guides/infrastructure/... instead of being placed under
docs/guides/admin/ or docs/guides/user/ as required. This breaks the required guide directory
convention and makes audience targeting ambiguous.
Code

↗ docs/guides/infrastructure/distributed-tracing.md

If the endpoint is unreachable, the CLI continues normally — local files are
Relevance

⭐ Low

Repo already uses docs/guides/infrastructure/ (distributed-tracing added there in PR #1489; mint
guide too).

PR-#1489
PR-#1708
PR-#2698

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062077 requires guide files under docs/guides/ to be placed in admin/ or
user/ subdirectories. The modified guide remains under docs/guides/infrastructure/.

docs/guides/infrastructure/distributed-tracing.md[1-6]
Skill: writing-user-docs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The guide file is under `docs/guides/infrastructure/`, but guides must live under either `docs/guides/admin/` or `docs/guides/user/`.

## Issue Context
This PR modifies `docs/guides/infrastructure/distributed-tracing.md`, so the placement requirement applies to this changed guide.

## Fix Focus Areas
- docs/guides/infrastructure/distributed-tracing.md[1-6]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread docs/guides/infrastructure/distributed-tracing.md
Comment thread docs/guides/infrastructure/distributed-tracing.md
Comment thread internal/telemetry/otlp/otlp.go
Comment thread internal/telemetry/otlp/otlp.go
dhshah13 added 2 commits July 9, 2026 12:16
…rror, docs)

- otlp: reject authority-less endpoints (e.g. http://) up front to match the
  "refuse malformed endpoint" guard, with a clearer error; add an http:// case
  to the fail-open test
- otlp: name OTEL_EXPORTER_OTLP_(TRACES_)PROTOCOL in the unsupported-protocol
  error (protocolFromEnv prefers the traces-specific variable)
- docs(tracing): add a Prerequisites section; convert Local development to
  numbered steps (writing-user-docs convention)

Signed-off-by: Dharit Shah <dhshah@redhat.com>
Signed-off-by: Dharit Shah <dhshah@redhat.com>

# Conflicts:
#	go.mod
#	go.sum
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

Site preview

Preview: https://55991576-site.fullsend-ai.workers.dev

Commit: fa11d3bd5229237d0014612b99bdc3e5be8380b3

@ascerra

ascerra commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 9, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:36 PM UTC · Completed 5:48 PM UTC
Commit: e8381e3 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review

Verdict: approve — well-engineered Level 2 OTLP export implementation. No medium or higher findings.

Summary

This PR implements ADR 0050 Level 2 distributed tracing: opt-in OTLP/HTTP export of the metadata spans already recorded at Level 1. The design is a replay of finalized L1 artifacts (run-telemetry.jsonl + run-summary.json) at run close, ensuring the local file and the wire spans are two views of one trace. The implementation is sound across all six review dimensions.

Correctness: The export flow is well-structured — env-var gating, URL validation (preventing the SDK's silent localhost fallback), kill switches (OTEL_SDK_DISABLED, OTEL_TRACES_EXPORTER=none), upstream sampling-decision respect, bounded 5s flush from context.Background(), and errors.Join for multi-error shutdown. The replay.go span reconstruction correctly handles parent resolution (local vs remote via TRACEPARENT), SpanKind mapping (Consumer for dispatched runs), status mapping, and attribute type preservation (json.Number for exact int64/float64 fidelity at 2^53+1). The defer ordering in run.go is correct: Finalize writes and closes the L1 file, then ExportRunDir reopens it read-only.

Security: Content sandboxing is intact — the data path from eventRecord.Attrs through replay.go to the wire carries only metadata (timing, token counts, tool names, cost, model). No prompts, completions, or source code flow to the wire. No TLS skip-verify option exists. The OTEL_EXPORTER_OTLP_TRACES_HEADERS secret is correctly scoped — the workflow's action code comes from the trusted base branch, and the secret's exposure follows the same trust model as existing GCP credentials in the step. No secrets found in the diff.

Intent & coherence: The change traces cleanly to issue #2862 and ADR 0050. The feat prefix is correct per COMMITS.md — this is a user-facing capability (operators set env vars and their traces appear in a backend). No breaking change exists (purely additive, inert by default). The replay-based design is architecturally coherent with the project's fail-open and local-artifacts-first philosophy.

Test adequacy: The test suite is thorough — ~930 lines covering gating (endpoint presence, kill switches, malformed endpoints, unsupported protocols, unsampled traces), fidelity (span identity byte-equality, attribute type preservation, parent resolution, SpanKind, status mapping, work-item-id propagation, resource attributes), fail-open (TCP black hole, hanging HTTP, connection refused, DNS .invalid, HTTP 4xx, 503→retry→delivered), and edge cases (malformed lines, invalid hex IDs, unpaired spans, duplicate starts, unexpected attribute shapes, large integer fidelity). No test weakening detected in modified test files.

Cross-repo contracts: All changes are backward-compatible. The new reusable workflow secret is required: false. The L1 schema gains gen_ai.* attributes additively (existing agent key preserved). No exported Go API changes.

Documentation: The extensive updates to distributed-tracing.md are accurate and comprehensive — they fix pre-existing staleness (old span hierarchy, old attribute names) and add new operational guidance (prerequisites, export timing, MLflow example, trace organization).

Low-severity notes

  1. OTLP env vars scoped to triage workflow only (reusable-triage.yml): The CI passthrough is intentionally scoped to triage per the commit message. The other five reusable workflows (code, fix, review, retro, prioritize) will need the same env var threading in a follow-up for orgs wanting full agent tracing. The code is inert without the endpoint, so there is no breakage.

  2. Case-sensitive OTEL_TRACES_EXPORTER check (otlp.go:91): Uses == "none" while the adjacent OTEL_SDK_DISABLED check uses strings.EqualFold. The OTel spec recommends case-insensitive comparison for env var values. Practical risk is negligible since none is the canonical form, but the inconsistency within the same function is worth tidying.

Positive signals

  • Fail-open is proven by tests, not promised by comments: TCP black-hole, hanging server, DNS failure, and HTTP error tests all assert bounded completion and L1 artifact integrity.
  • The url.Parse pre-validation at the endpoint gate prevents the SDK's silent localhost:4318 fallback on malformed URLs — a real production footgun caught.
  • json.Number usage in replay.go preserves integer fidelity through the JSON round-trip (2^53+1 tested), avoiding the classic float64 mangling bug.
  • The documentation update is unusually thorough for a feature PR, covering prerequisites, operational details, an MLflow integration example, and org-level trace organization guidance.

Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

  • .github/workflows/reusable-triage.yml

@fullsend-ai-review fullsend-ai-review 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.

See the review comment for full details.

Comment thread internal/telemetry/otlp/otlp.go
@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jul 9, 2026
…insensitively

The OTel spec recommends case-insensitive comparison of env var values;
the adjacent OTEL_SDK_DISABLED check already uses strings.EqualFold.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.26168% with 8 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/run.go 45.45% 6 Missing ⚠️
internal/telemetry/otlp/replay.go 98.70% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Summary-without-telemetry-file, blank lines, a line beyond the scanner
cap (the one readRun error), a bad end-side timestamp, and a number too
large for float64. Lifts the otlp package to 99.3% coverage; the one
remaining block is the summarySampled ParseUint fallback, unreachable
because ParseTraceParent already validates flags as two lowercase hex
chars.

Signed-off-by: Dharit Shah <dhshah@redhat.com>

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM waiting for @ascerra review

@ascerra ascerra 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.

Review

Exporter looks solid — rehearsal artifact 5a7818ccd9dd4a24bdc557b77e58f61c (run 28968403182) matches a real MLflow trace in experiment 11, so L1→OTLP→backend works. Not MLflow-locked (generic otlptracehttp + standard OTEL env).

The gap is enablement: the reusable workflow declares/consumes the headers secret, but callers never pass it, so the “set the org secret” path doesn’t work after merge.

Findings not attachable to this diff

  • [high][api-contract] Thread OTEL_EXPORTER_OTLP_TRACES_HEADERS through:
    • .github/workflows/reusable-dispatch.yml (triage job secrets:)
    • internal/scaffold/fullsend-repo/.github/workflows/triage.yml
    • Existing .fullsend callers until scaffold sync
      GHA secrets do not auto-inherit. Endpoint via vars.* works; headers silently empty → 401 on authenticated backends.
  • [medium][protected-path] .github/workflows/reusable-triage.yml — human approval still required.

Ask before merge

  1. Thread the headers secret through dispatch + scaffold (see inline on the consumer).
  2. Align the GHA docs example with the real vars/secrets names; note triage-only + crash-no-export.

Comment thread .github/workflows/reusable-triage.yml
Comment thread docs/guides/infrastructure/distributed-tracing.md
Comment thread .github/workflows/reusable-triage.yml
Comment thread docs/guides/infrastructure/distributed-tracing.md Outdated
Comment thread docs/guides/infrastructure/distributed-tracing.md
Comment thread internal/telemetry/otlp/replay.go
Comment thread internal/telemetry/otlp/otlp.go
GHA reusable workflows do not inherit secrets, so the secret consumed by
reusable-triage.yml must be forwarded at every hop: the per-repo chain
(shim -> reusable-dispatch -> reusable-triage) and the per-org thin
caller. TestOTELHeadersSecretThreading guards all four links — the
alignment test only enforces required secrets, and this optional one
arrives silently empty when a forward is missing.

Signed-off-by: Dharit Shah <dhshah@redhat.com>
@ralphbean
ralphbean enabled auto-merge July 10, 2026 18:25
@ralphbean

Copy link
Copy Markdown
Member

🥳 thanks for this @dhshah13 !!

@ralphbean
ralphbean added this pull request to the merge queue Jul 10, 2026
Merged via the queue into fullsend-ai:main with commit 9feeb1e Jul 10, 2026
27 of 30 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 10, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 6:38 PM UTC · Completed 6:47 PM UTC
Commit: 01a7342 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

PR #3903 implemented Level 2 OTLP trace export (1796 additions, 17 files). The fullsend review agent approved with only 1 low finding, but human reviewer ascerra subsequently found 1 high-severity API contract issue (GHA secrets not threaded through reusable workflow callers), 2 medium-severity documentation accuracy issues, and 3 low-severity documentation completeness issues. The most impactful miss was the secret threading gap: reusable-triage.yml consumed secrets.OTEL_EXPORTER_OTLP_TRACES_HEADERS but no caller forwarded it, meaning authenticated OTLP backends would silently 401. Three proposals filed to close the gap: broadening AGENTS.md workflow sync guidance, adding explicit GHA secret threading checks to the correctness sub-agent, and adding doc-code variable alignment checks to the docs-currency sub-agent.

Proposals filed

@github-actions
github-actions Bot deleted the feat/2862-level2-otlp branch August 16, 2026 03:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: implement Level 2 distributed tracing — OTLP export to a backend

4 participants