Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code improvements for ETW exporter #519

Merged
merged 94 commits into from
Apr 3, 2021
Merged
Changes from 1 commit
Commits
Show all changes
94 commits
Select commit Hold shift + click to select a range
6bd4aaa
Drafts for proper TLD operation. Verified that TLD flow works well wi…
maxgolov Jan 12, 2021
f8a3af1
Add CODEOWNERS for ETW exporter
maxgolov Jan 15, 2021
035d863
Add support for owning Properties container
maxgolov Jan 15, 2021
1141e29
Make SpanContextKeyValueIterable empty container by default
maxgolov Jan 15, 2021
01cd9ac
is_key_value_iterable does not property test for subclasses of KeyVal…
maxgolov Jan 15, 2021
01c693c
Allow C string type
maxgolov Jan 15, 2021
66b1ee2
Add ability for vendors to customize their ETW field names
maxgolov Jan 15, 2021
90ecfd0
Code refactor to address outstanding feature parity issues
maxgolov Jan 15, 2021
5acc2d3
Code formatting
maxgolov Jan 15, 2021
013d1fd
Fix glob for Bazel build
maxgolov Jan 15, 2021
956e38d
Fix Bazel formatting issue
maxgolov Jan 15, 2021
bcc54f7
Merge branch 'master' into maxgolov/etw_exporter
maxgolov Jan 15, 2021
846ad71
Wrong #ifdef in test
maxgolov Jan 16, 2021
16ccb2a
Add prometheus to Visual Studio 2019 CMake build configuration
maxgolov Jan 16, 2021
fc55f3c
Add support for span<uint8_t> and const char * to OTLP exporter
maxgolov Jan 16, 2021
1b039a8
Sort OwnedAttributeValue in the same order as AttributeValue (for con…
maxgolov Jan 16, 2021
af1718f
Use enums instead of hardcoded constants
maxgolov Jan 16, 2021
d142031
Add prometheus-cpp to vcpkg build on Windows
maxgolov Jan 16, 2021
c02a822
include(CTest) and only then if(BUILD_TESTING)
maxgolov Jan 16, 2021
d86267f
Merge branch 'maxgolov/etw_exporter' of https://github.com/open-telem…
maxgolov Jan 16, 2021
c4d73ad
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
maxgolov Jan 16, 2021
afd035e
Fix CI failures
maxgolov Jan 16, 2021
a1141e4
Add nlohmann/json to CMake build
maxgolov Jan 16, 2021
93b2480
Compile empty stub for writeMsgPack when HAVE_MSGPACK is not defined
maxgolov Jan 16, 2021
897da33
Build Bazel with ETW-MsgPack exporter
maxgolov Jan 16, 2021
07952be
Add nlohmann_json to bazel build of ETW exporter
maxgolov Jan 16, 2021
09d8fa9
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
maxgolov Jan 19, 2021
8a737c7
Incremental clean-up and simplify Bazel build
maxgolov Jan 20, 2021
a4b459d
Merge branch 'master' into maxgolov/etw_exporter
maxgolov Jan 20, 2021
0c3b167
Update .github/CODEOWNERS
maxgolov Jan 21, 2021
fc2cbb3
Merge branch 'master' into maxgolov/etw_exporter
maxgolov Jan 22, 2021
b7153eb
Allow populating Span from byte buffer
maxgolov Jan 29, 2021
a09a941
Allow populating TraceId from byte buffer
maxgolov Jan 29, 2021
5de423b
Allow populating ActivityId and RelatedActivity based on SpanId and P…
maxgolov Jan 29, 2021
4b5d3b7
Automated context propagation
maxgolov Jan 29, 2021
fdda4da
Include headers for CoCreateGuid
maxgolov Jan 29, 2021
ee14876
Added 3-level deep spans to test
maxgolov Jan 29, 2021
51461d4
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Jan 29, 2021
ebcd88e
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Feb 2, 2021
c81d08e
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Feb 2, 2021
44f0ce2
Reformat code
maxgolov Feb 2, 2021
c3a7442
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Feb 25, 2021
6d70f6d
Build Release without debug info
maxgolov Feb 25, 2021
81f0db9
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Mar 11, 2021
9475c16
Rename test provider to OpenTelemetry-ETW-Provider
maxgolov Mar 11, 2021
0d993d0
Merge branch 'maxgolov/etw_exporter_ActivityId' of https://github.com…
maxgolov Mar 11, 2021
e159b2c
Reformat code using latest formatter
maxgolov Mar 11, 2021
4adafce
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Mar 17, 2021
c9f1efa
define HAVE_TLD only if it hasn't been defined yet
maxgolov Mar 20, 2021
ec0ef2e
Fix C++17 issue regarding deprecated C++11 codecvt
maxgolov Mar 20, 2021
48adeea
Add tests for detailed decorator and min decorator
maxgolov Mar 20, 2021
c3d0dc8
Add smarter ref-counting logic to ETW provider, allowing multiple tra…
maxgolov Mar 20, 2021
753521b
ETW Tracer improvements:
maxgolov Mar 20, 2021
5ed213c
Code formatting changes
maxgolov Mar 20, 2021
1d46f76
Add time routines to utils
maxgolov Mar 23, 2021
131f801
Fix bug in format passing
maxgolov Mar 23, 2021
51bea36
Add separate tests for TLD (TraceLoggingDynamic) encoding and MsgPack…
maxgolov Mar 23, 2021
4543a47
Clean-up field names for ETW notation (common field names used for bo…
maxgolov Mar 23, 2021
48991c0
ETW/MsgPack: Chunk Span Start, Span Event, Span Stop as individual ET…
maxgolov Mar 23, 2021
8678a91
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Mar 26, 2021
665422e
Fix GMock linkage error on Windows
maxgolov Mar 26, 2021
fb71a95
Merge branch 'maxgolov/cmake_windows_fix' of https://github.com/open-…
maxgolov Mar 26, 2021
f459bd3
Addressing code review comments
maxgolov Mar 27, 2021
4fd5528
Remove unused templates
maxgolov Mar 27, 2021
da56e3e
Add benchmark to compare Properties vs initializer list vs unordered_…
maxgolov Mar 30, 2021
2951a1e
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Mar 30, 2021
127f149
Update exporters/etw/include/opentelemetry/exporters/etw/etw_provider.h
maxgolov Mar 31, 2021
b51040a
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Mar 31, 2021
0f2b8d6
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
maxgolov Apr 1, 2021
b78797f
Revert "Find GTest via CONFIG mode to reference gmock (#640)"
maxgolov Apr 1, 2021
a683ed5
Add time formatting utility
maxgolov Apr 1, 2021
1db003e
Rename 'kind' field to 'OpCode' (start or stop)
maxgolov Apr 1, 2021
3a7831a
Tests are no longer included in Windows build
maxgolov Apr 1, 2021
b0c31bf
New feature to consolidate all Span info on end of Span into separate…
maxgolov Apr 1, 2021
bcf7325
Apply code formatting rules
maxgolov Apr 1, 2021
1561bd0
Add support for const char* and byte arrays under feature gate in Zip…
maxgolov Apr 1, 2021
b51cd39
Rename type to Google coding style kTypeString
maxgolov Apr 2, 2021
61b2f7c
Addressing code review comments: remove api change
maxgolov Apr 2, 2021
467f823
Addressing code review comments: rename namespace from ETW to etw
maxgolov Apr 2, 2021
576d510
Remove ETW recordable class as it is not needed
maxgolov Apr 2, 2021
a1653b4
Remove HAVE_CSTRING_TYPE build option
maxgolov Apr 2, 2021
57ab55b
Remove API changes
maxgolov Apr 2, 2021
1b0baf7
Rename namespace from ETW to etw
maxgolov Apr 2, 2021
e37cee5
Rename namespace from ETW to etw
maxgolov Apr 2, 2021
63f793d
Address code review comment: rename enum types to follow Google codin…
maxgolov Apr 2, 2021
a8afc39
Remove HAVE_CSTRING_TYPE
maxgolov Apr 2, 2021
18b2f9b
Rename enum types to follow Google coding style
maxgolov Apr 2, 2021
c2777e2
Remove HAVE_CSTRING_TYPE build option
maxgolov Apr 2, 2021
bf8a3d4
Remove HAVE_CSTRING_TYPE option and rename enums to follow Google Cod…
maxgolov Apr 2, 2021
8feee53
Rename namespace from ETW to etw
maxgolov Apr 2, 2021
888a040
Rename types to match Google coding style
maxgolov Apr 2, 2021
69c5e66
Fix mismatched ifdef
maxgolov Apr 2, 2021
80512b5
Revert changes to TraceZ
maxgolov Apr 2, 2021
550f2d8
true should be returned from Null object pattern to indicate success
maxgolov Apr 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add support for owning Properties container
maxgolov committed Jan 15, 2021
commit 035d86301f6239e8d67d6e362354c69cdb2fb204
472 changes: 472 additions & 0 deletions exporters/etw/include/opentelemetry/exporters/etw/etw_properties.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,472 @@
#pragma once

#include "opentelemetry/version.h"

#include "opentelemetry/common/key_value_iterable_view.h"

#include <vector>
#include <string>
#include <map>
#include <opentelemetry/nostd/span.h>

OPENTELEMETRY_BEGIN_NAMESPACE
namespace core {};
namespace trace {};
namespace exporter
{
namespace ETW
{

/**
* @brief PropertyVariant provides:
* - a constructor to initialize from initializer lists
* - an owning wrapper around `common::AttributeValue`
*/
using PropertyVariant =
nostd::variant<bool,
int32_t,
int64_t,
uint32_t,
uint64_t,
double,
std::string,
#ifdef HAVE_CSTRING_TYPE
const char *,
#endif
#ifdef HAVE_SPAN_BYTE
// TODO: 8-bit byte arrays / binary blobs are not part of OT spec yet!
// Ref: https://github.com/open-telemetry/opentelemetry-specification/issues/780
std::vector<uint8_t>,
#endif
std::vector<bool>,
std::vector<int32_t>,
std::vector<int64_t>,
std::vector<uint32_t>,
std::vector<uint64_t>,
std::vector<double>,
std::vector<std::string>>;

/**
* @brief PropertyValue class that holds PropertyVariant and
* provides converter for non-owning common::AttributeValue
*/
class PropertyValue : public PropertyVariant
{

/**
* @brief Convert span<T> to vector<T>
* @tparam T
* @param source
* @return
*/
template <typename T>
static std::vector<T> to_vector(const nostd::span<const T, std::size_t(-1)> &source)
{
return std::vector<T>(source.begin(), source.end());
};

/**
* @brief Convert span<string_view> to vector<string>
* @param source
* @return
*/
std::vector<std::string> static to_vector(const nostd::span<const nostd::string_view> &source)
{
std::vector<std::string> result;
result.reserve(source.size());
for (const auto &item : source)
{
result.push_back(std::string(item.data()));
Copy link
Contributor

Choose a reason for hiding this comment

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

We should pass item.size() here too. Otherwise we might run into problems with string_views that contain NULLs in the middle of the string, or that are not NULL terminated.

Copy link
Contributor Author

@maxgolov maxgolov Jan 21, 2021

Choose a reason for hiding this comment

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

I sort of agree (partially). Let me digest this.. I agree with the .size() argument for security reasons, but not because of NULs in between. We want all incoming string properties be UTF-8, thus no nulls in the middle of a string.

For the byte arrays (which may have NULs in between) - we need to avoid string_view type. It'd be best to have this spec issue resolved, to allow for byte arrays as span<uint8_t>. I added a feature flag for that in my PR. So that even before spec issue is approved - it's possible to compile the SDK with byte arrays support:
open-telemetry/opentelemetry-specification#780

Also if we add bytes support now ahead of spec, we can:

  • export byte arrays as int arrays for OTLP protocol. This does not break the (current) spec.
  • export byte arrays as byte arrays for ETW. It supports win:Binary byte array type described here
  • maintain ABI compatibility with new spec once byte arrays support is approved (no need to change variant definitions)

Copy link
Contributor

Choose a reason for hiding this comment

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

We want all incoming string properties be UTF-8, thus no nulls in the middle of a string.

Agreed. Then I'll narrow my concern to strings that are not null terminated.

}
return result;
};

/**
* @brief Convert vector<INTEGRAL> to span<INTEGRAL>.
* @tparam T
* @param std::vector<T>
* @return nostd::span<const T>
*/
template <typename T, std::enable_if_t<std::is_integral<T>::value, bool> = true>
static nostd::span<const T> to_span(const std::vector<T> &vec)
{
nostd::span<const T> result(vec.data(), vec.size());
return result;
};

/**
* @brief Convert vector<FLOAT> to span<const FLOAT>.
* @tparam T
* @param std::vector<T>
* @return nostd::span<const T>
*/
template <typename T, std::enable_if_t<std::is_floating_point<T>::value, bool> = true>
static nostd::span<const T> to_span(const std::vector<T> &vec)
{
nostd::span<const T> result(vec.data(), vec.size());
return result;
};

#if 0
/**
* @brief Convert vector<bool> to span<const bool>.
* FIXME: std::vector<bool> is a special compact type that does not have .data() member
*
* @param v
* @return
*/
static nostd::span<const bool> to_span(const std::vector<bool> &vec)
{
nostd::span<const bool> result(vec.data(), vec.size());
return result;
};
#endif

public:
/**
* @brief PropertyValue from bool
* @param v
* @return
*/
PropertyValue(bool value) : PropertyVariant(value){};

/**
* @brief PropertyValue from integral.
* @param v
* @return
*/
template <typename TInteger, std::enable_if_t<std::is_integral<TInteger>::value, bool> = true>
PropertyValue(TInteger number) : PropertyVariant(number){};

/**
* @brief PropertyValue from floating point.
* @param v
* @return
*/
template <typename TFloat, std::enable_if_t<std::is_floating_point<TFloat>::value, bool> = true>
PropertyValue(TFloat number) : PropertyVariant(double(number)){};

/**
* @brief Default PropertyValue (int32_t=0)
* @param v
* @return
*/
PropertyValue() : PropertyVariant(int32_t(0)){};

/**
* @brief PropertyValue from array of characters as string.
*
* @param v
* @return
*/
PropertyValue(char value[]) : PropertyVariant(std::string(value)){};

/**
* @brief PropertyValue from array of characters as string.
*
* @param v
* @return
*/
PropertyValue(const char *value) : PropertyVariant(value){};

/**
* @brief PropertyValue from string.
*
* @param v
* @return
*/
PropertyValue(const std::string &value) : PropertyVariant(value){};

/**
* @brief PropertyValue from vector as array.
* @return
*/
template <typename T>
PropertyValue(std::vector<T> value) : PropertyVariant(value){};

/**
* @brief Convert owning PropertyValue to non-owning common::AttributeValue
Copy link
Member

Choose a reason for hiding this comment

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

Nit: probably function description need to be corrected.

* @return
*/
PropertyValue &FromAttributeValue(const common::AttributeValue& v)
{
switch (v.index())
{
case common::AttributeType::TYPE_BOOL:
PropertyVariant::operator=(nostd::get<bool>(v));
break;
case common::AttributeType::TYPE_INT:
PropertyVariant::operator=(nostd::get<int32_t>(v));
break;
case common::AttributeType::TYPE_INT64:
PropertyVariant::operator=(nostd::get<int64_t>(v));
break;
case common::AttributeType::TYPE_UINT:
PropertyVariant::operator=(nostd::get<uint32_t>(v));
break;
case common::AttributeType::TYPE_UINT64:
PropertyVariant::operator=(nostd::get<uint64_t>(v));
break;
case common::AttributeType::TYPE_DOUBLE:
PropertyVariant::operator=(nostd::get<double>(v));
break;
case common::AttributeType::TYPE_STRING: {
PropertyVariant::operator= (nostd::string_view(nostd::get<nostd::string_view>(v)).data());
break;
};

#ifdef HAVE_CSTRING_TYPE
case common::AttributeType::TYPE_CSTRING:
PropertyVariant::operator=(nostd::get<const char *>(v));
break;
#endif

#ifdef HAVE_SPAN_BYTE
case common::AttributeType::TYPE_SPAN_BYTE:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const uint8_t>>(v)));
break;
#endif

case common::AttributeType::TYPE_SPAN_BOOL:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const bool>>(v)));
break;

case common::AttributeType::TYPE_SPAN_INT:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const int32_t>>(v)));
break;

case common::AttributeType::TYPE_SPAN_INT64:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const int64_t>>(v)));
break;

case common::AttributeType::TYPE_SPAN_UINT:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const uint32_t>>(v)));
break;

case common::AttributeType::TYPE_SPAN_UINT64:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const uint64_t>>(v)));
break;

case common::AttributeType::TYPE_SPAN_DOUBLE:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const double>>(v)));
break;

case common::AttributeType::TYPE_SPAN_STRING:
PropertyVariant::operator=(to_vector(nostd::get<nostd::span<const nostd::string_view>>(v)));
break;

default:
break;
}
return (*this);
}

/**
* @brief Convert owning PropertyValue to non-owning common::AttributeValue
* @param other
*/
common::AttributeValue ToAttributeValue() const
{
common::AttributeValue value;

switch (this->index())
{
case common::AttributeType::TYPE_BOOL:
value = nostd::get<bool>(*this);
break;
case common::AttributeType::TYPE_INT:
value = nostd::get<int32_t>(*this);
break;
case common::AttributeType::TYPE_INT64:
value = nostd::get<int64_t>(*this);
break;
case common::AttributeType::TYPE_UINT:
value = nostd::get<uint32_t>(*this);
break;
case common::AttributeType::TYPE_UINT64:
value = nostd::get<uint64_t>(*this);
break;
case common::AttributeType::TYPE_DOUBLE:
value = nostd::get<double>(*this);
break;

case common::AttributeType::TYPE_STRING: {
const std::string &str = nostd::get<std::string>(*this);
return nostd::string_view(str.data(), str.size());
break;
}

#ifdef HAVE_CSTRING_TYPE
case common::AttributeType::TYPE_CSTRING:
value = nostd::get<const char *>(*this);
break;
#endif

#ifdef HAVE_SPAN_BYTE
case common::AttributeType::TYPE_SPAN_BYTE:
value = to_span(nostd::get<std::vector<uint8_t>>(self));
break;
#endif

case common::AttributeType::TYPE_SPAN_BOOL: {
const auto& vec = nostd::get<std::vector<bool>>(*this);
// FIXME: sort out how to remap from vector<bool> to span<bool>
break;
}

case common::AttributeType::TYPE_SPAN_INT:
value = to_span(nostd::get<std::vector<int32_t>>(*this));
break;

case common::AttributeType::TYPE_SPAN_INT64:
value = to_span(nostd::get<std::vector<int64_t>>(*this));
break;

case common::AttributeType::TYPE_SPAN_UINT:
value = to_span(nostd::get<std::vector<uint32_t>>(*this));
break;

case common::AttributeType::TYPE_SPAN_UINT64:
value = to_span(nostd::get<std::vector<uint64_t>>(*this));
break;

case common::AttributeType::TYPE_SPAN_DOUBLE:
value = to_span(nostd::get<std::vector<double>>(*this));
break;

case common::AttributeType::TYPE_SPAN_STRING:
// FIXME: sort out how to remap from vector<string> to span<string_view>
// value = to_span(nostd::get<std::vector<std::string>>(self));
break;

default:
break;
}
return value;
}
};

/**
* @brief Map of PropertyValue
*/
using PropertyValueMap = std::map<std::string, PropertyValue>;

/**
* @brief Map of PropertyValue with common::KeyValueIterable interface.
*/
class Properties : public common::KeyValueIterable, public PropertyValueMap
{

/**
* @brief Helper tyoe for map constructor.
*/
using PropertyValueType = std::pair<const std::string, PropertyValue>;

public:

/**
* @brief PropertyValueMap constructor.
*/
Properties() : PropertyValueMap() {};

/**
* @brief PropertyValueMap constructor from initializer list.
*/
Properties(const std::initializer_list<PropertyValueType> properties)
: PropertyValueMap()
{
(*this) = (properties);
};

/**
* @brief PropertyValueMap assignment operator from initializer list.
*/
Properties &operator=(std::initializer_list<PropertyValueType> properties)
{
PropertyValueMap::operator = (properties);
return (*this);
};

/**
* @brief PropertyValueMap constructor from map.
*/
Properties(const PropertyValueMap &properties) : PropertyValueMap()
{
(*this) = properties;
};

/**
* @brief PropertyValueMap assignment operator from map.
*/
Properties &operator=(const PropertyValueMap &properties)
{
PropertyValueMap::operator=(properties);
return (*this);
};

/**
* @brief PropertyValueMap constructor from KeyValueIterable
* allows to convert non-Owning KeyValueIterable to owning
* container.
*
*/
Properties(const common::KeyValueIterable &other)
{
(*this) = other;
}

/**
* @brief PropertyValueMap assignment operator.
*/
Properties &operator=(const common::KeyValueIterable &other)
{
clear();
other.ForEachKeyValue([&](nostd::string_view key, common::AttributeValue value) noexcept {
std::string k(key.data(), key.length());
(*this)[k].FromAttributeValue(value);
return true;
});
return (*this);
};

/**
* @brief PropertyValueMap property accessor.
*/
PropertyValue &operator[](const std::string &k)
{
return PropertyValueMap::operator[](k);
}

/**
* Iterate over key-value pairs
* @param callback a callback to invoke for each key-value. If the callback returns false,
* the iteration is aborted.
* @return true if every key-value pair was iterated over
*/
bool ForEachKeyValue(nostd::function_ref<bool(nostd::string_view, common::AttributeValue)>
callback) const noexcept override
{
for (const auto &kv : (*this))
{
const common::AttributeValue& value = kv.second.ToAttributeValue();
if (!callback(nostd::string_view{kv.first}, value))
{
return false;
}
}
return true;
};

/**
* @return the number of key-value pairs
*/
size_t size() const noexcept override
{
return PropertyValueMap::size();
};

};

}
} // namespace exporter
OPENTELEMETRY_END_NAMESPACE