Repository navigation
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
In the affected MsgPack configuration, _time is stored only in a temporary map, so the final Span write regenerates it and can emit a timestamp different from the Stop event’s value.
1 open issue
What changed in this PR
This PR reduces allocations and copying in the header-only ETW span export path while retaining owned properties for environment-property encoding and materializing properties for MessagePack.
Changes:
- Borrow span attributes and merge generated fields in sorted order for TLD encoding.
- Remove an extra attribute copy, reuse matching ID strings, and reserve initial encoding-buffer capacity.
- Add tests for attribute ownership, merged ordering, override precedence, and ID reuse and repair.
| File | Description |
|---|---|
| exporters/etw/test/etw_tracer_test.cc | Tests attribute ownership, self-assignment, and ID reuse and repair. |
| exporters/etw/test/etw_properties_test.cc | Tests borrowed properties, sorted merging, overrides, and empty inputs. |
| exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h | Uses borrowed span-end properties and avoids redundant attribute and ID copies. |
| exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h | Supports merged property encoding, MessagePack materialization, and buffer reservations. |
| exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h | Adds a borrowed property view with owned overrides and materialization. |
Review effort: Balanced
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4761 +/- ##
=======================================
Coverage 86.62% 86.62%
=======================================
Files 530 530
Lines 21169 21169
=======================================
Hits 18335 18335
Misses 2834 2834 🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reuse bounded generated-field nodes for untracked TLD span export, co-allocate spans, and avoid unnecessary parent lookup and temporary values. Preserve timestamp bytes with fixed-width formatting and cover payload parity, reentrancy, parent selection, and timestamp edge cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fall back to local properties after the thread-local cache is destroyed. Cover unfinished thread-local span destruction with a worker-thread regression, use C++11-compatible aligned byte storage, and document the generated-field overwrite invariant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep up to eleven owning generated entries in local storage, sort before each write, and preserve complete map semantics on overflow. Share span emission/status logic with the owned MessagePack and environment-property paths. Repair IDs against the effective generated-or-saved value, including custom field-name collisions. Cover payload parity, optional fields, reentrancy, overflow and thread exit, with CMake and Bazel registrations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove the accidental public access label introduced during the inline-field refactor. Keep the implementation and benchmarked runtime paths unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lalitb
marked this pull request as ready for review
November 3, 2026 23:58
This branch has not been deployed
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
Reduce ETW span allocations by avoiding attribute copies and using local inline storage for generated fields. Speed up timestamp formatting while preserving payload representation, field ordering, and custom field-name behavior. No new configuration options or dependencies.
Benchmark: main vs PR
Full span lifecycle, including active
Scopeand cleanup, with real enabled ETW writes on Windows x64. Main:0a5bb0b; PR:a1b8205.Medians of batch averages; shared-VM results vary. Tradeoff: measured warm stack usage increases from about 3.0 KB to 3.5 KB.
Validation: 91 ETW tests, five Windows Bazel targets, C++14 payload-parity checks, and zero capture loss.