Skip to content

feat(etw): Add provider name-to-GUID resolution with manifest and TraceLogging support#3559

Open
swashtek wants to merge 10 commits into
open-telemetry:mainfrom
swashtek:etw-nametoguid-resolution
Open

feat(etw): Add provider name-to-GUID resolution with manifest and TraceLogging support#3559
swashtek wants to merge 10 commits into
open-telemetry:mainfrom
swashtek:etw-nametoguid-resolution

Conversation

@swashtek

@swashtek swashtek commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Change Summary

feat(etw): Add provider name-to-GUID resolution with manifest and TraceLogging support

What issue does this PR close?

How are these changes tested?

Tested locally with TraceLogging and Manifest based providers

Are there any user-facing changes?

Yes, enhances the config with additional option for providers

Changelog

  • Added a .chloggen/*.yaml entry
  • This PR is a chore (indicated in title)
  • This is a documentation-only PR.

@github-actions github-actions Bot added area:pipeline Rust Pipeline Related Tasks lang:rust Pull requests that update Rust code area:receiver Core and Contrib receiver nodes labels Jul 23, 2026
@swashtek swashtek added area:pipeline Rust Pipeline Related Tasks lang:rust Pull requests that update Rust code area:receiver Core and Contrib receiver nodes labels Jul 23, 2026
@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 23, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-25 21:05:16 UTC.

  • Waiting on: Author
  • Next step: Investigate required status check failures.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. If you believe this pull request is incorrectly routed as waiting on the author, comment /dashboard route:reviewers to route it from waiting on the author to waiting on reviewers. If the last refreshed time above predates your latest reply or push, the dashboard hasn't processed it yet.

@swashtek
swashtek force-pushed the etw-nametoguid-resolution branch from e01100b to eb1c699 Compare July 23, 2026 07:20
Revert Rustic Provider configuration in YAML.
@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.88%. Comparing base (cbc8dd8) to head (f68ac2e).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3559      +/-   ##
==========================================
- Coverage   86.91%   86.88%   -0.04%     
==========================================
  Files         782      782              
  Lines      319143   319391     +248     
==========================================
+ Hits       277389   277488      +99     
- Misses      41230    41379     +149     
  Partials      524      524              
Components Coverage Δ
otap-dataflow 87.94% <ø> (-0.05%) ⬇️
query_engine 89.57% <ø> (ø)
otel-arrow-go 52.45% <ø> (ø)
quiver 92.20% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@swashtek
swashtek marked this pull request as ready for review July 23, 2026 08:48
@swashtek
swashtek requested a review from a team as a code owner July 23, 2026 08:48
Copilot AI review requested due to automatic review settings July 23, 2026 08:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enhances the Windows ETW receiver configuration to accept provider names (in addition to GUIDs) by resolving names to control GUIDs at startup, using TDH’s registered-provider database where applicable and falling back to the EventSource/TraceLogging name-hash convention when appropriate.

Changes:

  • Adds per-provider kind (auto/manifest/tracelogging) to control the name→GUID resolution strategy and validates kind is not used with guid.
  • Implements provider-name resolution with a lazily memoized TDH provider-database enumeration and EventSource/TraceLogging GUID derivation fallback.
  • Updates Windows-only dependencies, expands receiver docs/config examples, adds targeted tests, and includes a changelog entry.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
rust/otap-dataflow/crates/contrib-nodes/src/receivers/etw_receiver/session.rs Implements provider name resolution (TDH enumeration + hashing fallback), caching, logging, and adds tests.
rust/otap-dataflow/crates/contrib-nodes/src/receivers/etw_receiver/mod.rs Extends config schema/docs with ProviderKind, validates config constraints, and adds validation tests.
rust/otap-dataflow/crates/contrib-nodes/Cargo.toml Adds Windows-only windows-sys dependency and wires it into the etw-receiver feature.
rust/otap-dataflow/Cargo.toml Adds workspace dependency entry for windows-sys with required Win32 features.
rust/otap-dataflow/Cargo.lock Updates the lockfile to reflect dependency graph changes.
rust/otap-dataflow/.chloggen/etw-receiver-provider-name-resolution.yaml Adds a user-facing changelog entry for the new provider name support.
Comments suppressed due to low confidence (1)

rust/otap-dataflow/crates/contrib-nodes/src/receivers/etw_receiver/mod.rs:1027

  • Per rust/otap-dataflow/AGENTS.md the repository requires every Rust test to be documented immediately above its declaration with /// Scenario: and /// Guarantees:. This newly added test is missing those required doc comments.
    #[test]
    fn validate_accepts_name_with_explicit_kind() {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread rust/otap-dataflow/crates/contrib-nodes/src/receivers/etw_receiver/session.rs Outdated
Comment thread rust/otap-dataflow/crates/contrib-nodes/src/receivers/etw_receiver/session.rs Outdated
Comment thread rust/otap-dataflow/.chloggen/etw-receiver-provider-name-resolution.yaml Outdated
… resolution strategy, move provider lookup to one-collect
@swashtek
swashtek force-pushed the etw-nametoguid-resolution branch from 12a293f to b1dcc09 Compare July 24, 2026 03:52
@swashtek swashtek changed the title feat(etw): Enhance ETW receiver to support provider name resolution feat(etw): Add provider name-to-GUID resolution with manifest and TraceLogging support Jul 24, 2026
Comment thread rust/otap-dataflow/crates/contrib-nodes/src/receivers/etw_receiver/mod.rs Outdated
// registered-provider database is enumerated at most once and shared across
// all name lookups via `registered`.
let mut registered: Option<HashMap<String, RegisteredProvider>> = None;
let resolved_providers: Vec<(Guid, u8, Option<u64>)> = config

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reject duplicate resolved GUIDs. Two entries that map to the same control GUID (identical specs, a name that hashes to an explicit guid, or two names colliding) are currently merged silently by enable_provider. Since duplicates are only knowable after resolution, add the check in spawn_etw_session right after building resolved_providers, surfaced as a config error:

let mut seen = HashSet::with_capacity(resolved_providers.len());
for (guid, _, _) in &resolved_providers {
    if !seen.insert(*guid) {
        return Err(Error::ConfigError(Box::new(
            otap_df_config::error::Error::InvalidUserConfig {
                error: format!("multiple providers resolve to the same GUID {}", format_guid(guid)),
            },
        )));
    }
}

This single check also covers textual duplicate detection (identical guid strings / (name, kind) pairs resolve identically). It still allows the intended dual-capture case (name: X + kind: manifest vs + kind: tracelogging), since those produce different GUIDs.

@swashtek swashtek Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's a legitimate bug (silent merge and wrong level/keywords), and it preserves the intended dual-capture case (same name under kind: manifest vs kind: tracelogging can lead to two distinct GUIDs).

One small adjustment : it calls format_guid(guid), but the existing format_guid lives in the encoder module, is private, and takes &[u8; 16]. I'll add a local formatter and key the set on guid.to_bytes().

Fixed in f68ac2e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:pipeline Rust Pipeline Related Tasks area:receiver Core and Contrib receiver nodes lang:rust Pull requests that update Rust code size/XL

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants