feat: Send spans to an OTLP collector over HTTP - #91
Merged
Conversation
`OtlpExporter` now does what it has always claimed. Spans are POSTed to `/v1/traces` as OTLP/HTTP with JSON encoding, using `urllib` from the standard library, so no dependency is added -- gRPC would have pulled in `grpcio` and `protobuf` for a project that carries neither. The endpoint is used as given; only a missing path is filled in. The gRPC port 4317 is not rewritten to the HTTP one: quietly changing what the caller passed would break anyone serving OTLP/HTTP elsewhere, and quiet is what #88 was about. A connection failure to 4317 says so in the log instead. `flush()` returns False and logs the reason on failure, keeping the batch so the events can be retried rather than lost, and never raises -- exporters run from event callbacks, and a collector being down should not stop a trace session. `shutdown()` flushes rather than discarding, now that flushing means something. Two bugs fixed on the way, neither of which the existing tests could see. The JSON encoding sent enum *names*. OTLP is explicit: "only integer enum values are allowed in OTLP JSON Protobuf Encoding; the enum name strings MUST NOT be used". `kind` was "INTERNAL" and `status.code` was "OK"; both are now 1, so a collector will accept them. `event_to_span` crashed on every real event. `EtwEvent.timestamp` is an RFC 3339 string with nanosecond precision, and the code called `float()` on it. Every test used a mock or a plain number, so this only appeared when events from an actual ETL file were exported. `datetime.fromisoformat` cannot take nine fractional digits either, so the fraction is trimmed first. Closes #90 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HhPcm483PrDmuQEUSALBFn
This was referenced Sep 6, 2026
Merged
m96-chan
added a commit
that referenced
this pull request
Sep 6, 2026
OTLP export works for the first time. #91 gave OtlpExporter an actual transport -- OTLP/HTTP with JSON encoding over the standard library -- so the feature announced in v3.0.0 and shipped as a no-op is finally real. Minor rather than patch: `flush()` sends where it previously raised, which is new behaviour rather than a fix to existing behaviour, even though the version before it did nothing useful. Claude-Session: https://claude.ai/code/session_01HhPcm483PrDmuQEUSALBFn Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
5 tasks
m96-chan
added a commit
that referenced
this pull request
Sep 6, 2026
`event_to_span`, `event_to_log` and `SpanMapper` read events with `getattr`
only, so a `dict` got the default for every field. It produced a span named
"unknown.0" with no provider and no PID -- no error, no warning, just wrong:
event_to_span({"provider_name": "Microsoft-Windows-Kernel-Process",
"event_id": 1, "process_id": 4104})
# name='unknown.0', etw.provider='unknown', process.pid=0
`pyetwkit.export` has accepted both shapes all along, via `_event_to_dict`, so
the same dict exported to JSON perfectly well while OTLP silently emptied it.
Read fields through one helper that handles a Mapping or an object, and reject
anything that is neither with a TypeError rather than returning a span of
defaults. A partial event is still fine: absent fields fall back, because that
is a different thing from not being an event.
Found while implementing #91: the tests in this repo passed dicts, so they had
been asserting against spans of default values without anyone noticing. Those
were switched to SimpleNamespace at the time; this fixes the library instead.
Closes #92
Claude-Session: https://claude.ai/code/session_01HhPcm483PrDmuQEUSALBFn
Co-authored-by: Claude Opus 5 (1M context) <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.
Summary
OtlpExporternow does what it has always claimed. Spans are POSTed to/v1/tracesas OTLP/HTTP with JSON encoding, usingurllibfrom the standard library.No new dependency. gRPC would have pulled in
grpcioandprotobuffor a project that carries neither; OTLP/HTTP+JSON needs nothing.Changes
flush()sends, returnsTrueon 2xx,Falsewith a logged reason otherwise, and keeps the batch on failure so events can be retried rather than lost. It never raises — exporters run from event callbacks, and a collector being down should not stop a trace session.shutdown()flushes rather than discarding, now that flushing means something.Related Issue
Closes #90
Two bugs found on the way
Neither was visible to the existing tests, both would have broken this in production.
The JSON encoding sent enum names. The OTLP spec is explicit:
kindwas"INTERNAL"andstatus.codewas"OK". Both are now1, so a collector will accept them. (I had this backwards when I filed #90 — I assumedSPAN_KIND_INTERNALwas correct. Reading the spec rather than trusting the assumption is what caught it.)event_to_spancrashed on every real event.EtwEvent.timestampis an RFC 3339 string; the code calledfloat()on it. Every existing test used aMagicMockor a plain number, so this only appeared the moment events from an actual ETL file were exported.datetime.fromisoformatcannot take nine fractional digits either on 3.10, so the fraction is trimmed first.Test Plan
Added/Modified Tests
New
tests/test_otlp_transport.py: 17 tests that run a real HTTP server from the standard library and assert on what was actually received. No mocking of the send — this exporter shipped for months returning success with no transport at all, which no amount of mocking would have caught.Covered: the path and content type, the
resourceSpans/scopeSpans/spansenvelope, resource attributes, integer enums, hex trace/span ids, custom headers, an endpoint that already has a path, batch clearing,export_batch, auto-flush atbatch_size, ISO 8601 / datetime / unparseable timestamps, events read fromsample.etl, 503 handling with batch retention, an unreachable collector, andshutdownfrom a dead collector.Note the test events are
SimpleNamespace, not dicts: the exporter reads events by attribute, so a plain dict silently produces a span of defaults — which is what the pre-existing tests in this repo were unknowingly asserting against.The two
NotImplementedErrortests from #88 are updated to assertFalse. Their intent is unchanged — do not claim delivery — only the mechanism moved.Test Results
Driven by hand against real ETW events from the committed capture, with a real collector on the other end:
SpanMappernaming works, resource attributes arrive, and the enums are integers.Checklist
Additional Notes
Not done, deliberately: retries and backoff. The batch is kept on failure so a caller can retry, which is the mechanism; a policy on top of it is a separate decision. Also not done: gRPC and protobuf encoding, both of which need dependencies and would suit an extra if anyone asks.
Worth knowing:
event_to_spanreads events by attribute only, so passing a plaindictyields a span of default values rather than an error.export.pyhandles both shapes via_event_to_dict. Out of scope here, but it is a trap, and I can file it if you want it tracked.This lands after v3.1.0, so it belongs in the next release. The README changelog entry for v3.0.0 still notes that the transport was not implemented until v3.2.0 — worth confirming that is the version you want before cutting it.
🤖 Generated with Claude Code
https://claude.ai/code/session_01HhPcm483PrDmuQEUSALBFn