OTLP browser telemetry export: follow-up improvements (KERNEL-1596)#317
Draft
archandatta wants to merge 9 commits into
Draft
OTLP browser telemetry export: follow-up improvements (KERNEL-1596)#317archandatta wants to merge 9 commits into
archandatta wants to merge 9 commits into
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Export is off by default and gated on the toggle, independent of whether an endpoint is provisioned, so an always-injected relay does not export by default. Writer lifecycle moves under a runtime-toggleable controller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Surfaces queue-overflow drops (previously log-only) plus export failures and exported counts as Prometheus counters, owned above the writer so they stay monotonic across export toggles. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Detects Inspector.targetCrashed (an 'Aw, Snap!' renderer crash that leaves the browser process alive, so no service_crashed fires) and emits a page_crashed event, mapped to ERROR severity in the OTLP converter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Exercises the export path end-to-end against a real dockerized collector: severity mapping, category exclusion, promoted attributes, export counters, and a forced queue overflow proving the drop metric. Run with -tags livetest. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…, crash guard - Drop 'on the VM' from the public export spec descriptions. - Thread request ctx into export reconcile; log via the context logger, and drain with context.WithoutCancel so a toggle-off outlives the request. - Still emit page_crashed (and warn) if a crash fires on an untracked session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… test Forces a real batch-queue overflow against a local hung listener so CI exercises the SDK->global-logger->counter chain every run; an SDK message rename now fails here instead of silently zeroing the metric. Removes the build-tagged docker live test (its coverage is now unit-side). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
tldr
Follow-up improvements to the in-VM OTLP browser-telemetry exporter (KERNEL-1596), post #308. Independent, reviewable commits. Off-by-default export is preserved; nothing changes for existing sessions unless the new toggle/knobs are set.
What changed
BTEL_OTLP_MAX_QUEUE_SIZE(2048),BTEL_OTLP_EXPORT_INTERVAL(1s),BTEL_OTLP_EXPORT_TIMEOUT(30s). Defaults match the OTel SDK's, so behavior is unchanged unless set; validated as > 0.export.otlp.enabledon the telemetry API. Export is off by default and gated on the toggle, independent of whether an export destination is configured, so an always-injected relay does not generate work that gets dropped. The writer moves under a runtime-toggleable controller; toggling is best-effort (never fails a telemetry apply).kernel_otlp_records_dropped_total,_export_failures_total,_records_exported_total), owned above the writer so they stay monotonic across export toggles.service_crashedandsystem_oom_killnow map to ERROR in the OTLP converter (were INFO), so consumers can alert on them.Inspector.targetCrashed(an "Aw, Snap!" renderer crash that leaves the browser process alive, so noservice_crashedfires) and emit apage_crashedevent, mapped to ERROR.Scope notes
main(which already has Add in-VM OTLP telemetry export sink #308 + the metrics endpoint), keepingmain's existing export auth wiring; thex-api-keyswap remains Send instance name as x-api-key to the OTLP relay #316's concern.Generated code (
oapi.go)The
oapi.godiff looks large but decomposes as: ~727 lines are theembedded-specgzip blob (the whole spec is embedded, so any spec change rewrites it — same as #308), ~105 lines are the genuinely new generated types (page_crashed+ export config + union accessors), and 4 lines are an oapi-codegen naming side-effect: adding a second standalonepage-category event renames the existingBrowserPageTabOpenedEventCategoryPageconst toPage. That constant isn't referenced anywhere in-repo, and regeneratingmain's own spec with the pinned toolchain reproducesmain'soapi.goexactly (0 diff), so this is faithfulmake oapi-generateoutput, not hand-editing or transitive drift.Testing
go build ./...,go vet ./..., and the full unit suite (22 packages) pass.Inspector.targetCrashedhandler.dropped=1980.Review fixes applied
context.WithoutCancelso a toggle-off outlives the request.page_crashedstill emits (and warns) if a crash fires on an untracked session.🤖 Generated with Claude Code