Skip to content

Commit

Permalink
ETW exporter warnings clean-up, compliance, implementing 2 TODOs (#702)
Browse files Browse the repository at this point in the history
  • Loading branch information
@maxgolov
maxgolov authored May 6, 2021
1 parent 556061a commit 32f10c7
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 72 deletions.
7 changes: 6 additions & 1 deletion exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@
#include <string>
#include <vector>

#ifdef _WIN32
# include <Windows.h>
#endif

OPENTELEMETRY_BEGIN_NAMESPACE
namespace exporter
{
Expand Down Expand Up @@ -178,7 +182,7 @@ class PropertyValue : public PropertyVariant
* @param v
* @return
*/
PropertyValue(const char *value) : PropertyVariant(value){};
PropertyValue(const char *value) : PropertyVariant(std::string(value)){};

/**
* @brief PropertyValue from string.
Expand Down Expand Up @@ -309,6 +313,7 @@ class PropertyValue : public PropertyVariant
case PropertyType::kTypeSpanBool: {
const auto &vec = nostd::get<std::vector<bool>>(*this);
// FIXME: sort out how to remap from vector<bool> to span<bool>
UNREFERENCED_PARAMETER(vec);
break;
}
case PropertyType::kTypeSpanInt:
Expand Down
28 changes: 11 additions & 17 deletions exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
# pragma warning(push)
# pragma warning(disable : 4459)
# pragma warning(disable : 4018)
# pragma warning(disable : 5054)
#endif

#include "opentelemetry/exporters/etw/etw_properties.h"
Expand Down Expand Up @@ -52,10 +53,9 @@

#define MICROSOFT_EVENTTAG_NORMAL_PERSISTENCE 0x01000000

OPENTELEMETRY_BEGIN_NAMESPACE
using namespace OPENTELEMETRY_NAMESPACE::exporter::etw;

using Properties = exporter::etw::Properties;
using PropertyType = exporter::etw::PropertyType;
OPENTELEMETRY_BEGIN_NAMESPACE

class ETWProvider
{
Expand Down Expand Up @@ -137,7 +137,6 @@ class ETWProvider

switch (format)
{
#ifdef HAVE_TLD
// Register with TraceLoggingDynamic facility - dynamic manifest ETW events.
case EventFormat::ETW_MANIFEST: {
tld::ProviderMetadataBuilder<std::vector<BYTE>> providerMetaBuilder(
Expand All @@ -164,7 +163,6 @@ class ETWProvider
};
};
break;
#endif

#ifdef HAVE_MSGPACK
// Register for MsgPack payload ETW events.
Expand Down Expand Up @@ -217,14 +215,12 @@ class ETWProvider
data.refCount--;
if (data.refCount == 0)
{
#ifdef HAVE_TLD
if (data.providerMetaVector.size())
{
// ETW/TraceLoggingDynamic provider
result = tld::UnregisterProvider(data.providerHandle);
}
else
#endif
{
// Other provider types, e.g. ETW/MsgPack
result = EventUnregister(data.providerHandle);
Expand Down Expand Up @@ -412,6 +408,11 @@ class ETWProvider
};
return (unsigned long)(writeResponse);
#else
UNREFERENCED_PARAMETER(providerData);
UNREFERENCED_PARAMETER(eventData);
UNREFERENCED_PARAMETER(ActivityId);
UNREFERENCED_PARAMETER(RelatedActivityId);
UNREFERENCED_PARAMETER(Opcode);
return STATUS_ERROR;
#endif
}
Expand All @@ -428,7 +429,6 @@ class ETWProvider
LPCGUID RelatedActivityId = nullptr,
uint8_t Opcode = 0 /* Information */)
{
#ifdef HAVE_TLD
// Make sure you stop sending event before register unregistering providerData
if (providerData.providerHandle == INVALID_HANDLE)
{
Expand Down Expand Up @@ -465,10 +465,7 @@ class ETWProvider
for (auto &kv : eventData)
{
const char *name = kv.first.data();
// Don't include event name field in the payload
if (EVENT_NAME == name)
continue;
auto &value = kv.second;
auto &value = kv.second;
switch (value.index())
{
case PropertyType::kTypeBool: {
Expand Down Expand Up @@ -518,15 +515,15 @@ class ETWProvider
dbuilder.AddString(temp);
break;
}
# if HAVE_TYPE_GUID
#if HAVE_TYPE_GUID
// TODO: consider adding UUID/GUID to spec
case PropertyType::kGUID: {
builder.AddField(name.c_str(), TypeGuid);
auto temp = nostd::get<GUID>(value);
dbuilder.AddBytes(&temp, sizeof(GUID));
break;
}
# endif
#endif

// TODO: arrays are not supported
case PropertyType::kTypeSpanByte:
Expand Down Expand Up @@ -577,9 +574,6 @@ class ETWProvider
};

return (unsigned long)(writeResponse);
#else
return STATUS_ERROR;
#endif
}

unsigned long write(Handle &providerData,
Expand Down
170 changes: 124 additions & 46 deletions exporters/etw/include/opentelemetry/exporters/etw/etw_tracer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ typedef struct
bool enableActivityTracking; // Emit TraceLogging events for Span/Start and Span/Stop
bool enableRelatedActivityId; // Assign parent `SpanId` to `RelatedActivityId`
bool enableAutoParent; // Start new spans as children of current active span
ETWProvider::EventFormat
encoding; // Event encoding to use for this provider (TLD, MsgPack, XML, etc.).
} TracerProviderConfiguration;

/**
Expand Down Expand Up @@ -104,6 +106,72 @@ static inline void GetOption(const TracerProviderOptions &options,
}
}

/**
* @brief Helper template to convert encoding config option to EventFormat.
* Configuration option passed as `options["encoding"] = "MsgPack"`.
* Default encoding is TraceLogging Dynamic Manifest (TLD).
*
* Valid encoding names listed below.
*
* For MessagePack encoding:
* - "MSGPACK"
* - "MsgPack"
* - "MessagePack"
*
* For XML encoding:
* - "XML"
* - "xml"
*
* For TraceLogging Dynamic encoding:
* - "TLD"
* - "tld"
*
*/
static inline ETWProvider::EventFormat GetEncoding(const TracerProviderOptions &options)
{
ETWProvider::EventFormat evtFmt = ETWProvider::EventFormat::ETW_MANIFEST;

auto it = options.find("encoding");
if (it != options.end())
{
auto varValue = it->second;
std::string val = nostd::get<std::string>(varValue);

#pragma warning(push)
#pragma warning(disable : 4307) /* Integral constant overflow - OK while computing hash */
auto h = utils::hashCode(val.c_str());
switch (h)
{
case CONST_HASHCODE(MSGPACK):
// nobrk
case CONST_HASHCODE(MsgPack):
// nobrk
case CONST_HASHCODE(MessagePack):
evtFmt = ETWProvider::EventFormat::ETW_MSGPACK;
break;

case CONST_HASHCODE(XML):
// nobrk
case CONST_HASHCODE(xml):
evtFmt = ETWProvider::EventFormat::ETW_XML;
break;

case CONST_HASHCODE(TLD):
// nobrk
case CONST_HASHCODE(tld):
// nobrk
evtFmt = ETWProvider::EventFormat::ETW_MANIFEST;
break;

default:
break;
}
#pragma warning(pop)
}

return evtFmt;
}

class Span;

/**
Expand Down Expand Up @@ -215,6 +283,12 @@ TracerProviderConfiguration &GetConfiguration(T &t)
return t.config_;
}

template <class T>
void UpdateStatus(T &t, Properties &props)
{
t.UpdateStatus(props);
}

/**
* @brief Utility method to convert SppanContext.span_id() (8 byte) to ActivityId GUID (16 bytes)
* @param span OpenTelemetry Span pointer
Expand Down Expand Up @@ -399,9 +473,18 @@ class Tracer : public trace::Tracer
// since Unix epoch instead of string, but that implies additional tooling
// is needed to convert it, rendering it NOT human-readable.
evt[ETW_FIELD_STARTTIME] = utils::formatUtcTimestampMsAsISO8601(startTimeMs);

#ifdef ETW_FIELD_ENDTTIME
// ETW has its own precise timestamp at envelope layer for every event.
// However, in some scenarios it is easier to deal with ISO8601 strings.
// In that case we convert the app-created timestamp and place it into
// Payload[$ETW_FIELD_TIME] field. The option configurable at compile-time.
evt[ETW_FIELD_ENDTTIME] = utils::formatUtcTimestampMsAsISO8601(endTimeMs);
#endif
// Duration of Span in milliseconds
evt[ETW_FIELD_DURATION] = endTimeMs - startTimeMs;
// Presently we assume that all spans are server spans
evt[ETW_FIELD_SPAN_KIND] = uint32_t(trace::SpanKind::kServer);
UpdateStatus(currentSpan, evt);
etwProvider().write(provHandle, evt, ActivityIdPtr, RelatedActivityIdPtr, 0, encoding);
}

Expand All @@ -426,10 +509,6 @@ class Tracer : public trace::Tracer
*/
ETWProvider::Handle &initProvHandle()
{
#if defined(HAVE_MSGPACK) && !defined(HAVE_TLD)
/* Fallback to MsgPack encoding if TraceLoggingDynamic feature gate is off */
encoding = ETWProvider::EventFormat::ETW_MSGPACK;
#endif
isClosed_ = false;
return etwProvider().open(provId, encoding);
}
Expand Down Expand Up @@ -687,6 +766,14 @@ class Tracer : public trace::Tracer
ActivityIdPtr = &ActivityId;
}

#ifdef HAVE_FIELD_TIME
{
auto timeNow = std::chrono::system_clock::now().time_since_epoch();
auto millis = std::chrono::duration_cast<std::chrono::milliseconds>(timeNow).count();
evt[ETW_FIELD_TIME] = utils::formatUtcTimestampMsAsISO8601(millis);
}
#endif

etwProvider().write(provHandle, evt, ActivityIdPtr, nullptr, 0, encoding);
};

Expand Down Expand Up @@ -736,6 +823,9 @@ class Span : public trace::Span
common::SystemTimestamp start_time_;
common::SystemTimestamp end_time_;

trace::StatusCode status_code_{trace::StatusCode::kUnset};
std::string status_description_;

/**
* @brief Owner Tracer of this Span
*/
Expand Down Expand Up @@ -791,6 +881,27 @@ class Span : public trace::Span
}

public:
/**
* @brief Update Properties object with current Span status
* @param evt
*/
void UpdateStatus(Properties &evt)
{
/* Should we avoid populating this extra field if status is unset? */
if ((status_code_ == trace::StatusCode::kUnset) || (status_code_ == trace::StatusCode::kOk))
{
evt[ETW_FIELD_SUCCESS] = "True";
evt[ETW_FIELD_STATUSCODE] = uint32_t(status_code_);
evt[ETW_FIELD_STATUSMESSAGE] = status_description_;
}
else
{
evt[ETW_FIELD_SUCCESS] = "False";
evt[ETW_FIELD_STATUSCODE] = uint32_t(status_code_);
evt[ETW_FIELD_STATUSMESSAGE] = status_description_;
}
}

/**
* @brief Get start time of this Span.
* @return
Expand Down Expand Up @@ -876,9 +987,8 @@ class Span : public trace::Span
*/
void SetStatus(trace::StatusCode code, nostd::string_view description) noexcept override
{
// TODO: not implemented
UNREFERENCED_PARAMETER(code);
UNREFERENCED_PARAMETER(description);
status_code_ = code;
status_description_ = description.data();
}

void SetAttributes(Properties attributes) { attributes_ = attributes; }
Expand Down Expand Up @@ -1007,6 +1117,9 @@ class TracerProvider : public trace::TracerProvider

// When a new Span is started, the current span automatically becomes its parent.
GetOption(options, "enableAutoParent", config_.enableAutoParent, false);

// Determines what encoding to use for ETW events: TraceLogging Dynamic, MsgPack, XML, etc.
config_.encoding = GetEncoding(options);
}

TracerProvider() : trace::TracerProvider()
Expand All @@ -1017,6 +1130,7 @@ class TracerProvider : public trace::TracerProvider
config_.enableActivityTracking = false;
config_.enableRelatedActivityId = false;
config_.enableAutoParent = false;
config_.encoding = ETWProvider::EventFormat::ETW_MANIFEST;
}

/**
Expand All @@ -1033,44 +1147,8 @@ class TracerProvider : public trace::TracerProvider
nostd::shared_ptr<trace::Tracer> GetTracer(nostd::string_view name,
nostd::string_view args = "") override
{
#if defined(HAVE_MSGPACK)
// Prefer MsgPack over ETW by default
ETWProvider::EventFormat evtFmt = ETWProvider::EventFormat::ETW_MSGPACK;
#else
// Fallback to ETW TraceLoggingDynamic if MsgPack support is not compiled in
ETWProvider::EventFormat evtFmt = ETWProvider::EventFormat::ETW_MANIFEST;
#endif

#pragma warning(push)
#pragma warning(disable : 4307) /* Integral constant overflow - OK while computing hash */
auto h = utils::hashCode(args.data());
switch (h)
{
case CONST_HASHCODE(MSGPACK):
// nobrk
case CONST_HASHCODE(MsgPack):
// nobrk
case CONST_HASHCODE(MessagePack):
evtFmt = ETWProvider::EventFormat::ETW_MSGPACK;
break;

case CONST_HASHCODE(XML):
// nobrk
case CONST_HASHCODE(xml):
evtFmt = ETWProvider::EventFormat::ETW_XML;
break;

case CONST_HASHCODE(TLD):
// nobrk
case CONST_HASHCODE(tld):
// nobrk
evtFmt = ETWProvider::EventFormat::ETW_MANIFEST;
break;

default:
break;
}
#pragma warning(pop)
UNREFERENCED_PARAMETER(args);
ETWProvider::EventFormat evtFmt = config_.encoding;
return nostd::shared_ptr<trace::Tracer>{new (std::nothrow) Tracer(*this, name, evtFmt)};
}
};
Expand Down
Loading

0 comments on commit 32f10c7

Please sign in to comment.