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

Support passing attributes with AddEvent #60

Merged
merged 45 commits into from
May 11, 2020
Merged
Show file tree
Hide file tree
Changes from 37 commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
2983982
Add varint files
rnburn Mar 31, 2020
0f7b217
Fill out variant backport
rnburn Mar 31, 2020
642eb73
Add type_pack_element
rnburn Mar 31, 2020
cdb6281
Add variant_size
rnburn Mar 31, 2020
8aee486
Add variant_alternative
rnburn Mar 31, 2020
56332bc
Fill out variant
rnburn Apr 1, 2020
998d1ed
Fill out variant backport
rnburn Apr 1, 2020
1515e08
Fill out variant back port
rnburn Apr 2, 2020
355bea0
Fill out variant backport
rnburn Apr 3, 2020
67dae8a
Fill out variant
rnburn Apr 3, 2020
ff80f15
Fill out variant
rnburn Apr 3, 2020
b25271c
Add variant test
rnburn Apr 3, 2020
5cfe001
Add get tests
rnburn Apr 3, 2020
9399c02
Add visit test
rnburn Apr 3, 2020
1b605b1
Add more tests
rnburn Apr 3, 2020
0462eea
Fix variant tests
rnburn Apr 3, 2020
4b0b985
Rename
rnburn Apr 3, 2020
aeefb7a
Reformat
rnburn Apr 3, 2020
73e5a44
Add copyrights
rnburn Apr 3, 2020
6382f42
Add more variant tests
rnburn Apr 3, 2020
9ecdcda
Reformat
rnburn Apr 3, 2020
18761c6
Add more tests
rnburn Apr 3, 2020
a77ca9a
Fix for gcc-4.8
rnburn Apr 3, 2020
7d81a52
Add AttributeValue
rnburn Apr 7, 2020
bb7147d
Support passing attributes in AddEvents api
rnburn Apr 7, 2020
f61290f
Support passing attributes in AddEvent api
rnburn Apr 7, 2020
92da4a8
Reformat
rnburn Apr 7, 2020
841d831
Add cmake test
rnburn Apr 7, 2020
66228c3
Tweak function_ref
rnburn Apr 7, 2020
f9fabf1
Tweak function_ref
rnburn Apr 7, 2020
0d90ba5
Fix for gcc-4.8
rnburn Apr 9, 2020
273d5b2
Reformat
rnburn Apr 9, 2020
0389471
Add function_ref test
rnburn Apr 9, 2020
0fcf7c1
Support arrays of primitives
rnburn Apr 20, 2020
cea161b
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
rnburn Apr 20, 2020
c275ae8
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
rnburn May 1, 2020
48151b1
Support bool in AttributeValue
rnburn May 1, 2020
8f0b128
Add documentation
rnburn May 4, 2020
5338087
Allow sdk to use different clocks for timestamping
rnburn May 4, 2020
9bb4444
Remove dead code
rnburn May 4, 2020
aa39603
Increase windows ci time limit
rnburn May 4, 2020
fc0e120
Reformat
rnburn May 4, 2020
45636a5
Merge branch 'master' of https://github.com/open-telemetry/openteleme…
rnburn May 10, 2020
e92a7c4
Move AttributeValue to common
rnburn May 10, 2020
92df11e
Add missing type to AttributeValue
rnburn May 10, 2020
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
2 changes: 1 addition & 1 deletion api/include/opentelemetry/nostd/variant.h
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,7 @@ struct convert_to_bool
static_assert(std::is_convertible<invoke_result_t<RelOp, Lhs, Rhs>, bool>::value,
"relational operators must return a type"
" implicitly convertible to bool");
return invoke(RelOp{}, std::forward<Lhs>(lhs), std::forward<Rhs>(rhs));
return nostd::invoke(RelOp{}, std::forward<Lhs>(lhs), std::forward<Rhs>(rhs));
}
};
} // namespace detail
Expand Down
7 changes: 7 additions & 0 deletions api/include/opentelemetry/plugin/tracer.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ class Span final : public trace::Span
span_->AddEvent(name, timestamp);
}

void AddEvent(nostd::string_view name,
core::SystemTimestamp timestamp,
const trace::KeyValueIterable &attributes) noexcept override
{
span_->AddEvent(name, timestamp, attributes);
}

void SetStatus(trace::CanonicalCode code, nostd::string_view description) noexcept override
{
span_->SetStatus(code, description);
Expand Down
27 changes: 27 additions & 0 deletions api/include/opentelemetry/trace/attribute_value.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
#pragma once
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder about moving this from trace to common? We will need the same functionality for metrics labels and, I assume, also for logs at some point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


#include <cstdint>

#include "opentelemetry/nostd/span.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/nostd/variant.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace trace
{
using AttributeValue = nostd::variant<bool,
int,
g-easy marked this conversation as resolved.
Show resolved Hide resolved
int64_t,
unsigned int,
uint64_t,
double,
nostd::string_view,
nostd::span<const bool>,
nostd::span<const int>,
nostd::span<const int64_t>,
nostd::span<const unsigned int>,
nostd::span<const uint64_t>,
nostd::span<const nostd::string_view>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

There's also this discussion going on: open-telemetry/opentelemetry-specification#376.

I'm not sure how serious that is, but it seems the idea is getting traction and is actively discussed currently. Maybe @reiley knows how likely this is to come and whether we should consider it.

} // namespace trace
OPENTELEMETRY_END_NAMESPACE
21 changes: 21 additions & 0 deletions api/include/opentelemetry/trace/key_value_iterable.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#pragma once

#include "opentelemetry/nostd/function_ref.h"
#include "opentelemetry/trace/attribute_value.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace trace
{
class KeyValueIterable
{
public:
virtual ~KeyValueIterable() = default;

virtual bool ForEachKeyValue(
g-easy marked this conversation as resolved.
Show resolved Hide resolved
nostd::function_ref<bool(nostd::string_view, AttributeValue)> callback) const noexcept = 0;

virtual size_t size() const noexcept = 0;
};
} // namespace trace
OPENTELEMETRY_END_NAMESPACE
65 changes: 65 additions & 0 deletions api/include/opentelemetry/trace/key_value_iterable_view.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#pragma once

#include <iterator>
#include <type_traits>
#include <utility>

#include "opentelemetry/nostd/utility.h"
#include "opentelemetry/trace/key_value_iterable.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace trace
{
namespace detail
{
inline void take_key_value(nostd::string_view, AttributeValue) {}

template <class T>
auto is_key_value_iterable_impl(T iterable)
/* -> decltype(std::pair<nostd::string_view, AttributeValue>(std::begin(iterable)->first, */
/* std::begin(iterable)->second), */
-> decltype(take_key_value(std::begin(iterable)->first, std::begin(iterable)->second),
nostd::size(iterable),
std::true_type{});

std::false_type is_key_value_iterable_impl(...);

template <class T>
struct is_key_value_iterable
{
static const bool value = decltype(detail::is_key_value_iterable_impl(std::declval<T>()))::value;
};
} // namespace detail

template <class T>
class KeyValueIterableView final : public KeyValueIterable
{
static_assert(detail::is_key_value_iterable<T>::value, "Must be a key-value iterable");

public:
explicit KeyValueIterableView(const T &container) noexcept : container_{&container} {}

// KeyValueIterable
bool ForEachKeyValue(nostd::function_ref<bool(nostd::string_view, AttributeValue)> callback) const
noexcept override
{
auto iter = std::begin(*container_);
auto last = std::end(*container_);
for (; iter != last; ++iter)
{
if (!callback(iter->first, iter->second))
{
return false;
}
}
return true;
}

size_t size() const noexcept override { return nostd::size(*container_); }

private:
const T *container_;
};
} // namespace trace
OPENTELEMETRY_END_NAMESPACE
14 changes: 10 additions & 4 deletions api/include/opentelemetry/trace/noop.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@ class NoopSpan final : public Span
public:
explicit NoopSpan(const std::shared_ptr<Tracer> &tracer) noexcept : tracer_{tracer} {}

void AddEvent(nostd::string_view name) noexcept override {}
void AddEvent(nostd::string_view /*name*/) noexcept override {}

void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override {}
void AddEvent(nostd::string_view /*name*/, core::SystemTimestamp /*timestamp*/) noexcept override
{}

void AddEvent(nostd::string_view /*name*/,
core::SystemTimestamp /*timestamp*/,
const trace::KeyValueIterable & /*attributes*/) noexcept override
{}

void SetStatus(CanonicalCode code, nostd::string_view description) noexcept override {}
void SetStatus(CanonicalCode /*code*/, nostd::string_view /*description*/) noexcept override {}

void UpdateName(nostd::string_view name) noexcept override {}
void UpdateName(nostd::string_view /*name*/) noexcept override {}

void End() noexcept override {}

Expand Down
38 changes: 38 additions & 0 deletions api/include/opentelemetry/trace/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
#include <cstdint>

#include "opentelemetry/core/timestamp.h"
#include "opentelemetry/nostd/span.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/trace/canonical_code.h"
#include "opentelemetry/trace/key_value_iterable_view.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand Down Expand Up @@ -82,6 +84,42 @@ class Span
// virtual void AddEvent(nostd::string_view name, core::SteadyTimestamp
// timestamp, nostd::span<const std::pair<nostd::string_view name, AttributeValue
// value>> attributes) noexcept = 0;
virtual void AddEvent(nostd::string_view name,
core::SystemTimestamp timestamp,
const KeyValueIterable &attributes) noexcept = 0;

template <class T, nostd::enable_if_t<detail::is_key_value_iterable<T>::value> * = nullptr>
void AddEvent(nostd::string_view name,
core::SystemTimestamp timestamp,
const T &attributes) noexcept
{
this->AddEvent(name, timestamp, KeyValueIterableView<T>{attributes});
}

template <class T, nostd::enable_if_t<detail::is_key_value_iterable<T>::value> * = nullptr>
void AddEvent(nostd::string_view name, const T &attributes) noexcept
{
this->AddEvent(name, std::chrono::system_clock::now(), KeyValueIterableView<T>{attributes});
g-easy marked this conversation as resolved.
Show resolved Hide resolved
}

void AddEvent(nostd::string_view name,
core::SystemTimestamp timestamp,
std::initializer_list<std::pair<nostd::string_view, trace::AttributeValue>>
attributes) noexcept
{
this->AddEvent(name, timestamp,
nostd::span<const std::pair<nostd::string_view, trace::AttributeValue>>{
attributes.begin(), attributes.end()});
}

void AddEvent(nostd::string_view name,
std::initializer_list<std::pair<nostd::string_view, trace::AttributeValue>>
attributes) noexcept
{
this->AddEvent(name, std::chrono::system_clock::now(),
nostd::span<const std::pair<nostd::string_view, trace::AttributeValue>>{
attributes.begin(), attributes.end()});
}

// Sets the status of the span. The default status is OK. Only the value of the last call will be
// recorded, and implementations are free to ignore previous calls.
Expand Down
11 changes: 11 additions & 0 deletions api/test/trace/BUILD
Original file line number Diff line number Diff line change
@@ -1,5 +1,16 @@
load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")

cc_test(
name = "key_value_iterable_view_test",
srcs = [
"key_value_iterable_view_test.cc",
],
deps = [
"//api",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "noop_test",
srcs = [
Expand Down
4 changes: 2 additions & 2 deletions api/test/trace/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
foreach(testname noop_test provider_test span_id_test trace_id_test
trace_flags_test)
foreach(testname key_value_iterable_view_test noop_test provider_test
span_id_test trace_id_test trace_flags_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(${testname} ${GTEST_BOTH_LIBRARIES}
${CMAKE_THREAD_LIBS_INIT} opentelemetry_api)
Expand Down
65 changes: 65 additions & 0 deletions api/test/trace/key_value_iterable_view_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#include "opentelemetry/trace/key_value_iterable_view.h"

#include <map>

#include <gtest/gtest.h>

using namespace opentelemetry;

static int TakeKeyValues(const trace::KeyValueIterable &iterable)
{
std::map<std::string, trace::AttributeValue> result;
int count = 0;
iterable.ForEachKeyValue([&](nostd::string_view key, trace::AttributeValue value) noexcept {
++count;
return true;
});
return count;
}

template <class T, nostd::enable_if_t<trace::detail::is_key_value_iterable<T>::value> * = nullptr>
static int TakeKeyValues(const T &iterable)
{
return TakeKeyValues(trace::KeyValueIterableView<T>{iterable});
}

TEST(KeyValueIterableViewTest, is_key_value_iterable)
{
using M1 = std::map<std::string, std::string>;
EXPECT_TRUE(bool{trace::detail::is_key_value_iterable<M1>::value});

using M2 = std::map<std::string, int>;
EXPECT_TRUE(bool{trace::detail::is_key_value_iterable<M2>::value});

using M3 = std::map<std::string, trace::AttributeValue>;
EXPECT_TRUE(bool{trace::detail::is_key_value_iterable<M3>::value});

struct A
{};
using M4 = std::map<std::string, A>;
EXPECT_FALSE(bool{trace::detail::is_key_value_iterable<M4>::value});
}

TEST(KeyValueIterableViewTest, ForEachKeyValue)
{
std::map<std::string, std::string> m1 = {{"abc", "123"}, {"xyz", "456"}};
EXPECT_EQ(TakeKeyValues(m1), 2);

std::vector<std::pair<std::string, int>> v1 = {{"abc", 123}, {"xyz", 456}};
EXPECT_EQ(TakeKeyValues(v1), 2);
}

TEST(KeyValueIterableViewTest, ForEachKeyValueWithExit)
{
using M = std::map<std::string, std::string>;
M m1 = {{"abc", "123"}, {"xyz", "456"}};
trace::KeyValueIterableView<M> iterable{m1};
int count = 0;
auto exit = iterable.ForEachKeyValue([&count](nostd::string_view /*key*/,
trace::AttributeValue /*value*/) noexcept {
++count;
return false;
});
EXPECT_EQ(count, 1);
EXPECT_FALSE(exit);
}
13 changes: 13 additions & 0 deletions api/test/trace/noop_test.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#include "opentelemetry/trace/noop.h"

#include <map>
#include <memory>
#include <string>

#include <gtest/gtest.h>

Expand All @@ -12,4 +14,15 @@ TEST(NoopTest, UseNoopTracers)
std::shared_ptr<Tracer> tracer{new NoopTracer{}};
auto s1 = tracer->StartSpan("abc");
EXPECT_EQ(&s1->tracer(), tracer.get());

std::map<std::string, std::string> attributes1;
s1->AddEvent("abc", attributes1);

std::vector<std::pair<std::string, int>> attributes2;
s1->AddEvent("abc", attributes2);

s1->AddEvent("abc", {{"a", 1}, {"b", "2"}, {"c", 3.0}});

std::vector<std::pair<std::string, std::vector<int>>> attributes3;
s1->AddEvent("abc", attributes3);
}
5 changes: 5 additions & 0 deletions examples/plugin/plugin/tracer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class Span final : public trace::Span
void AddEvent(nostd::string_view /*name*/, core::SystemTimestamp /*timestamp*/) noexcept override
{}

void AddEvent(nostd::string_view /*name*/,
core::SystemTimestamp /*timestamp*/,
const trace::KeyValueIterable & /*attributes*/) noexcept override
{}

void SetStatus(trace::CanonicalCode /*code*/,
nostd::string_view /*description*/) noexcept override
{}
Expand Down
9 changes: 9 additions & 0 deletions sdk/src/trace/span.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,15 @@ void Span::AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) no
(void)timestamp;
}

void Span::AddEvent(nostd::string_view name,
core::SystemTimestamp timestamp,
const trace_api::KeyValueIterable &attributes) noexcept
{
(void)name;
(void)timestamp;
(void)attributes;
}

void Span::SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept
{
std::lock_guard<std::mutex> lock_guard{mu_};
Expand Down
4 changes: 4 additions & 0 deletions sdk/src/trace/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ class Span final : public trace_api::Span

void AddEvent(nostd::string_view name, core::SystemTimestamp timestamp) noexcept override;

void AddEvent(nostd::string_view name,
core::SystemTimestamp timestamp,
const trace_api::KeyValueIterable &attributes) noexcept override;

void SetStatus(trace_api::CanonicalCode code, nostd::string_view description) noexcept override;

void UpdateName(nostd::string_view name) noexcept override;
Expand Down