Discussion: do we need OTEP for ETW exporter?

[maxgolov]

@lalitb @alolita @open-telemetry/cpp-approvers

I'm opening here so we can discuss the issue openly :)

We discussed on a weekly SIG call today to propose OTEP for ETW. However, since ETW is accessible only via Win32 API on Windows, which itself is only accessible from C / C++ code only -- no other languages have easy-to-use direct bindings to ETW. They all go thru native C function interface / add-on for that.

Reading thru the OTEP guidelines here:

ETW presently affects only one language implementation. Which is C++. Thus, it is not significant to other languages, and does not break any other OpenTelemetry SDKs for the other languages.

I would rather propose the following phased approach:

• we merge the ETW exporter from @mishal23 .
• we further add examples how to use OpenTelemetry with ETW exporter from node.js to OpenTelemetry C++ SDK in https://github.com/open-telemetry/opentelemetry-cpp-contrib repo.
• once this work is done, we I would propose an OTEP that illustrates how the other higher-level languages may utilize bindings to OpenTelemetry C++ SDK API ( no notion of ETW per se), in order to implement various exporters available to C++ SDK. Including Event Tracing for Windows. That would essentially eliminate the need to mention ETW in a cross-language OTEP - that will be just bindings to OpenTelemetry C++ SDK proposal.

If you believe this approach is not in alignment with OTEP guidelines, please let me know your concerns.

[lalitb]

I think this would be more generic otep ( not specifically for ETW) if we plan to create i.e., From language prospective which exporters are important to reside in core. Putting ETW as example why it is significant from C++ (and possibly .Net ) prospective.

Also there is an existing otep discussion going regarding location of exporters : open-telemetry/oteps#142 driven by @alolita . We can chime in there too if required.

[maxgolov]

@lalitb - yeah, but projection is a different initiative.

My metapoint is - ETW exporter, being primarily accessible only from C++ code and normally not directly exposed to other languages - does not need an OTEP. Because it falls under the Things that affect only a single language or implementation exception. That is, predominantly, ETW is relevant to C++ SDK at this point ... because you just can't easily code ETW events in other languages.

Then as for the other languages, should they see the need to support ETW on Windows, e.g. node.js app services running with Azure monitoring agent that listens to ETW events would need that; OR customers integrating via OpenTelemetry SDK with Application Insights ETW Collector - need that too. We can provide generic guidelines / instructions how to consume OpenTelemetry C++ built-in ETW exporter from other programming languages.

It is easier to bridge from OpenTelemetry API in other languages to OpenTelemetry C++ API (instead of writing C++ code to talk to ETW directly), as OT API is cross-plat and unified. So the other languages then reuse the existing ETW exporter bundled within the OpenTelemetry C++ SDK via OpenTelemetry unified cross-plat API surface.

I expressed my logical reasoning why it has to be part of the main repo here. Please check it out:
open-telemetry/community#468 (comment)
#326

I'd like to understand why we should be moving it to contrib? Based on above info, I don't think we should move it.

[pyohannes]

Instead of writing an OTEP, one could just try to get those lines of the spec removed:

Other vendor-specific exporters (exporters that implement vendor protocols) should not be included in language libraries and should be placed elsewhere (the exact approach for storing and maintaining vendor-specific exporters will be defined in the future).

It's anyway not clear what vendor-specific exactly means.

[lalitb]

Instead of writing an OTEP, one could just try to get those lines of the spec removed:

That was the initial suggestion to raise PR to modify language guideline. We have a strong reason to add this as core exporter based on the project statement:
"
Add exporter for ETW (Event Tracing for Windows) as one of the optional recommended exporters that languages may decide to implement as part of SDK.
"

[lalitb]

@alolita - Would need your input please for both the exporters ( ETW and Elastic ) . If we plan to move it to -contrib repo, the repo is not yet ready to take merges ( need to add build / ci scripts for this repo separately ).

[maxgolov]

We discussed on a call with @alolita @MarkSeufert @xukaren @ThomsonTan last week that we let the intern projects merged here, both Elastic and ETW. Once we get the contrib repo set up and Alolita's OTEP approved (moving exporters to contrib), we may map opentelemetry-cpp-contrib repo as git submodule at /contrib/ and modify the build scripts to respect the WITH_${XYZ} or WITH_CONTRIB option. That way the main repo build may include whatever exporters from contrib, e.g. from /contrib/exporters/ . Meanwhile it'd be practical to use the CODEOWNERS file here for each exporter integrated (flagging the owners of Elastic and ETW exporter). I can be the owner of ETW exporter.

[maxgolov]

I'll close this issue. There are two possible options:

• once we set up the contrib repo - we move ETW in there. That'd be in alignment with the OTEP proposal; PR
• we keep it and add CODEOWNERS file to directory that contains ETW exporter.

Anyways, this is not a topic that deserves OTEP. Let's close it and follow-up during our usual C++ SIG meetings.