Skip to content

Commit

Permalink
[SDK] Fix include instrumentation scope attributes in equal method (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
dbarker authored Dec 18, 2024
1 parent 0b94d71 commit 92bf8da
Show file tree
Hide file tree
Showing 11 changed files with 528 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ Increment the:
* [SDK] Do not frequently create and destroy http client threads
[#3198](https://github.com/open-telemetry/opentelemetry-cpp/pull/3198)

* [SDK] Fix instrumentation scope attributes evaluated in equal method
[#3214](https://github.com/open-telemetry/opentelemetry-cpp/pull/3214)

## [1.18 2024-11-25]

* [EXPORTER] Fix crash in ElasticsearchLogRecordExporter
Expand Down
83 changes: 83 additions & 0 deletions sdk/include/opentelemetry/sdk/common/attribute_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,60 @@ struct AttributeConverter
}
};

/**
* Evaluates if an owned value (from an OwnedAttributeValue) is equal to another value (from a
* non-owning AttributeValue). This only supports the checking equality with
* nostd::visit(AttributeEqualToVisitor, OwnedAttributeValue, AttributeValue).
*/
struct AttributeEqualToVisitor
{
// Different types are not equal including containers of different element types
template <typename T, typename U>
bool operator()(const T &, const U &) const noexcept
{
return false;
}

// Compare the same arithmetic types
template <typename T>
bool operator()(const T &owned_value, const T &value) const noexcept
{
return owned_value == value;
}

// Compare std::string and const char*
bool operator()(const std::string &owned_value, const char *value) const noexcept
{
return owned_value == value;
}

// Compare std::string and nostd::string_view
bool operator()(const std::string &owned_value, nostd::string_view value) const noexcept
{
return owned_value == value;
}

// Compare std::vector<std::string> and nostd::span<const nostd::string_view>
bool operator()(const std::vector<std::string> &owned_value,
const nostd::span<const nostd::string_view> &value) const noexcept
{
return owned_value.size() == value.size() &&
std::equal(owned_value.begin(), owned_value.end(), value.begin(),
[](const std::string &owned_element, nostd::string_view element) {
return owned_element == element;
});
}

// Compare nostd::span<const T> and std::vector<T> for arithmetic types
template <typename T>
bool operator()(const std::vector<T> &owned_value,
const nostd::span<const T> &value) const noexcept
{
return owned_value.size() == value.size() &&
std::equal(owned_value.begin(), owned_value.end(), value.begin());
}
};

/**
* Class for storing attributes.
*/
Expand Down Expand Up @@ -162,8 +216,37 @@ class AttributeMap : public std::unordered_map<std::string, OwnedAttributeValue>
(*this)[std::string(key)] = nostd::visit(converter_, value);
}

// Compare the attributes of this map with another KeyValueIterable
bool EqualTo(const opentelemetry::common::KeyValueIterable &attributes) const noexcept
{
if (attributes.size() != this->size())
{
return false;
}

const bool is_equal = attributes.ForEachKeyValue(
[this](nostd::string_view key,
const opentelemetry::common::AttributeValue &value) noexcept {
// Perform a linear search to find the key assuming the map is small
// This avoids temporary string creation from this->find(std::string(key))
for (const auto &kv : *this)
{
if (kv.first == key)
{
// Order of arguments is important here. OwnedAttributeValue is first then
// AttributeValue AttributeEqualToVisitor does not support the reverse order
return nostd::visit(equal_to_visitor_, kv.second, value);
}
}
return false;
});

return is_equal;
}

private:
AttributeConverter converter_;
AttributeEqualToVisitor equal_to_visitor_;
};

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ class InstrumentationScope
*/
bool operator==(const InstrumentationScope &other) const noexcept
{
return equal(other.name_, other.version_, other.schema_url_);
return this->name_ == other.name_ && this->version_ == other.version_ &&
this->schema_url_ == other.schema_url_ && this->attributes_ == other.attributes_;
}

/**
Expand All @@ -112,14 +113,31 @@ class InstrumentationScope
* @param name name of the instrumentation scope to compare.
* @param version version of the instrumentation scope to compare.
* @param schema_url schema url of the telemetry emitted by the scope.
* @param attributes attributes of the instrumentation scope to compare.
* @returns true if name and version in this instrumentation scope are equal with the given name
* and version.
*/
bool equal(const nostd::string_view name,
const nostd::string_view version,
const nostd::string_view schema_url = "") const noexcept
const nostd::string_view schema_url = "",
const opentelemetry::common::KeyValueIterable *attributes = nullptr) const noexcept
{
return this->name_ == name && this->version_ == version && this->schema_url_ == schema_url;

if (this->name_ != name || this->version_ != version || this->schema_url_ != schema_url)
{
return false;
}

if (attributes == nullptr)
{
if (attributes_.empty())
{
return true;
}
return false;
}

return attributes_.EqualTo(*attributes);
}

const std::string &GetName() const noexcept { return name_; }
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/logs/logger_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ opentelemetry::nostd::shared_ptr<opentelemetry::logs::Logger> LoggerProvider::Ge
{
auto &logger_lib = logger->GetInstrumentationScope();
if (logger->GetName() == logger_name &&
logger_lib.equal(library_name, library_version, schema_url))
logger_lib.equal(library_name, library_version, schema_url, &attributes))
{
return opentelemetry::nostd::shared_ptr<opentelemetry::logs::Logger>{logger};
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/metrics/meter_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ nostd::shared_ptr<metrics_api::Meter> MeterProvider::GetMeter(
for (auto &meter : context_->GetMeters())
{
auto meter_lib = meter->GetInstrumentationScope();
if (meter_lib->equal(name, version, schema_url))
if (meter_lib->equal(name, version, schema_url, attributes))
{
return nostd::shared_ptr<metrics_api::Meter>{meter};
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/trace/tracer_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ nostd::shared_ptr<trace_api::Tracer> TracerProvider::GetTracer(
for (auto &tracer : tracers_)
{
auto &tracer_scope = tracer->GetInstrumentationScope();
if (tracer_scope.equal(name, version, schema_url))
if (tracer_scope.equal(name, version, schema_url, attributes))
{
return nostd::shared_ptr<trace_api::Tracer>{tracer};
}
Expand Down
132 changes: 132 additions & 0 deletions sdk/test/common/attribute_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@
// SPDX-License-Identifier: Apache-2.0

#include <gtest/gtest.h>
#include <array>
#include <map>
#include <string>
#include <unordered_map>

#include "opentelemetry/common/key_value_iterable_view.h"
#include "opentelemetry/nostd/span.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/nostd/variant.h"
#include "opentelemetry/sdk/common/attribute_utils.h"

Expand Down Expand Up @@ -55,3 +58,132 @@ TEST(OrderedAttributeMapTest, AttributesConstruction)
EXPECT_EQ(opentelemetry::nostd::get<int>(attribute_map.GetAttributes().at(keys[i])), values[i]);
}
}

TEST(AttributeEqualToVisitorTest, AttributeValueEqualTo)
{
namespace sdk = opentelemetry::sdk::common;
namespace api = opentelemetry::common;
namespace nostd = opentelemetry::nostd;

using AV = api::AttributeValue;
using OV = sdk::OwnedAttributeValue;

sdk::AttributeEqualToVisitor equal_to_visitor;

// arithmetic types
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{bool(true)}, AV{bool(true)}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{int32_t(22)}, AV{int32_t(22)}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{int64_t(22)}, AV{int64_t(22)}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{uint32_t(22)}, AV{uint32_t(22)}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{uint64_t(22)}, AV{uint64_t(22)}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{double(22.0)}, AV{double(22.0)}));

// string types
EXPECT_TRUE(opentelemetry::nostd::visit(
equal_to_visitor, OV{std::string("string to const char*")}, AV{"string to const char*"}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor,
OV{std::string("string to string_view")},
AV{nostd::string_view("string to string_view")}));

// container types
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<bool>{true, false}},
AV{std::array<const bool, 2>{true, false}}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<uint8_t>{33, 44}},
AV{std::array<const uint8_t, 2>{33, 44}}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<int32_t>{33, 44}},
AV{std::array<const int32_t, 2>{33, 44}}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<int64_t>{33, 44}},
AV{std::array<const int64_t, 2>{33, 44}}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<uint32_t>{33, 44}},
AV{std::array<const uint32_t, 2>{33, 44}}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<uint64_t>{33, 44}},
AV{std::array<const uint64_t, 2>{33, 44}}));
EXPECT_TRUE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<double>{33.0, 44.0}},
AV{std::array<const double, 2>{33.0, 44.0}}));
EXPECT_TRUE(opentelemetry::nostd::visit(
equal_to_visitor, OV{std::vector<std::string>{"a string", "another string"}},
AV{std::array<const nostd::string_view, 2>{"a string", "another string"}}));
}

TEST(AttributeEqualToVisitorTest, AttributeValueNotEqualTo)
{
namespace sdk = opentelemetry::sdk::common;
namespace api = opentelemetry::common;
namespace nostd = opentelemetry::nostd;

using AV = api::AttributeValue;
using OV = sdk::OwnedAttributeValue;

sdk::AttributeEqualToVisitor equal_to_visitor;

// check different values of the same type
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{bool(true)}, AV{bool(false)}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{int32_t(22)}, AV{int32_t(33)}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{int64_t(22)}, AV{int64_t(33)}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{uint32_t(22)}, AV{uint32_t(33)}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{double(22.2)}, AV{double(33.3)}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::string("string one")},
AV{"another string"}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::string("string one")},
AV{nostd::string_view("another string")}));

// check different value types
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{bool(true)}, AV{uint32_t(1)}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{int32_t(22)}, AV{uint32_t(22)}));

// check containers of different element values
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<bool>{true, false}},
AV{std::array<const bool, 2>{false, true}}));
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<int32_t>{22, 33}},
AV{std::array<const int32_t, 2>{33, 44}}));
EXPECT_FALSE(opentelemetry::nostd::visit(
equal_to_visitor, OV{std::vector<std::string>{"a string", "another string"}},
AV{std::array<const nostd::string_view, 2>{"a string", "a really different string"}}));

// check containers of different element types
EXPECT_FALSE(opentelemetry::nostd::visit(equal_to_visitor, OV{std::vector<int32_t>{22, 33}},
AV{std::array<const uint32_t, 2>{22, 33}}));
}

TEST(AttributeMapTest, EqualTo)
{
using Attributes = std::initializer_list<
std::pair<opentelemetry::nostd::string_view, opentelemetry::common::AttributeValue>>;

// check for case where both are empty
Attributes attributes_empty = {};
auto kv_iterable_empty =
opentelemetry::common::MakeKeyValueIterableView<Attributes>(attributes_empty);
opentelemetry::sdk::common::AttributeMap attribute_map_empty(kv_iterable_empty);
EXPECT_TRUE(attribute_map_empty.EqualTo(kv_iterable_empty));

// check for equality with a range of attributes and types
Attributes attributes = {{"key0", "some value"}, {"key1", 1}, {"key2", 2.0}, {"key3", true}};
auto kv_iterable_match = opentelemetry::common::MakeKeyValueIterableView<Attributes>(attributes);
opentelemetry::sdk::common::AttributeMap attribute_map(attributes);
EXPECT_TRUE(attribute_map.EqualTo(kv_iterable_match));

// check for several cases where the attributes are different
Attributes attributes_different_value = {
{"key0", "some value"}, {"key1", 1}, {"key2", 2.0}, {"key3", false}};
Attributes attributes_different_type = {
{"key0", "some value"}, {"key1", 1.0}, {"key2", 2.0}, {"key3", true}};
Attributes attributes_different_size = {{"key0", "some value"}};

// check for the case where the number of attributes is the same but all keys are different
Attributes attributes_different_all = {{"a", "b"}, {"c", "d"}, {"e", 4.0}, {"f", uint8_t(5)}};

auto kv_iterable_different_value =
opentelemetry::common::MakeKeyValueIterableView<Attributes>(attributes_different_value);
auto kv_iterable_different_type =
opentelemetry::common::MakeKeyValueIterableView<Attributes>(attributes_different_type);
auto kv_iterable_different_size =
opentelemetry::common::MakeKeyValueIterableView<Attributes>(attributes_different_size);
auto kv_iterable_different_all =
opentelemetry::common::MakeKeyValueIterableView<Attributes>(attributes_different_all);

EXPECT_FALSE(attribute_map.EqualTo(kv_iterable_different_value));
EXPECT_FALSE(attribute_map.EqualTo(kv_iterable_different_type));
EXPECT_FALSE(attribute_map.EqualTo(kv_iterable_different_size));
EXPECT_FALSE(attribute_map.EqualTo(kv_iterable_different_all));
}
Loading

0 comments on commit 92bf8da

Please sign in to comment.