telemetry: remove span event emission from Recorder.Info and Recorder.Error
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 76/100
- Issue type
- Refactor
- Clarity
- Clearly specified
- Activity status
- Quiet
- Tech stack
- go
- Domain
- observability
Research direction
Start in the enginekit/telemetry package at the Recorder.Info and Recorder.Error entry points. Remove their span event emission while preserving Error's span error recording and status update, the log records, and counters. Done means Info and Error no longer add span events and the telemetry package tests still pass.
Written by the indexing model from the issue text.
Description
Background
While designing observability for runkit I reviewed enginekit/telemetry in detail against what we need. Recorder.Info and Recorder.Error make one opinionated choice that is wrong for our use case -- and is probably wrong for persistencekit too.
Problem
Info and Error emit span events
Recorder.Info and Recorder.Error both call span.AddEvent(event) on the active span in addition to emitting an OTel log record.
This is unnecessary coupling. Log records already carry trace_id and span_id as first-class OTel log fields, which is sufficient for log-trace correlation in any compliant backend. Span events serve a different purpose -- annotating a span's timeline -- and that decision belongs to the caller, not the logger.
In practice, persistencekit calls Recorder.Info on every successful operation (e.g. journal.get.ok). Under load that's a large volume of span events that add no value.
Fix: remove span event emission from Info and Error entirely.
What to keep
- Auto-counters on
StartSpan(operations,operations_in_flight) -- these are sampling-independent, which means accurate operation throughput metrics at any trace sample rate. Worth keeping. Errormarking the active span (span.RecordError+span.SetStatus(codes.Error)) -- correct and desirable. If you're logging an error, the span should be marked as errored.- Auto error counter on
Error-- same reasoning as operation counters. - Everything else in the package (
Provider,Attrconstructors,Instrument,NewSLogProvider,NewTestProvider) -- all fine as-is.
Impact on persistencekit
persistencekit uses Recorder.StartSpan, Info, Error, and metric instruments. The only behavioural change it would see is Info no longer emitting span events -- which is a net improvement.
- Dominant language
- Go
- Stars
- 0
- Forks
- 0
- Avg merge
- 7m
- Merged PRs (30d)
- 6
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from dogmatiq/enginekit
-
Difficulty 1/5 Under an hour Newbie friendliness 68/100
-
Difficulty 3/5 1-2 days Newbie friendliness 40/100
-
Difficulty 5/5 Over a week Newbie friendliness 15/100
All issues in dogmatiq/enginekit
Similar issues
-
Difficulty 1/5 Under an hour Newbie friendliness 84/100
-
enhancement needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
kind/cleanup
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
kubernetes-sigs/kueue#15947 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
sympozium-ai/sympozium#627 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100