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.

Sign up for GitHub

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

Jump to bottom

Fix lifetime for sdk::ReadWriteLogRecord #3147

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+421 −21
Open

Fix lifetime for sdk::ReadWriteLogRecord #3147

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa4471d
Fix lifetime for sdk::ReadWriteLogRecord
owent Nov 15, 2024
8c86bbd
Fix unit test
owent Nov 15, 2024
cae98c6
Fix unit test
owent Nov 15, 2024
b933ee8
Merge branch 'main' into fix_lifetime_in_log_record
owent Nov 15, 2024
cfd5db3
Fix a typo
owent Nov 16, 2024
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
3 changes: 3 additions & 0 deletions exporters/otlp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ if(BUILD_TESTING)
else()
find_library(GMOCK_LIB gmock PATH_SUFFIXES lib)
endif()
if(NOT GMOCK_LIB)
unset(GMOCK_LIB CACHE)
endif()
endif()

if(WITH_OTLP_GRPC)
Expand Down
3 changes: 3 additions & 0 deletions exporters/zipkin/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ if(BUILD_TESTING)
else()
find_library(GMOCK_LIB gmock PATH_SUFFIXES lib)
endif()
if(NOT GMOCK_LIB)
unset(GMOCK_LIB CACHE)
endif()

add_executable(zipkin_exporter_test test/zipkin_exporter_test.cc)

Expand Down
245 changes: 245 additions & 0 deletions sdk/include/opentelemetry/sdk/common/attribute_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <cstdint>
#include <initializer_list>
#include <map>
#include <memory>
#include <string>
#include <unordered_map>
#include <utility>
Expand Down Expand Up @@ -49,6 +50,8 @@ using OwnedAttributeValue = nostd::variant<bool,
std::vector<uint64_t>,
std::vector<uint8_t>>;

using OwnedAttributeView = nostd::variant<std::vector<nostd::string_view>, std::unique_ptr<bool[]>>;

enum OwnedAttributeType
{
kTypeBool,
Expand Down Expand Up @@ -215,6 +218,248 @@ class OrderedAttributeMap : public std::map<std::string, OwnedAttributeValue>
AttributeConverter converter_;
};

/**
* Class for storing attributes.
*/
struct MixedAttributeMapStorage
{
std::unordered_map<std::string, opentelemetry::common::AttributeValue> attributes;
AttributeMap owened_attributes;
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a typo:

Suggested change
AttributeMap owened_attributes;
AttributeMap owned_attributes;

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, it's fixed now.

std::unordered_map<std::string, OwnedAttributeView> owened_attributes_view;
};

/**
* Set an owned copy (OwnedAttributeValue) and attribute view of a non-owning AttributeValue.
*/
class MixedAttributeViewSetter
{
public:
inline MixedAttributeViewSetter(const nostd::string_view &key,
MixedAttributeMapStorage &storage,
AttributeConverter &converter) noexcept
: key_(&key), storage_(&storage), converter_(&converter)
{}

void operator()(bool v)
{
storage_->owened_attributes[std::string(*key_)] = (*converter_)(v);
storage_->attributes[std::string(*key_)] = v;
}

void operator()(int32_t v)
{
storage_->owened_attributes[std::string(*key_)] = (*converter_)(v);
storage_->attributes[std::string(*key_)] = v;
}

void operator()(uint32_t v)
{
storage_->owened_attributes[std::string(*key_)] = (*converter_)(v);
storage_->attributes[std::string(*key_)] = v;
}

void operator()(int64_t v)
{
storage_->owened_attributes[std::string(*key_)] = (*converter_)(v);
storage_->attributes[std::string(*key_)] = v;
}

void operator()(uint64_t v)
{
storage_->owened_attributes[std::string(*key_)] = (*converter_)(v);
storage_->attributes[std::string(*key_)] = v;
}

void operator()(double v)
{
storage_->owened_attributes[std::string(*key_)] = (*converter_)(v);
storage_->attributes[std::string(*key_)] = v;
}

void operator()(nostd::string_view v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::string>(owned_value);
}

void operator()(const char *v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::string>(owned_value).c_str();
}

void operator()(nostd::span<const uint8_t> v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::vector<uint8_t>>(owned_value);
}

void operator()(nostd::span<const bool> v)
{
storage_->owened_attributes[std::string(*key_)] = (*converter_)(v);
if (v.empty())
{
storage_->attributes[std::string(*key_)] = nostd::span<const bool>{};
}
else
{
std::unique_ptr<bool[]> owned_view{new bool[v.size()]};
for (size_t i = 0; i < v.size(); i++)
{
owned_view[i] = v[i];
}

storage_->attributes[std::string(*key_)] =
nostd::span<const bool>{owned_view.get(), v.size()};
storage_->owened_attributes_view[std::string(*key_)] = std::move(owned_view);
}
}

void operator()(nostd::span<const int32_t> v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::vector<int32_t>>(owned_value);
}

void operator()(nostd::span<const uint32_t> v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::vector<uint32_t>>(owned_value);
}

void operator()(nostd::span<const int64_t> v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::vector<int64_t>>(owned_value);
}

void operator()(nostd::span<const uint64_t> v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::vector<uint64_t>>(owned_value);
}

void operator()(nostd::span<const double> v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);
storage_->attributes[std::string(*key_)] = nostd::get<std::vector<double>>(owned_value);
}

void operator()(nostd::span<const nostd::string_view> v)
{
auto &owned_value = storage_->owened_attributes[std::string(*key_)];
owned_value = (*converter_)(v);

if (v.empty())
{
storage_->attributes[std::string(*key_)] = nostd::span<const nostd::string_view>{};
}
else
{
auto &owned_view = storage_->owened_attributes_view[std::string(*key_)];
owned_view = std::vector<nostd::string_view>{};
auto &owned_vector = nostd::get<std::vector<nostd::string_view>>(owned_view);

owned_vector.reserve(v.size());
for (auto &data : nostd::get<std::vector<std::string>>(owned_value))
{
owned_vector.push_back(data);
}

storage_->attributes[std::string(*key_)] =
nostd::span<const nostd::string_view>{owned_vector.data(), owned_vector.size()};
}
}

private:
const nostd::string_view *key_;
MixedAttributeMapStorage *storage_;
AttributeConverter *converter_;
};

/**
* Class for storing attributes and attribute view.
*/
class MixedAttributeMap
{
public:
// Construct empty attribute map
MixedAttributeMap() {}

// Construct attribute map and populate with attributes
MixedAttributeMap(const opentelemetry::common::KeyValueIterable &attributes) : MixedAttributeMap()
{
attributes.ForEachKeyValue(
[&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
nostd::visit(MixedAttributeViewSetter(key, storage_, converter_), value);
return true;
});
}

// Construct attribute map and populate with optional attributes
MixedAttributeMap(const opentelemetry::common::KeyValueIterable *attributes) : MixedAttributeMap()
{
if (attributes != nullptr)
{
attributes->ForEachKeyValue(
[&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
nostd::visit(MixedAttributeViewSetter(key, storage_, converter_), value);
return true;
});
}
}

// Construct map from initializer list by applying `SetAttribute` transform for every attribute
MixedAttributeMap(
std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
attributes)
: MixedAttributeMap()
{
for (auto &kv : attributes)
{
nostd::visit(MixedAttributeViewSetter(kv.first, storage_, converter_), kv.second);
}
}

// Returns a reference to this map
const std::unordered_map<std::string, opentelemetry::common::AttributeValue> &GetAttributes()
const noexcept
{
return storage_.attributes;
}

const AttributeMap &GetOwnedAttributes() const noexcept { return storage_.owened_attributes; }

// Convert non-owning key-value to owning std::string(key) and OwnedAttributeValue(value)
void SetAttribute(nostd::string_view key,
const opentelemetry::common::AttributeValue &value) noexcept
{
nostd::visit(MixedAttributeViewSetter(key, storage_, converter_), value);
}

void Reserve(AttributeMap::size_type size)
{
storage_.attributes.reserve(size);
storage_.owened_attributes.reserve(size);
}

AttributeMap::size_type Size() const noexcept { return storage_.attributes.size(); }

bool Empty() const noexcept { return storage_.attributes.empty(); }

private:
MixedAttributeMapStorage storage_;
AttributeConverter converter_;
};

} // namespace common
} // namespace sdk
OPENTELEMETRY_END_NAMESPACE
5 changes: 3 additions & 2 deletions sdk/include/opentelemetry/sdk/logs/read_write_log_record.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,9 @@ class ReadWriteLogRecord final : public ReadableLogRecord
const opentelemetry::sdk::resource::Resource *resource_;
const opentelemetry::sdk::instrumentationscope::InstrumentationScope *instrumentation_scope_;

std::unordered_map<std::string, opentelemetry::common::AttributeValue> attributes_map_;
opentelemetry::common::AttributeValue body_;
common::MixedAttributeMap attributes_map_;
// We resue the same utility functions of MixedAttributeMap with key="" for the body field
common::MixedAttributeMap body_;
opentelemetry::common::SystemTimestamp timestamp_;
opentelemetry::common::SystemTimestamp observed_timestamp_;

Expand Down
13 changes: 7 additions & 6 deletions sdk/src/logs/read_write_log_record.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ ReadWriteLogRecord::ReadWriteLogRecord()
: severity_(opentelemetry::logs::Severity::kInvalid),
resource_(nullptr),
instrumentation_scope_(nullptr),
body_(nostd::string_view()),
observed_timestamp_(std::chrono::system_clock::now()),
event_id_(0),
event_name_("")
{}
{
body_.SetAttribute("", nostd::string_view());
}

ReadWriteLogRecord::~ReadWriteLogRecord() {}

Expand Down Expand Up @@ -70,12 +71,12 @@ opentelemetry::logs::Severity ReadWriteLogRecord::GetSeverity() const noexcept

void ReadWriteLogRecord::SetBody(const opentelemetry::common::AttributeValue &message) noexcept
{
body_ = message;
body_.SetAttribute("", message);
}

const opentelemetry::common::AttributeValue &ReadWriteLogRecord::GetBody() const noexcept
{
return body_;
return body_.GetAttributes().begin()->second;
}

void ReadWriteLogRecord::SetEventId(int64_t id, nostd::string_view name) noexcept
Expand Down Expand Up @@ -160,13 +161,13 @@ const opentelemetry::trace::TraceFlags &ReadWriteLogRecord::GetTraceFlags() cons
void ReadWriteLogRecord::SetAttribute(nostd::string_view key,
const opentelemetry::common::AttributeValue &value) noexcept
{
attributes_map_[static_cast<std::string>(key)] = value;
attributes_map_.SetAttribute(key, value);
}

const std::unordered_map<std::string, opentelemetry::common::AttributeValue> &
ReadWriteLogRecord::GetAttributes() const noexcept
{
return attributes_map_;
return attributes_map_.GetAttributes();
}

const opentelemetry::sdk::resource::Resource &ReadWriteLogRecord::GetResource() const noexcept
Expand Down
Loading