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

Metrics SDK: Filtering metrics attributes #1191

Merged
merged 19 commits into from
Feb 5, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,25 @@ namespace metrics
{
using MetricAttributes = opentelemetry::sdk::common::AttributeMap;

/**
* The AttributesProcessor is responsible for customizing which
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this can be made internal/private to the SDK for now.

The specification only allows very limited manipulation on attributes (e.g. remove certain attributes from the measurement), this generic solution could work but it comes with perf cost (e.g. the virtual AttributesProcessor::process call).

Doesn't have to be a blocker though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I do suspect some perf cost associated with virtual and attributes copy. I have added a benchmark test to get cost statistics and would be helpful if have to consider performance improvement.

Also, I realized that we need to calculate the hash on the attribute map to store metrics data. Have added functionality to calculate the hash based on std::hash and boost::hash_combine`. As hash should be the same irrespective of the order of key/values in the AttributeMap, I have changed the internal storage for AttributeMap from unordered_map to map. This way we can avoid sorting of keys for every measurement.

Let me see how can we make this class internal/private to SDK, as C++ doesn't have a direct way of doing it. We can make AttributeProcessor::process() as private, and other SDK classes as its friend to it but would like to see a better approach if possible.

* attribute(s) are to be reported as metrics dimension(s).
*/

class AttributesProcessor
{
public:
// Process the metric instrument attributes.
// @returns The processed attributes
virtual MetricAttributes process(
const opentelemetry::common::KeyValueIterable &attributes) noexcept = 0;
};

/**
* DefaultAttributesProcessor returns copy of input instrument attributes without
* any modification.
*/

class DefaultAttributesProcessor : public AttributesProcessor
{
MetricAttributes process(
Expand All @@ -28,6 +40,39 @@ class DefaultAttributesProcessor : public AttributesProcessor
}
};

/**
* FilteringAttributesProcessor filters by allowed attribute names and drops any names
* that are not in the allow list.
*/

class FilteringAttributesProcessor : public AttributesProcessor
{
public:
FilteringAttributesProcessor(
const std::unordered_map<std::string, bool> allowed_attribute_keys = {})
: allowed_attribute_keys_(std::move(allowed_attribute_keys))
{}

MetricAttributes process(
const opentelemetry::common::KeyValueIterable &attributes) noexcept override
{
MetricAttributes result;
attributes.ForEachKeyValue(
[&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
if (allowed_attribute_keys_.find(key.data()) != allowed_attribute_keys_.end())
{
result.SetAttribute(key, value);
return true;
}
return true;
});
return result;
}

private:
std::unordered_map<std::string, bool> allowed_attribute_keys_;
};

} // namespace metrics
} // namespace sdk
OPENTELEMETRY_END_NAMESPACE
Expand Down
11 changes: 11 additions & 0 deletions sdk/test/metrics/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,14 @@ cc_test(
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "attributes_processor_test",
srcs = [
"attributes_processor_test.cc",
],
deps = [
"//sdk/src/metrics",
"@com_google_googletest//:gtest_main",
],
)
3 changes: 2 additions & 1 deletion sdk/test/metrics/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
foreach(testname meter_provider_sdk_test view_registry_test)
foreach(testname meter_provider_sdk_test view_registry_test
attributes_processor_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(
${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
Expand Down
49 changes: 49 additions & 0 deletions sdk/test/metrics/attributes_processor_test.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#ifndef ENABLE_METRICS_PREVIEW
# include "opentelemetry/sdk/metrics/view/attributes_processor.h"
# include <gtest/gtest.h>

using namespace opentelemetry::sdk::metrics;
using namespace opentelemetry::common;
using namespace opentelemetry::sdk::common;

TEST(AttributesProcessor, FilteringAttributesProcessor)
{
const int kNumFilterAttributes = 3;
std::unordered_map<std::string, bool> filter = {
{"attr2", true}, {"attr4", true}, {"attr6", true}};
const int kNumAttributes = 6;
std::string keys[kNumAttributes] = {"attr1", "attr2", "attr3", "attr4", "attr5", "attr6"};
int values[kNumAttributes] = {10, 20, 30, 40, 50, 60};
std::map<std::string, int> attributes = {{keys[0], values[0]}, {keys[1], values[1]},
{keys[2], values[2]}, {keys[3], values[3]},
{keys[4], values[4]}, {keys[5], values[5]}};
FilteringAttributesProcessor attributes_processor(filter);
opentelemetry::common::KeyValueIterableView<std::map<std::string, int>> iterable(attributes);
auto filtered_attributes = attributes_processor.process(iterable);
for (auto &e : filtered_attributes)
{
EXPECT_FALSE(filter.find(e.first) == filter.end());
}
EXPECT_EQ(filter.size(), kNumFilterAttributes);
}

TEST(AttributesProcessor, FilteringAllAttributesProcessor)
{
const int kNumFilterAttributes = 0;
std::unordered_map<std::string, bool> filter = {};
const int kNumAttributes = 6;
std::string keys[kNumAttributes] = {"attr1", "attr2", "attr3", "attr4", "attr5", "attr6"};
int values[kNumAttributes] = {10, 20, 30, 40, 50, 60};
std::map<std::string, int> attributes = {{keys[0], values[0]}, {keys[1], values[1]},
{keys[2], values[2]}, {keys[3], values[3]},
{keys[4], values[4]}, {keys[5], values[5]}};
FilteringAttributesProcessor attributes_processor(filter);
opentelemetry::common::KeyValueIterableView<std::map<std::string, int>> iterable(attributes);
auto filtered_attributes = attributes_processor.process(iterable);
EXPECT_EQ(filter.size(), kNumFilterAttributes);
}

#endif