Skip to content

feat: add OpenTelemetry bridge for AI observability - #306

Open
posthog[bot] wants to merge 12 commits into
mainfrom
posthog-self-driving/featposthog-go-add-otel-bridge-for-ai-c11a1b
Open

feat: add OpenTelemetry bridge for AI observability#306
posthog[bot] wants to merge 12 commits into
mainfrom
posthog-self-driving/featposthog-go-add-otel-bridge-for-ai-c11a1b

Conversation

@posthog

@posthog posthog Bot commented Aug 28, 2026

Copy link
Copy Markdown

💡 Motivation and Context

  • Go teams building agent backends had no supported path to PostHog AI observability. Over the last 30 days, not a single $ai_* event arrived with $lib = posthog-go, which matched the code: grepping the SDK for $ai_generation, $ai_span, or anything llm-shaped returned nothing. Those users got a flat "no" on a product they were otherwise ready to pay for.
  • The ingestion side already worked. posthog-python ships a generic OpenTelemetry bridge that keeps gen_ai./llm./ai./traceloop. spans and forwards them to /i/v0/ai/otel. Google's Agent Development Kit (ADK) for Go emits exactly those gen_ai.* spans — there was just nothing on the Go side pointing at them.

This PR ports that bridge to Go.

  • New github.com/posthog/posthog-go/otel module (posthogotel):
    • SpanProcessor — recommended integration; register it on a TracerProvider. Filters, batches, and exports AI spans.
    • Exporter — for frameworks that accept only a span exporter.
    • IsAISpan — the shared prefix filter (gen_ai., llm., ai., traceloop.), matched against the span name and every attribute key.
    • Both target {host}/i/v0/ai/otel over OTLP/HTTP with Authorization: Bearer <project api key>; WithHost switches cloud region.
  • example/ wires the processor to the tracer provider an ADK Go agent emits through.

Why a separate Go module

The core SDK today has a lean dependency graph. Folding the OTel SDK, otlptracehttp, and their protobuf/grpc transitive tree into the root go.mod would burden every posthog-go consumer for an optional feature. A nested module keeps those dependencies opt-in, so only users who import .../otel pull them.

💚 How did you test it?

  • Unit tests over IsAISpan: name-prefix match, attribute-key match, and a non-AI negative, using real ReadOnlySpans from a SpanRecorder.
  • End-to-end tests against an httptest OTLP server that decodes the protobuf payload and asserts:
    • only the AI span (gen_ai.chat) reaches the endpoint when an AI and a non-AI span are emitted together, for both SpanProcessor and Exporter;
    • the request path is /i/v0/ai/otel and the header is Authorization: Bearer <key>;
    • a batch of only non-AI spans produces no HTTP request.
  • Empty-API-key rejection and WithHost normalisation (blank falls back to default, trailing slash trimmed).
  • go build, go vet, and go test -race pass in the module. Added an otel-bridge CI job (Go 1.25 and 1.x), since the root ./... does not reach a nested module.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

The changeset release workflow versions the root module and this PR does not touch it. The otel module versions independently through otel/vX.Y.Z git tags, so no changeset is added.

🤖 Agent context

Autonomy: Fully autonomous

  • Authored by Claude (via PostHog Desktop) from an inbox report about a customer asking for ADK Go support in AI observability.
  • Read the posthog-python posthog/ai/otel/ bridge (spans.py, exporter.py, processor.py) as the reference and ported its behaviour.
  • Chose a separate Go module over inline .go files in the root package: the report suggested following the error_tracking*.go pattern, but that product added no third-party dependencies, whereas the OTel bridge pulls in grpc/protobuf. Isolating it keeps the core SDK lean.
  • Pinned OpenTelemetry to v1.43.0 and the bridge module to Go 1.25 to avoid reachable vulnerabilities in older OTel releases; the separate core module remains on Go 1.21.
  • Kept the example credential-free and runnable: real ADK wiring needs model credentials and a heavy dependency, so the example registers the processor on the tracer provider ADK Go emits through and emits one representative gen_ai.* span, with the ADK hookup documented.

Created with PostHog Desktop from this inbox report.

Adds a posthogotel Go module that forwards OpenTelemetry AI spans to PostHog
AI observability. It keeps only spans that follow a known AI semantic
convention (gen_ai., llm., ai., traceloop.) and sends them over OTLP/HTTP to
the /i/v0/ai/otel endpoint with the project API key as a bearer token.

The bridge is a separate Go module so the core posthog-go SDK stays free of
OpenTelemetry dependencies. It ports the posthog-python OTel bridge: a
SpanProcessor for TracerProvider setups and an Exporter for frameworks that
accept only a span exporter. An example wires it to a Google ADK Go agent
through the shared tracer provider.

Generated-By: PostHog Desktop
Task-Id: 5d65718b-f73a-48c4-9c4d-0fdd2d00fa17
@posthog

posthog Bot commented Aug 28, 2026

Copy link
Copy Markdown
Author

🦔 PostHog Review reviewed this pull request

Found 3 must fix, 8 should fix, 0 consider.

Published 11 findings (view the review).

Resolved comments: 8 fixed, 3 left for you

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

posthog-go Compliance Report

Date: 2026-08-31 09:49:25 UTC
Duration: 168968ms

✅ All Tests Passed!

111/111 tests passed


Capture_V1 Tests

94/94 tests passed

View Details
Test Status Duration
Endpoint And Method.Targets V1 Endpoint 17ms
Endpoint And Method.Does Not Use Legacy Endpoints 6ms
Required Headers.Has Authorization Bearer Header 5ms
Required Headers.Has Content Type Json 6ms
Required Headers.Has Posthog Sdk Info Format 15ms
Required Headers.Has Posthog Attempt Header 6ms
Required Headers.Has Posthog Request Id 5ms
Required Headers.Has Posthog Request Timestamp 5ms
Required Headers.Has User Agent 5ms
Body Format.Body Has Created At And Batch 5ms
Body Format.No Api Key In Body 5ms
Body Format.No Sent At In Body 5ms
Event Format.Event Has Required Root Fields 5ms
Event Format.Event Uuid Is Valid 15ms
Event Format.Event Timestamp Is Rfc3339 16ms
Event Format.Distinct Id Is String 6ms
Event Format.Distinct Id At Root Not Properties 4ms
Event Format.Custom Properties Preserved 5ms
Event Format.Set Properties Preserved 5ms
Event Format.Set Once Properties Preserved 4ms
Event Format.Groups Properties Preserved 6ms
Event Format.Sdk Generates Uuid If Not Provided 4ms
Event Format.Event Has Required Root Fields Batch 6ms
Event Format.Event Uuid Is Valid Batch 7ms
Event Format.Event Timestamp Is Rfc3339 Batch 7ms
Event Format.Distinct Id Is String Batch 7ms
Event Format.Distinct Id At Root Not Properties Batch 6ms
Event Format.Custom Properties Preserved Batch 7ms
Event Format.Set Properties Preserved Batch 7ms
Event Format.Set Once Properties Preserved Batch 6ms
Event Format.Groups Properties Preserved Batch 7ms
Event Format.Sdk Generates Uuid If Not Provided Batch 7ms
Batch Behavior.Multiple Events In Single Batch 28ms
Batch Behavior.Batch Envelope Smoke 8ms
Batch Behavior.Flush With No Events Sends Nothing 3ms
Batch Behavior.Flush At Triggers Batch 1006ms
Batch Behavior.Created At Reflects Batch Creation Time 6ms
Deduplication.Generates Unique Uuids 11ms
Deduplication.Different Events Same Content Different Uuids 6ms
Deduplication.Preserves Uuid On Retry 5133ms
Deduplication.Preserves Timestamp On Retry 5132ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 5135ms
Deduplication.No Duplicate Events In Batch 30ms
Header Behavior On Retry.Attempt Header Starts At One 5ms
Header Behavior On Retry.Attempt Header Increments On Retry 10138ms
Header Behavior On Retry.Request Id Preserved On Retry 5134ms
Header Behavior On Retry.Different Requests Have Different Request Ids 2011ms
Header Behavior On Retry.Request Timestamp Changes On Retry 5134ms
Response Format Validation.Success Response Has Uuid Keyed Results 6ms
Response Format Validation.Success Response Has Ok For Each Event 26ms
Response Format Validation.Success No Retry After When All Ok 27ms
Response Format Validation.Success Retry After Present When Retry Events 128ms
Response Format Validation.Success No Retry After When Drop Only 26ms
Response Format Validation.Response Echoes Request Id 5ms
Retry Behavior.Retries On 408 5133ms
Retry Behavior.Retries On 500 5133ms
Retry Behavior.Retries On 503 5135ms
Retry Behavior.Retries On 504 5133ms
Retry Behavior.Retryable Errors Have Retry After 2131ms
Retry Behavior.Respects Retry After On Retryable Error 8133ms
Retry Behavior.Does Not Retry On 400 2131ms
Retry Behavior.Does Not Retry On 401 2131ms
Retry Behavior.Does Not Retry On 402 2131ms
Retry Behavior.Does Not Retry On 413 2132ms
Retry Behavior.Does Not Retry On 415 2130ms
Retry Behavior.Non Retryable Errors Have No Retry After 2130ms
Retry Behavior.Implements Backoff 15143ms
Retry Behavior.Max Retries Respected 15144ms
Partial Batch Handling.Handles 200 Full Success 2008ms
Partial Batch Handling.Handles 200 With All Ok 3030ms
Partial Batch Handling.Does Not Retry Dropped Events 3028ms
Partial Batch Handling.Does Not Retry Limited Events 3029ms
Partial Batch Handling.Prunes Ok Events On Partial Retry 5133ms
Partial Batch Handling.Prunes Dropped Events On Partial Retry 5133ms
Partial Batch Handling.Retries Only Retry Events From Partial 5135ms
Partial Batch Handling.Partial Retry Preserves Uuids 5134ms
Partial Batch Handling.Partial Retry Attempt Header Increments 5134ms
Partial Batch Handling.Partial Retry Request Id Preserved 5132ms
Partial Batch Handling.Respects Retry After On Partial 5132ms
Partial Batch Handling.Unknown Result Treated As Terminal 3039ms
Partial Batch Handling.Mixed Ok Drop Limited No Retry 3033ms
Compression.Sends Gzip Content Encoding 6ms
Compression.No Content Encoding When Disabled 5ms
Compression.Compressed Body Is Decompressible 4ms
Error Handling.Does Not Retry On Unknown 4Xx 2131ms
Event Options.Cookieless Mode Override 6ms
Event Options.Disable Skew Correction Override 4ms
Event Options.Process Person Profile Override 5ms
Event Options.Product Tour Id Override 4ms
Event Options.Unset Options Omitted 5ms
Event Options.Options Override In Batch 6ms
Geoip And Historical Migration.Geoip Disable Injected Into Properties 5ms
Geoip And Historical Migration.Historical Migration Set In Body 4ms
Geoip And Historical Migration.Historical Migration Absent By Default 5ms

Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 14ms
Request Payload.Flags Request Uses V2 Query Param 14ms
Request Payload.Flags Request Hits Flags Path Not Decide 16ms
Request Payload.Flags Request Omits Authorization Header 14ms
Request Payload.Token In Flags Body Matches Init 15ms
Request Payload.Groups Round Trip 15ms
Request Payload.Groups Default To Empty Object 14ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 14ms
Request Payload.Disable Geoip Omitted Defaults To False 15ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 14ms
Request Lifecycle.No Flags Request On Init Alone 2ms
Request Lifecycle.No Flags Request On Normal Capture 4ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 27ms
Request Lifecycle.Mock Response Value Is Returned To Caller 14ms
Retry Behavior.Retries Flags On 502 16ms
Retry Behavior.Retries Flags On 504 14ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 16ms

@posthog
posthog Bot marked this pull request as ready for review August 28, 2026 23:50
@posthog
posthog Bot requested a review from a team as a code owner August 28, 2026 23:50
@posthog

posthog Bot commented Aug 29, 2026

Copy link
Copy Markdown
Author

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

PostHog Review

Found 3 must fix, 8 should fix.

Comment thread otel/processor.go Outdated
Comment thread otel/example/main.go Outdated
Comment thread otel/README.md
Comment thread otel/processor.go
Comment thread otel/go.mod Outdated
Comment on lines +6 to +8
go.opentelemetry.io/otel v1.28.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.28.0
go.opentelemetry.io/otel/sdk v1.28.0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Upgrade dependencies with reachable security vulnerabilities

must_fix security

Why we think it's a valid issue
  • Checked: Ran govulncheck v1.7.0 over ./... in the otel module with the module cache populated, then parsed the JSON output to separate findings by reachability level (CALLED / IMPORTED / REQUIRED). Also read the four cited advisories from vuln.go.dev, the .mod files of newer OTel SDK releases, the root go.mod, and the CI workflow.
  • Found: Two of the four cited IDs are confirmed CALLED and are fixable from this go.mod. GO-2026-4394 resolves to go.opentelemetry.io/otel/sdk — it is CVE-2026-24051, arbitrary code execution through PATH hijacking, introduced in 1.21.0 and fixed in 1.40.0, so the v1.28.0 pin at otel/go.mod:8 is in range. GO-2026-4985 resolves to go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp — the exporter does not limit the size of the HTTP response body it reads, so a large response causes memory exhaustion and process termination. The advisory lists otlptracehttp as fixed in 1.43.0, and otel/go.mod:7 pins v1.28.0.
  • Found: The x/net half of the finding is misattributed. govulncheck traces GO-2026-4918 and GO-2026-5026 to stdlib, not to golang.org/x/net. Both advisories carry a stdlib entry alongside the golang.org/x/net entry, and the reached path runs through the toolchain's own net/http. Every one of the 13 golang.org/x/net findings in this module sits at REQUIRED level, which means the module is in the graph but no vulnerable symbol is reached. Raising golang.org/x/net to v0.55.0 at otel/go.mod:22 would therefore clear neither cited ID. Those two are fixed by the consumer's Go toolchain: net/http is fixed in 1.25.10 / 1.26.3 for GO-2026-4918 and 1.25.13 / 1.26.6 for GO-2026-5026.
  • Found: The fix has a real cost the author must weigh. go.opentelemetry.io/otel/sdk@v1.40.0 declares go 1.24.0 and @v1.43.0 declares go 1.25.0. Clearing GO-2026-4985 needs 1.43.0 or newer, so otel/go.mod:3 must move from go 1.21 to go 1.25. The PR body gives Go 1.21 parity as the reason for the current pin. That parity is preserved for the core SDK either way, because the root go.mod stays at go 1.21 and this is a separate module — only users who opt into the bridge would need Go 1.25. The otel-bridge matrix at .github/workflows/unit-tests.yml:38 tests Go 1.21 and would need updating.
  • Impact: This PR introduces a new public module. Any customer who runs go get github.com/posthog/posthog-go/otel and then govulncheck gets a CALLED arbitrary-code-execution finding and a CALLED memory-exhaustion finding on day one. Many teams gate dependency adoption on exactly that scan, so the pin works against the PR's stated goal of unblocking Go teams. Because the module is new, changing the pin costs no migration — only a decision about the minimum Go version.
  • Impact: The memory-exhaustion path is not theoretical for this module. The exporter built at otel/config.go reads the response from the PostHog endpoint on every export, which is the code path the advisory describes.
  • Impact: One caveat for the suggested CI step. The same scan reports 17 CALLED stdlib findings that come from the scanning toolchain, not from this go.mod. Adding govulncheck to the otel-bridge job without scoping it would make the job permanently red for reasons the module cannot fix.
Issue description

The module pins dependencies with published vulnerabilities in reachable code. OTel SDK v1.28.0 has the PATH-hijacking flaw GO-2026-4394. The OTLP HTTP exporter has the memory-exhaustion flaw GO-2026-4985. x/net v0.26.0 also has reachable GO-2026-4918 and GO-2026-5026 paths.

Suggested fix

Upgrade all OTel packages to v1.43.0 or newer and x/net to v0.55.0 or newer. Raise the nested module's Go version or backport the upstream fixes if necessary. Add govulncheck to the otel CI job.

Prompt to fix with AI (copy-paste)
## Context
@otel/go.mod#L6-8
@otel/go.mod#L21

<issue_description>
The module pins dependencies with published vulnerabilities in reachable code. OTel SDK v1.28.0 has the PATH-hijacking flaw GO-2026-4394. The OTLP HTTP exporter has the memory-exhaustion flaw GO-2026-4985. x/net v0.26.0 also has reachable GO-2026-4918 and GO-2026-5026 paths.
</issue_description>

<issue_validation>
- **Checked:** Ran `govulncheck v1.7.0` over `./...` in the `otel` module with the module cache populated, then parsed the JSON output to separate findings by reachability level (CALLED / IMPORTED / REQUIRED). Also read the four cited advisories from `vuln.go.dev`, the `.mod` files of newer OTel SDK releases, the root `go.mod`, and the CI workflow.
- **Found:** Two of the four cited IDs are confirmed CALLED and are fixable from this `go.mod`. `GO-2026-4394` resolves to `go.opentelemetry.io/otel/sdk` — it is CVE-2026-24051, arbitrary code execution through PATH hijacking, introduced in 1.21.0 and fixed in 1.40.0, so the `v1.28.0` pin at `otel/go.mod:8` is in range. `GO-2026-4985` resolves to `go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp` — the exporter does not limit the size of the HTTP response body it reads, so a large response causes memory exhaustion and process termination. The advisory lists `otlptracehttp` as fixed in 1.43.0, and `otel/go.mod:7` pins `v1.28.0`.
- **Found:** The `x/net` half of the finding is misattributed. `govulncheck` traces `GO-2026-4918` and `GO-2026-5026` to **stdlib**, not to `golang.org/x/net`. Both advisories carry a `stdlib` entry alongside the `golang.org/x/net` entry, and the reached path runs through the toolchain's own `net/http`. Every one of the 13 `golang.org/x/net` findings in this module sits at REQUIRED level, which means the module is in the graph but no vulnerable symbol is reached. Raising `golang.org/x/net` to v0.55.0 at `otel/go.mod:22` would therefore clear neither cited ID. Those two are fixed by the consumer's Go toolchain: `net/http` is fixed in 1.25.10 / 1.26.3 for `GO-2026-4918` and 1.25.13 / 1.26.6 for `GO-2026-5026`.
- **Found:** The fix has a real cost the author must weigh. `go.opentelemetry.io/otel/sdk@v1.40.0` declares `go 1.24.0` and `@v1.43.0` declares `go 1.25.0`. Clearing `GO-2026-4985` needs 1.43.0 or newer, so `otel/go.mod:3` must move from `go 1.21` to `go 1.25`. The PR body gives Go 1.21 parity as the reason for the current pin. That parity is preserved for the core SDK either way, because the root `go.mod` stays at `go 1.21` and this is a separate module — only users who opt into the bridge would need Go 1.25. The `otel-bridge` matrix at `.github/workflows/unit-tests.yml:38` tests Go 1.21 and would need updating.
- **Impact:** This PR introduces a new public module. Any customer who runs `go get github.com/posthog/posthog-go/otel` and then `govulncheck` gets a CALLED arbitrary-code-execution finding and a CALLED memory-exhaustion finding on day one. Many teams gate dependency adoption on exactly that scan, so the pin works against the PR's stated goal of unblocking Go teams. Because the module is new, changing the pin costs no migration — only a decision about the minimum Go version.
- **Impact:** The memory-exhaustion path is not theoretical for this module. The exporter built at `otel/config.go` reads the response from the PostHog endpoint on every export, which is the code path the advisory describes.
- **Impact:** One caveat for the suggested CI step. The same scan reports 17 CALLED stdlib findings that come from the scanning toolchain, not from this `go.mod`. Adding `govulncheck` to the `otel-bridge` job without scoping it would make the job permanently red for reasons the module cannot fix.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Upgrade all OTel packages to v1.43.0 or newer and x/net to v0.55.0 or newer. Raise the nested module's Go version or backport the upstream fixes if necessary. Add govulncheck to the otel CI job.
</potential_solution>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Escalating this one for a human decision rather than applying it automatically — it sits on three things an unattended fixer shouldn't decide on its own: a third-party dependency upgrade, a minimum-Go-version change, and a CI matrix edit.

The substance looks right and worth doing. Two of the four advisories are genuinely reachable (CALLED) and fixable from this module: GO-2026-4394 (arbitrary code execution via PATH hijacking) in go.opentelemetry.io/otel/sdk, fixed in 1.40.0; and GO-2026-4985 (unbounded HTTP response read → memory exhaustion) in otlptracehttp, fixed in 1.43.0 — and the exporter this module builds does read the endpoint's response on every export, so that path is real. The catch is the cost the PR already weighed the other way: OTel sdk v1.40.0 requires go 1.24 and v1.43.0 requires go 1.25, so clearing GO-2026-4985 means moving otel/go.mod from go 1.21 to go 1.25 and updating the otel-bridge CI matrix (currently Go 1.21). That only affects users who opt into the bridge — the root module stays at go 1.21 — but dropping the stated Go 1.21 parity for the bridge is a maintainer call, not a mechanical one.

The other two IDs (GO-2026-4918, GO-2026-5026) are misattributed to golang.org/x/net: govulncheck traces them to stdlib net/http, they sit at REQUIRED (not reachable) level here, and they're fixed by the consumer's Go toolchain — so bumping x/net would clear neither and isn't needed.

What a human needs to decide: (1) accept raising the bridge module's minimum to Go 1.25 (and update the CI matrix accordingly) in exchange for upgrading OTel to >= 1.43.0, or keep Go 1.21 parity and accept the two CALLED findings for now. If you want, also consider whether to scope any govulncheck CI step to this module's own findings, since the reviewer noted the raw scan reports toolchain stdlib findings the module can't fix. I've left the code untouched.

Comment thread otel/config.go
Comment on lines +52 to +57
func newOTLPExporter(ctx context.Context, apiKey string, cfg config) (sdktrace.SpanExporter, error) {
return otlptracehttp.New(ctx,
otlptracehttp.WithEndpointURL(cfg.host+ingestPath),
otlptracehttp.WithHeaders(map[string]string{
"Authorization": "Bearer " + apiKey,
}),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Normalize the API key before using it

should_fix bug

Why we think it's a valid issue
  • Checked: Ran a throwaway test inside the module that builds a SpanProcessor against an httptest server for five key shapes and records what the server actually receives, then traced the server-side bearer parsing in rust/capture/src/otel/mod.rs and rust/capture/src/token.rs. The temporary test was deleted; the tree is clean.
  • Found: A newline in the key breaks every export, permanently. With "phc_nl\n" and with "\nphc_lead", NewSpanProcessor returned no error, and then ForceFlush failed with traces export: Post "...": net/http: invalid header field value for "Authorization". The server received no request at all — Go's net/http rejects the value before sending. The constructor's guard at otel/processor.go:22 accepts these keys precisely because strings.TrimSpace trims \n, so the check passes and the untrimmed value then goes to otel/config.go:56.
  • Found: A leading space produces a polluted token rather than a clean rejection. With " phc_pad " the server received Authorization: "Bearer phc_pad". On the server, rust/capture/src/otel/mod.rs:124-135 does auth_header.starts_with("Bearer ") and then the fixed slice &auth_header[7..], so the token becomes " phc_pad". validate_token in rust/capture/src/token.rs:45-68 checks only empty, length, ASCII, phx_ prefix, and null byte — a space passes all five — and never trims. The untrimmed token flows on into CapturedEvent.token at filtering.rs:125. There is no 401 from this handler; the token simply carries whitespace into an exact-match project lookup downstream.
  • Found: The finding's wording needs one correction that matters for the tests it asks for. Trailing space and trailing tab are harmless: " phc_pad " arrived with the trailing spaces already gone, and "phc_tab\t" arrived as a clean "Bearer phc_tab" and exported successfully, because an HTTP recipient strips trailing optional whitespace. Only leading whitespace pollutes the token, and only a newline breaks the request outright. A test that pads the key with a trailing space would pass today and prove nothing.
  • Found: The module already treats whitespace as insignificant everywhere else. WithHost at otel/config.go:35-36 computes strings.TrimSpace(host) and stores the trimmed value. The key path computes the same trim at otel/processor.go:22 and otel/exporter.go:22 but discards it, passing the original to newOTLPExporter. This is an inconsistency inside one file, not a missing hypothetical guard.
  • Impact: The realistic trigger is a secret read from a file. Kubernetes secrets mounted as files and Docker secrets under /run/secrets/ routinely end with a trailing newline, and os.ReadFile preserves it. A Go service wired that way gets a constructor that reports success and then loses every span, with an error naming neither the API key nor whitespace. That is a total failure with a misleading diagnostic.
  • Impact: The current state is the one state that should not ship: the code validates a trimmed key and then uses an untrimmed one. Either fix is defensible — normalise with apiKey = strings.TrimSpace(apiKey) before use, or reject a key that is not already trimmed with a clear error. Both are a line per constructor and both remove the accept-then-fail-opaquely behaviour.
Issue description

The constructors use TrimSpace only for the empty check. They pass the original key into the Authorization header. A key with surrounding whitespace can produce an invalid token or header and make every export fail.

Suggested fix

Assign apiKey = strings.TrimSpace(apiKey) before validation and pass the normalized value. Add constructor tests for leading and trailing whitespace.

Prompt to fix with AI (copy-paste)
## Context
@otel/config.go#L52-57

<issue_description>
The constructors use TrimSpace only for the empty check. They pass the original key into the Authorization header. A key with surrounding whitespace can produce an invalid token or header and make every export fail.
</issue_description>

<issue_validation>
- **Checked:** Ran a throwaway test inside the module that builds a `SpanProcessor` against an `httptest` server for five key shapes and records what the server actually receives, then traced the server-side bearer parsing in `rust/capture/src/otel/mod.rs` and `rust/capture/src/token.rs`. The temporary test was deleted; the tree is clean.
- **Found:** A newline in the key breaks every export, permanently. With `"phc_nl\n"` and with `"\nphc_lead"`, `NewSpanProcessor` returned no error, and then `ForceFlush` failed with `traces export: Post "...": net/http: invalid header field value for "Authorization"`. The server received **no request at all** — Go's `net/http` rejects the value before sending. The constructor's guard at `otel/processor.go:22` accepts these keys precisely because `strings.TrimSpace` trims `\n`, so the check passes and the untrimmed value then goes to `otel/config.go:56`.
- **Found:** A leading space produces a polluted token rather than a clean rejection. With `"  phc_pad  "` the server received `Authorization: "Bearer   phc_pad"`. On the server, `rust/capture/src/otel/mod.rs:124-135` does `auth_header.starts_with("Bearer ")` and then the fixed slice `&auth_header[7..]`, so the token becomes `"  phc_pad"`. `validate_token` in `rust/capture/src/token.rs:45-68` checks only empty, length, ASCII, `phx_` prefix, and null byte — a space passes all five — and never trims. The untrimmed token flows on into `CapturedEvent.token` at `filtering.rs:125`. There is no 401 from this handler; the token simply carries whitespace into an exact-match project lookup downstream.
- **Found:** The finding's wording needs one correction that matters for the tests it asks for. Trailing space and trailing tab are harmless: `"  phc_pad  "` arrived with the trailing spaces already gone, and `"phc_tab\t"` arrived as a clean `"Bearer phc_tab"` and exported successfully, because an HTTP recipient strips trailing optional whitespace. Only **leading** whitespace pollutes the token, and only a **newline** breaks the request outright. A test that pads the key with a trailing space would pass today and prove nothing.
- **Found:** The module already treats whitespace as insignificant everywhere else. `WithHost` at `otel/config.go:35-36` computes `strings.TrimSpace(host)` and stores the trimmed value. The key path computes the same trim at `otel/processor.go:22` and `otel/exporter.go:22` but discards it, passing the original to `newOTLPExporter`. This is an inconsistency inside one file, not a missing hypothetical guard.
- **Impact:** The realistic trigger is a secret read from a file. Kubernetes secrets mounted as files and Docker secrets under `/run/secrets/` routinely end with a trailing newline, and `os.ReadFile` preserves it. A Go service wired that way gets a constructor that reports success and then loses every span, with an error naming neither the API key nor whitespace. That is a total failure with a misleading diagnostic.
- **Impact:** The current state is the one state that should not ship: the code validates a trimmed key and then uses an untrimmed one. Either fix is defensible — normalise with `apiKey = strings.TrimSpace(apiKey)` before use, or reject a key that is not already trimmed with a clear error. Both are a line per constructor and both remove the accept-then-fail-opaquely behaviour.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Assign apiKey = strings.TrimSpace(apiKey) before validation and pass the normalized value. Add constructor tests for leading and trailing whitespace.
</potential_solution>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This is a real bug, but it touches how the project API key credential is handled and how the Authorization: Bearer header is built, so per our policy on auth/secrets-handling changes I'm escalating it for a human rather than applying it automatically.

What's happening: both constructors run strings.TrimSpace(apiKey) only for the empty-key check and then pass the original, untrimmed key into the header ("Bearer " + apiKey). A trailing newline — which os.ReadFile preserves on Kubernetes/Docker secret files mounted at /run/secrets/... — makes Go's net/http reject the header value, so the constructor reports success and then every export fails with invalid header field value for "Authorization", an error that names neither the key nor whitespace. Leading whitespace instead yields a polluted "Bearer phc_..." token. (As the thread's own correction notes, trailing spaces/tabs are harmless — an HTTP recipient strips them — so only leading whitespace and newlines actually bite.)

What a human needs to decide: whether to normalize the key with apiKey = strings.TrimSpace(apiKey) before use (consistent with how WithHost already trims), or reject a non-trimmed key with a clear error. Either is one line per constructor and both remove the current accept-then-fail-opaquely behaviour; the choice and the sign-off on a credential-handling change should be a maintainer's. I've left the code untouched.

Comment thread otel/config.go
Comment thread otel/README.md Outdated
Comment thread otel/README.md Outdated
Comment thread otel/go.mod Outdated
Comment on lines +1 to +10
module github.com/posthog/posthog-go/otel

go 1.21

require (
go.opentelemetry.io/otel v1.28.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.28.0
go.opentelemetry.io/otel/sdk v1.28.0
go.opentelemetry.io/proto/otlp v1.3.1
google.golang.org/protobuf v1.34.2

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Add dependency updates for the nested module

should_fix best_practice

Why we think it's a valid issue
  • Checked: The repository's .github/dependabot.yml, the CI workflow this PR modifies, and the actual freshness of the pins in the new module.
  • Found: The premise is exactly right. .github/dependabot.yml declares three ecosystems — gomod, npm, and github-actions — and every one of them sets directory: "/". There is no /otel entry. Dependabot's directory points at the location of a single manifest, and each go.mod is its own manifest, so the gomod block covers only the root module and will never open a PR for the nested one.
  • Found: The suggested policy matches what already exists, so the fix is a copy of the current block with a new directory. The gomod entry uses schedule.interval: weekly, cooldown.default-days: 7, and a go-dependencies group with patterns: ["*"]. A directories: ["/", "/otel"] list on the existing block is an equally small alternative to a second entry.
  • Found: The author already solved the identical problem for the other automation. This PR adds an otel-bridge job at .github/workflows/unit-tests.yml:38-59 with working-directory: otel, and the PR body explains it exists because "the root ./... does not reach a nested module." The same reasoning applies to Dependabot, and only the CI half was done.
  • Found: The consequence is already visible in the file being flagged, not hypothetical. Running govulncheck against this module reports GO-2026-4394 — arbitrary code execution through PATH hijacking in go.opentelemetry.io/otel/sdk, fixed in 1.40.0 — and GO-2026-4985 — unbounded OTLP HTTP response reads in otlptracehttp, fixed in 1.43.0 — both at CALLED reachability against the v1.28.0 pins at otel/go.mod:6-8. The module ships stale on day one.
  • Impact: Without a Dependabot entry, nothing in the repository will ever surface the next advisory for this module's OpenTelemetry, gRPC, protobuf, or golang.org/x/net tree. The root module keeps getting weekly grouped PRs while the nested one silently drifts, and every future fix depends on somebody thinking to run a manual scan.
  • Impact: This pairs with the separate finding about upgrading the vulnerable pins, and the two should be treated as one piece of work. That one clears the current advisories; this one is the mechanism that keeps them clear. Fixing only the pins leaves the same gap open for the next release.
Issue description

The nested go.mod creates an independent dependency graph. The repository's Dependabot configuration scans only directory /. It will not update this module's OpenTelemetry and network dependencies. Security and compatibility fixes will require manual discovery.

Suggested fix

Add a gomod Dependabot entry for /otel. Use the same weekly grouping policy as the root module.

Prompt to fix with AI (copy-paste)
## Context
@otel/go.mod#L1-10

<issue_description>
The nested go.mod creates an independent dependency graph. The repository's Dependabot configuration scans only directory /. It will not update this module's OpenTelemetry and network dependencies. Security and compatibility fixes will require manual discovery.
</issue_description>

<issue_validation>
- **Checked:** The repository's `.github/dependabot.yml`, the CI workflow this PR modifies, and the actual freshness of the pins in the new module.
- **Found:** The premise is exactly right. `.github/dependabot.yml` declares three ecosystems — `gomod`, `npm`, and `github-actions` — and every one of them sets `directory: "/"`. There is no `/otel` entry. Dependabot's `directory` points at the location of a single manifest, and each `go.mod` is its own manifest, so the `gomod` block covers only the root module and will never open a PR for the nested one.
- **Found:** The suggested policy matches what already exists, so the fix is a copy of the current block with a new directory. The `gomod` entry uses `schedule.interval: weekly`, `cooldown.default-days: 7`, and a `go-dependencies` group with `patterns: ["*"]`. A `directories: ["/", "/otel"]` list on the existing block is an equally small alternative to a second entry.
- **Found:** The author already solved the identical problem for the other automation. This PR adds an `otel-bridge` job at `.github/workflows/unit-tests.yml:38-59` with `working-directory: otel`, and the PR body explains it exists because "the root `./...` does not reach a nested module." The same reasoning applies to Dependabot, and only the CI half was done.
- **Found:** The consequence is already visible in the file being flagged, not hypothetical. Running `govulncheck` against this module reports `GO-2026-4394` — arbitrary code execution through PATH hijacking in `go.opentelemetry.io/otel/sdk`, fixed in 1.40.0 — and `GO-2026-4985` — unbounded OTLP HTTP response reads in `otlptracehttp`, fixed in 1.43.0 — both at CALLED reachability against the `v1.28.0` pins at `otel/go.mod:6-8`. The module ships stale on day one.
- **Impact:** Without a Dependabot entry, nothing in the repository will ever surface the next advisory for this module's OpenTelemetry, gRPC, protobuf, or `golang.org/x/net` tree. The root module keeps getting weekly grouped PRs while the nested one silently drifts, and every future fix depends on somebody thinking to run a manual scan.
- **Impact:** This pairs with the separate finding about upgrading the vulnerable pins, and the two should be treated as one piece of work. That one clears the current advisories; this one is the mechanism that keeps them clear. Fixing only the pins leaves the same gap open for the next release.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Add a gomod Dependabot entry for /otel. Use the same weekly grouping policy as the root module.
</potential_solution>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Escalating this to sit alongside the related dependency-upgrade thread, rather than applying it automatically. The premise is correct: .github/dependabot.yml has a single gomod block at directory: "/" and no /otel entry, so Dependabot only ever updates the root module — the nested otel module's OpenTelemetry, gRPC, protobuf, and x/net tree will silently drift and every future advisory there depends on someone running a manual scan.

Why a human should take this one: it's repository dependency-automation policy, and it's tightly coupled to the still-open decision on upgrading this module's vulnerable pins. A /otel Dependabot entry would immediately open PRs bumping go.opentelemetry.io/otel/sdk and otlptracehttp to their latest (1.40+/1.43+), which require Go 1.24/1.25 and clash with the module's intentional go 1.21 pin — so whether those grouped bumps are even mergeable, and whether you want ignore rules for the Go-version-raising majors, depends on how you resolve the minimum-Go question. There are also two equally clean ways to write it that are a maintainer's call: a second gomod entry with directory: "/otel", or switching the existing block to directories: ["/", "/otel"]. Either mirrors the current weekly / 7-day-cooldown / go-dependencies group policy. I've left the config untouched so you can decide the shape together with the pin upgrade.

posthog Bot added 8 commits August 29, 2026 00:22
The PostHog AI observability OTLP endpoint rejects requests carrying more
than 100 spans with a non-retryable HTTP 400, which drops the whole batch.
NewSpanProcessor used the OTel SDK default batch size (512), so any window
producing more than 100 AI spans silently lost data.

Cap the batch span processor at a shared maxSpansPerRequest (100) constant
and add a regression test that emits more than 100 AI spans and asserts no
export request exceeds the limit.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
The example queued the span and immediately logged "sent AI span to
PostHog", then relied on the deferred Shutdown to flush. The batch span
processor's Shutdown returns only context errors, so a rejected export
(bad API key or host) never surfaced and the program printed success and
exited 0 regardless.

Call provider.ForceFlush before the success message so the export error is
surfaced and reported, and print the success line only once the export is
confirmed. On failure the program returns rather than exiting hard so the
deferred Shutdown still runs.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
The README's ADK Go section implied a complete integration ("no further
code"). The bridge forwards spans only, but ADK Go emits prompt and
response message content as OpenTelemetry log records, not span attributes,
so those fields arrive empty in PostHog. Document the gap so users do not
read the empty prompt/response as a broken integration.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
The PostHog AI Gateway captures its own $ai_generation on every routed
call. A service that both routes through the gateway and exports spans
through this bridge double-counts and double-bills every generation, with
no signal that it is happening.

The sibling posthog-python and posthog-js SDKs guard against this with a
warning; the Go port dropped it. Port the guard: SpanProcessor.OnEnd now
inspects the AI span's server.address / url.full attributes against the
known gateway hosts and logs a warning (matching the siblings' plain-warn
behaviour, never dropping the span). Host list and detection logic mirror
posthog-python's posthog/ai/gateway.py.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
otlptracehttp.WithEndpointURL swallows a URL parse error: it logs and
keeps its localhost:4318 default, so a malformed WithHost value produces a
working-looking processor that never reaches PostHog while the request
still carries the Authorization header. A scheme-less host such as
"us.i.posthog.com" parses but leaves the endpoint empty. Both are silent
data loss reachable straight from the documented POSTHOG_ENDPOINT path.

Validate the resolved host in newConfig and return an error from
NewSpanProcessor / NewExporter, completing the input validation the module
already performs on the API key. The host must be an absolute http or https
URL with a hostname; query and fragment are left alone as the review
scoped.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
The PostHog AI observability endpoint rejects requests carrying more than
100 spans with a non-retryable HTTP 400 that discards the whole batch.
Capping the SpanProcessor's batch size covered only that entry point; the
caller-supplied Exporter path (wired with its own batch processor, default
512) could still hand an oversized slice to a single request, because
nothing below this module splits a batch.

Wrap the OTLP exporter in a chunkingExporter that splits each ExportSpans
call into slices of at most maxSpansPerRequest. This lives in
newOTLPExporter, so it protects both the SpanProcessor and the Exporter
regardless of the feeding processor's batch size. Adds a 101+-span
regression test for the Exporter path.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
The README's usage snippet reused the construction context for the
deferred provider.Shutdown and discarded its error. When that context is
already canceled at shutdown — the standard signal.NotifyContext graceful
shutdown idiom — the OTel SDK returns ctx.Err() before running the batch
processor's final export, so every buffered AI span is dropped with no
error surfaced. This is the module's primary copy-paste path and it
contradicted the fresh-timeout-context pattern the shipped example already
uses. Mirror the example: a fresh context.WithTimeout and a reported error.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
The usage snippet built a new sdktrace.TracerProvider and made it global.
For the flagship case — an ADK Go agent inside a service that already has
OpenTelemetry configured — this loses AI spans: the OTel global delegation
fires only once, so an already-installed provider's tracers (including
ADK's, cached at package init) stay bound to it and never reach the PostHog
processor, while the replacement provider also drops the user's resource,
sampler, and existing exporters. The README prose already said "register it
on your TracerProvider", which the code contradicted.

Show provider.RegisterSpanProcessor(processor) on the application's own
*sdktrace.TracerProvider, and shut down the processor (not the provider) so
PostHog's setup does not tear down the user's tracing pipeline. Keeps the
fresh-context, error-reported shutdown from the previous fix.

Generated-By: PostHog Desktop
Task-Id: 01e1bd45-4478-4317-8196-8ddfcfe917bf
@marandaneto
marandaneto requested a review from a team August 31, 2026 07:05
@marandaneto marandaneto self-assigned this Aug 31, 2026
@marandaneto

marandaneto commented Aug 31, 2026

Copy link
Copy Markdown
Member

Live US Cloud validation succeeded for the expanded example: the span was ingested as $ai_generation, and trace/span IDs, provider, model, messages, token counts, latency, base URL, and calculated costs were all populated as expected.

cc @PostHog/team-ai-observability
One caveat for reviewers: gen_ai.response.finish_reasons and gen_ai.response.id remain raw OpenTelemetry properties rather than being normalized to $ai_stop_reason and $ai_generation_id. The Go bridge is exporting the standard GenAI attributes correctly; this is a generic PostHog OTel ingestion normalization gap, not a Go bridge issue.

@marandaneto

Copy link
Copy Markdown
Member

ingested event example

{
  "createdAt": "2026-08-31T09:44:37.958000+00:00",
  "event": "$ai_generation",
  "id": "01a05734-e115-7103-8a19-54b4da519853",
  "properties": {
    "$ai_ingestion_source": "otel",
    "$ai_span_id": "3d0d99fac071acbe",
    "$ai_trace_id": "b46ea40bb3555a8eb0be877729a2013b",
    "$geoip_disable": true,
    "example.run_id": "20260831T094437.958644000Z",
    "gen_ai.response.finish_reasons": [
      "stop"
    ],
    "gen_ai.response.id": "chatcmpl-posthog-go-example",
    "service.name": "posthog-go-otel-example",
    "$ip": "84.115.233.26",
    "$ai_input_tokens": 18,
    "$ai_output_tokens": 11,
    "$ai_model": "gpt-4o-mini-2024-07-18",
    "$ai_provider": "openai",
    "$ai_base_url": "api.openai.com",
    "$ai_span_name": "chat posthog-go OTel example",
    "$ai_latency": 0.051059583,
    "$ai_cache_reporting_exclusive": false,
    "$ai_input_cost_usd": 0.0000027,
    "$ai_output_cost_usd": 0.0000066,
    "$ai_request_cost_usd": 0,
    "$ai_web_search_cost_usd": 0,
    "$ai_total_cost_usd": 0.0000093,
    "$ai_model_cost_used": "openai/gpt-4o-mini-2024-07-18",
    "$ai_cost_model_source": "openrouter",
    "$ai_cost_model_provider": "openai",
    "$ai_input": [
      {
        "role": "system",
        "content": "Answer concisely."
      },
      {
        "role": "user",
        "content": "What is PostHog?"
      }
    ],
    "$ai_output_choices": [
      {
        "role": "assistant",
        "content": "PostHog is an open-source product analytics platform."
      }
    ]
  },
  "sentiment": null
}

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.

1 participant