Skip to content

Commit

Permalink
Fix attribute filtering for synchronous instruments. (#2472)
Browse files Browse the repository at this point in the history
  • Loading branch information
lalitb authored Feb 8, 2024
1 parent d32f960 commit cf5cdaa
Show file tree
Hide file tree
Showing 12 changed files with 213 additions and 29 deletions.
4 changes: 2 additions & 2 deletions sdk/include/opentelemetry/sdk/metrics/data/exemplar_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
#include <memory>

#include "opentelemetry/common/timestamp.h"
#include "opentelemetry/sdk/common/attribute_utils.h"
#include "opentelemetry/sdk/metrics/data/metric_data.h"
#include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
#include "opentelemetry/trace/span_context.h"
#include "opentelemetry/version.h"

Expand All @@ -16,7 +16,7 @@ namespace sdk
{
namespace metrics
{
using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap;
using MetricAttributes = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap;
/**
* A sample input measurement.
*
Expand Down
4 changes: 2 additions & 2 deletions sdk/include/opentelemetry/sdk/metrics/exemplar/filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

#include <memory>

#include "opentelemetry/sdk/common/attribute_utils.h"
#include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand All @@ -23,7 +23,7 @@ class OrderedAttributeMap;

namespace metrics
{
using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap;
using MetricAttributes = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap;

/**
* Exemplar filters are used to pre-filter measurements before attempting to store them in a
Expand Down
4 changes: 2 additions & 2 deletions sdk/include/opentelemetry/sdk/metrics/instruments.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#include <functional>
#include <string>

#include "opentelemetry/sdk/common/attribute_utils.h"
#include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
Expand Down Expand Up @@ -63,7 +63,7 @@ struct InstrumentDescriptor
InstrumentValueType value_type_;
};

using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap;
using MetricAttributes = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap;
using AggregationTemporalitySelector = std::function<AggregationTemporality(InstrumentType)>;

/*class InstrumentSelector {
Expand Down
10 changes: 1 addition & 9 deletions sdk/include/opentelemetry/sdk/metrics/observer_result.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,7 @@ class ObserverResultT final : public opentelemetry::metrics::ObserverResultT<T>

void Observe(T value, const opentelemetry::common::KeyValueIterable &attributes) noexcept override
{
if (attributes_processor_)
{
auto attr = attributes_processor_->process(attributes);
data_.insert({attr, value});
}
else
{
data_.insert({MetricAttributes{attributes}, value});
}
data_.insert({MetricAttributes{attributes, attributes_processor_}, value});
}

const std::unordered_map<MetricAttributes, T, AttributeHashGenerator> &GetMeasurements()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ class AttributesHashMap
* value and store in the hash
*/
Aggregation *GetOrSetDefault(const opentelemetry::common::KeyValueIterable &attributes,
const AttributesProcessor *attributes_processor,
std::function<std::unique_ptr<Aggregation>()> aggregation_callback,
size_t hash)
{
Expand All @@ -83,7 +84,7 @@ class AttributesHashMap
return GetOrSetOveflowAttributes(aggregation_callback);
}

MetricAttributes attr{attributes};
MetricAttributes attr{attributes, attributes_processor};

hash_map_[hash] = {attr, aggregation_callback()};
return hash_map_[hash].second.get();
Expand Down Expand Up @@ -133,6 +134,7 @@ class AttributesHashMap
* Set the value for given key, overwriting the value if already present
*/
void Set(const opentelemetry::common::KeyValueIterable &attributes,
const AttributesProcessor *attributes_processor,
std::unique_ptr<Aggregation> aggr,
size_t hash)
{
Expand All @@ -149,7 +151,7 @@ class AttributesHashMap
}
else
{
MetricAttributes attr{attributes};
MetricAttributes attr{attributes, attributes_processor};
hash_map_[hash] = {attr, std::move(aggr)};
}
}
Expand All @@ -169,8 +171,7 @@ class AttributesHashMap
}
else
{
MetricAttributes attr{attributes};
hash_map_[hash] = {attr, std::move(aggr)};
hash_map_[hash] = {attributes, std::move(aggr)};
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#pragma once

#include <map>
#include <string>
#include "opentelemetry/sdk/common/attributemap_hash.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
{
namespace metrics
{
class AttributesProcessor;
class FilteredOrderedAttributeMap : public opentelemetry::sdk::common::OrderedAttributeMap
{
public:
FilteredOrderedAttributeMap() = default;
FilteredOrderedAttributeMap(
std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
attributes)
: OrderedAttributeMap(attributes)
{}
FilteredOrderedAttributeMap(const opentelemetry::common::KeyValueIterable &attributes)
: FilteredOrderedAttributeMap(attributes, nullptr)
{}
FilteredOrderedAttributeMap(const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::sdk::metrics::AttributesProcessor *processor);
FilteredOrderedAttributeMap(
std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
attributes,
const opentelemetry::sdk::metrics::AttributesProcessor *processor);
};
} // namespace metrics
} // namespace sdk
OPENTELEMETRY_END_NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
});

std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
attributes_hashmap_->GetOrSetDefault(attributes, create_default_aggregation_, hash)
attributes_hashmap_
->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, hash)
->Aggregate(value);
}

Expand Down Expand Up @@ -148,7 +149,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
}
});
std::lock_guard<opentelemetry::common::SpinLockMutex> guard(attribute_hashmap_lock_);
attributes_hashmap_->GetOrSetDefault(attributes, create_default_aggregation_, hash)
attributes_hashmap_
->GetOrSetDefault(attributes, attributes_processor_, create_default_aggregation_, hash)
->Aggregate(value);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,18 @@

#include <string>
#include <unordered_map>

#include "opentelemetry/common/key_value_iterable.h"
#include "opentelemetry/nostd/string_view.h"
#include "opentelemetry/sdk/common/attribute_utils.h"
#include "opentelemetry/sdk/metrics/instruments.h"
#include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
#include "opentelemetry/version.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
{
namespace metrics
{
using MetricAttributes = opentelemetry::sdk::common::OrderedAttributeMap;
using MetricAttributes = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap;

/**
* The AttributesProcessor is responsible for customizing which
Expand Down
1 change: 1 addition & 0 deletions sdk/src/metrics/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ add_library(
instrument_metadata_validator.cc
export/periodic_exporting_metric_reader.cc
export/periodic_exporting_metric_reader_factory.cc
state/filtered_ordered_attribute_map.cc
state/metric_collector.cc
state/observable_registry.cc
state/sync_metric_storage.cc
Expand Down
43 changes: 43 additions & 0 deletions sdk/src/metrics/state/filtered_ordered_attribute_map.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

#include "opentelemetry/sdk/metrics/state/filtered_ordered_attribute_map.h"
#include "opentelemetry/sdk/metrics/view/attributes_processor.h"

OPENTELEMETRY_BEGIN_NAMESPACE
namespace sdk
{
namespace metrics
{
FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
const opentelemetry::common::KeyValueIterable &attributes,
const AttributesProcessor *processor)
: OrderedAttributeMap()
{
attributes.ForEachKeyValue(
[&](nostd::string_view key, opentelemetry::common::AttributeValue value) noexcept {
if (processor && processor->isPresent(key))
{
SetAttribute(key, value);
}
return true;
});
}

FilteredOrderedAttributeMap::FilteredOrderedAttributeMap(
std::initializer_list<std::pair<nostd::string_view, opentelemetry::common::AttributeValue>>
attributes,
const AttributesProcessor *processor)
: OrderedAttributeMap()
{
for (auto &kv : attributes)
{
if (processor && processor->isPresent(kv.first))
{
SetAttribute(kv.first, kv.second);
}
}
}
} // namespace metrics
} // namespace sdk
OPENTELEMETRY_END_NAMESPACE
10 changes: 5 additions & 5 deletions sdk/test/metrics/cardinality_limit_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
long record_value = 100;
for (auto i = 0; i < 10; i++)
{
OrderedAttributeMap attributes = {{"key", std::to_string(i)}};
auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
static_cast<LongSumAggregation *>(
hash_map.GetOrSetDefault(attributes, aggregation_callback, hash))
->Aggregate(record_value);
Expand All @@ -37,16 +37,16 @@ TEST(CardinalityLimit, AttributesHashMapBasicTests)
// overflowmetric point.
for (auto i = 10; i < 15; i++)
{
OrderedAttributeMap attributes = {{"key", std::to_string(i)}};
auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
FilteredOrderedAttributeMap attributes = {{"key", std::to_string(i)}};
auto hash = opentelemetry::sdk::common::GetHashForAttributeMap(attributes);
static_cast<LongSumAggregation *>(
hash_map.GetOrSetDefault(attributes, aggregation_callback, hash))
->Aggregate(record_value);
}
EXPECT_EQ(hash_map.Size(), 10); // only one more metric point should be added as overflow.
// get the overflow metric point
auto agg = hash_map.GetOrSetDefault(
OrderedAttributeMap({{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}}),
FilteredOrderedAttributeMap({{kAttributesLimitOverflowKey, kAttributesLimitOverflowValue}}),
aggregation_callback, kOverflowAttributesHash);
EXPECT_NE(agg, nullptr);
auto sum_agg = static_cast<LongSumAggregation *>(agg);
Expand Down
Loading

1 comment on commit cf5cdaa

@github-actions
Copy link

Choose a reason for hiding this comment

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

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: cf5cdaa Previous: d32f960 Ratio
BM_BaselineBuffer/2 16782052.516937256 ns/iter 5712235.450744629 ns/iter 2.94

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.