Code improvements for ETW exporter

[maxgolov]

Clean-up of ETW exporter

The following feature may be reusable by other components:

• Owning Properties collection (container) with convenient initializer list operators and constructors to pass incoming event properties as std::string, vectors of integers/floats, string literals, etc. The way how it should be to be simple and look like JSON object initialization.

Usage example

// Span attributes
  Properties attribs =
  {
    {"attrib1", 1},
    {"attrib2", 2}
  };
  auto outerSpan = tracer->StartSpan("MySpanOuter", attribs);

  // Add first event
  std::string eventName1 = "MyEvent1";
  Properties event1 =
  {
    {"uint32Key", (uint32_t)1234},
    {"uint64Key", (uint64_t)1234567890},
    {"strKey", "someValue"}
  };
  outerSpan->AddEvent(eventName1, event1);

  // Add second event
  std::string eventName2 = "MyEvent2";
  Properties event2 =
  {
    {"uint32Key", (uint32_t)9876},
    {"uint64Key", (uint64_t)987654321},
    {"strKey", "anotherValue"}
  };
  outerSpan->AddEvent(eventName2, event2);

  // Create nested span. Note how we share the attributes here..
  // It is Okay to either share, OR have your own attributes map.
  auto innerSpan = tracer->StartSpan("MySpanInner", attribs);
  std::string eventName3= "MyEvent3";
  Properties event3 =
  {
    {"uint32Key", (uint32_t)9876},
    {"uint64Key", (uint64_t)987654321},
    // {"int32array", {{-1,0,1,2,3}} },
    // Since this collection is owning, we may store values returned by other utility functions
    {"tempString", getTemporaryValue() }
  };
  
  innerSpan->AddEvent(eventName3, event3);
  innerSpan->End();    // end innerSpan
  outerSpan->End();    // end outerSpan
  tracer->CloseWithMicroseconds(0);

Note that the new class is fully compatible with KeyValueIterable, i.e. provides ABI-safe transform - should there be the need for ABI-safe interface. Note that across Visual Studio 2015-2017-2019, the ABI-safety for STL types is readily guaranteed on Windows. Thus, strictly speaking, unnecessary requirement that can be heavily optimized by avoiding the unnecessary tranformation.

ETW channel -specific improvements

List of features added:

• preliminary support for Span attributes and links
• adding TraceId + SpanId on individual events
• separate expression for Spans being started / stopped; support for nested Spans
• code comments clean-up
• code refactor

Tests

ETW listener 'Test Framework' is written in C# - will be submited in contrib repo. This code also provides ability to Debug OpenTelemetry ETW exporter events in Visual Studio 2019 using built-in Diagnostic Events tab. Code is vendor-agnostic. I included examples of some other vendors using ETW. Those could be the candidates to adopt OpenTelemetry C++ SDK with ETW channel. Field names for ETW channel are also configurable, i.e. vendors may decide their own naming convention. It would make sense to normalize this with OTLP protocol names.

Debugging Telemetry in Visual Studio 2019

Example screenshot that illustrates how test events show-up in Visual Studio 2019 with no additional tooling necessary:

This article explains more about Event Tracing for Windows and possible ways to export the data further:
https://docs.microsoft.com/en-us/azure/service-fabric/service-fabric-diagnostics-how-to-monitor-and-diagnose-services-locally

Possible flows will be explained further in contrib repo. Existing tools that allow to monitor and export ETW events to Elastic Search, for example: https://www.fireeye.com/blog/threat-research/2019/03/silketw-because-free-telemetry-is-free.html

[codecov]

Codecov Report

Merging #519 (550f2d8) into main (9b6a50e) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #519      +/-   ##
==========================================
+ Coverage   94.51%   94.52%   +0.01%     
==========================================
  Files         199      199              
  Lines        9136     9136              
==========================================
+ Hits         8635     8636       +1     
+ Misses        501      500       -1     

Impacted Files	Coverage Δ
...ude/opentelemetry/common/key_value_iterable_view.h	100.00% <ø> (ø)
...ude/opentelemetry/trace/span_context_kv_iterable.h	100.00% <ø> (ø)
api/include/opentelemetry/trace/tracer.h	100.00% <ø> (ø)
api/test/nostd/string_view_test.cc	100.00% <ø> (ø)
...include/opentelemetry/sdk/common/attribute_utils.h	100.00% <ø> (ø)
sdk/include/opentelemetry/sdk/metrics/instrument.h	93.44% <100.00%> (ø)
sdk/src/logs/batch_log_processor.cc	95.00% <0.00%> (+1.25%)	⬆️

[maxgolov]

@pyohannes - I addressed all of your comments, including stylistic comments. Re. API changes in a separate PR: instead I minimized the changes, and removed the HAVE_C_STRING build option you didn't like - and cleaned it up wherever it was referenced; renamed the enums to match Google coding style. There are no API / contract breaking changes in this PR. All tests for all possible exporters also pass.

[pyohannes]

Some additional comments for API related changes.

For lack of time I didn't give a deep review and test to the SDK related changes.

[pyohannes]

It's a long name, but let's use NullSpanContextKeyValueIterable. To be consistent with SpanContextKeyValueIterable. NullSpanContext is ambiguous.

[pyohannes]

Anyway, to avoid an additional class, one could also do this:

using SpanContextKeyValueIterableInitializerList = SpanContextKeyValueIterableView<std::initializer_list<
          std::pair<SpanContext,
                    std::initializer_list<std::pair<nostd::string_view, common::AttributeValue>>>>>;

And then define your additional StartSpan method like this:

 nostd::shared_ptr<Span> StartSpan(nostd::string_view name,
                                    const common::KeyValueIterable &attributes,
                                    const StartSpanOptions &options = {}) noexcept
  {
    return this->StartSpan(name, attributes, SpanContextKeyValueIterableInitializerList({}), options);
  }

Just to avoid a clutter of types that we also need to document (and that are error-prone, e. g. return false in case of success). We also have NullKeyValueIterable, however, it's not used anywhere ...

[maxgolov]

I can refactor this in a separate (unrelated PR) by moving these into separate helper header or something. That would also enable NullTracerProvider cases, unrelated to ETW per se. Right now it is more or less "internal" detail that I think we can rename / adjust before GA.

[pyohannes]

This should be safe. The actual contract is captured in the signature of ForEachKeyValue.

[pyohannes]

No problematic changes on API side. If not in this PR, we can remove NullSpanContext (and maybe even NullKeyValuIterable, which is unused) in a separate PR.

[jsuereth]

Approving changes to Attribute type enums + adding better HAVE_SPAN_BYTE support. +1 for diving through all that :)

Are you looking for bazel support for the various configuration of builds or are we ok just using CMake for that for now?

[maxgolov]

Approving changes to Attribute type enums + adding better HAVE_SPAN_BYTE support. +1 for diving through all that :)
Are you looking for bazel support for the various configuration of builds or are we ok just using CMake for that for now?

Thanks Josh. I'm good with CMake for now, but I'll take a look at Bazel right after. The way it stands - this part is kinda "self-sufficient" in a way that it's a header-only library, and the current customer who's gonna "preview" it - uses MSBuild / Visual Studio anyways (not even CMake nor Bazel). So I'll get back when I get a confirmation from them it works the way they wanted. Then will add the "proper" full range support in terms of our both official build systems.

I will add E2E usage sample that covers the path from OT C++ => ETW => out-of-proc listener => Vendor-agnostic flow , maybe even using OpenTelemetry.NET SDK in a "sink" (listener) to re-ingest. Sample app will be in contrib repo, as it'll depend on non-C++ technology in out-of-proc ETW listener / sink.

[lalitb]

@maxgolov - While we merge this, we can also have a ticket to consolidate unresolved comments, to ensure they are handled after merge ?

[maxgolov]

we can also have a ticket to consolidate unresolved comments

Yes, I'll take care of it. Logged two issues here:

• For the API comment: Null object pattern TracerProvider + container classes #648

• For the Span Links comment: Proper handling of Span Links in ETW exporter #649