Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ Increment the:
`AlwaysOff`/`TraceBased`)
[#4267](https://github.com/open-telemetry/opentelemetry-cpp/pull/4267)

* [METRICS SDK] Avoid materializing owned exemplar attributes before
fixed-size reservoir selection.
[#4475](https://github.com/open-telemetry/opentelemetry-cpp/pull/4475)

Important changes:

* [API] Never set a null global provider or propagator
Expand Down Expand Up @@ -337,6 +341,13 @@ Breaking changes:
* This is an incompatible API and ABI change for custom exemplar reservoirs.
Implementations and callers must remove the timestamp parameter.

* [METRICS SDK] Add non-owning `KeyValueIterable` overloads to the preview
`ExemplarReservoir` and `ReservoirCellSelector` interfaces. Custom reservoir
implementations inherit compatibility adapters but must be rebuilt because
the SDK vtable changes. Custom selector implementations must additionally
implement the new `int64_t` and `double` overloads.
[#4475](https://github.com/open-telemetry/opentelemetry-cpp/pull/4475)

* [METRICS SDK] Breaking change to the preview metrics exemplar surface: the
`SyncMetricStorage`/`AsyncMetricStorage` constructors now take an
`ExemplarFilterType`, and `ExemplarData::Create` takes the `SpanContext`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# include <memory>
# include <vector>

# include "opentelemetry/common/key_value_iterable.h"
# include "opentelemetry/sdk/common/global_log_handler.h"
# include "opentelemetry/sdk/metrics/data/exemplar_data.h"
# include "opentelemetry/sdk/metrics/exemplar/filter_type.h"
Expand Down Expand Up @@ -49,18 +50,46 @@ class AlignedHistogramBucketExemplarReservoir : public FixedSizeExemplarReservoi
public:
HistogramCellSelector(const std::vector<double> &boundaries) : boundaries_(boundaries) {}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> &cells,
int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
int64_t value,
const MetricAttributes &attributes,
const opentelemetry::context::Context &context) override
const MetricAttributes & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return ReservoirCellIndexFor(cells, static_cast<double>(value), attributes, context);
return FindCellIndex(static_cast<double>(value));
}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
double value,
const MetricAttributes & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return FindCellIndex(value);
}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
int64_t value,
const opentelemetry::common::KeyValueIterable & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return FindCellIndex(static_cast<double>(value));
}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
double value,
const opentelemetry::common::KeyValueIterable & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return FindCellIndex(value);
}

public:
void reset() override
{
// Do nothing
}

private:
int FindCellIndex(double value) const
{
size_t max_size = boundaries_.size();
for (size_t i = 0; i < max_size; ++i)
Expand All @@ -75,13 +104,6 @@ class AlignedHistogramBucketExemplarReservoir : public FixedSizeExemplarReservoi
return static_cast<int>(max_size);
}

public:
void reset() override
{
// Do nothing
}

private:
std::vector<double> boundaries_;
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# include <memory>
# include <vector>

# include "opentelemetry/common/key_value_iterable.h"
# include "opentelemetry/context/context.h"
# include "opentelemetry/nostd/function_ref.h"
# include "opentelemetry/nostd/shared_ptr.h"
Expand Down Expand Up @@ -68,6 +69,40 @@ class FixedSizeExemplarReservoir : public ExemplarReservoir
}
}

void OfferMeasurement(int64_t value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept override
{
if (!reservoir_cell_selector_)
{
return;
}
auto idx =
reservoir_cell_selector_->ReservoirCellIndexFor(storage_, value, attributes, context);
if (idx != -1)
{
MetricAttributes owned_attributes{attributes};
storage_[idx].RecordLongMeasurement(value, owned_attributes, context);
}
}

void OfferMeasurement(double value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept override
{
if (!reservoir_cell_selector_)
{
return;
}
auto idx =
reservoir_cell_selector_->ReservoirCellIndexFor(storage_, value, attributes, context);
if (idx != -1)
{
MetricAttributes owned_attributes{attributes};
storage_[idx].RecordDoubleMeasurement(value, owned_attributes, context);
}
}

std::vector<std::shared_ptr<ExemplarData>> CollectAndReset(
const MetricAttributes &pointAttributes) noexcept override
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# include <memory>
# include <vector>

# include "opentelemetry/common/key_value_iterable.h"
# include "opentelemetry/common/timestamp.h"
# include "opentelemetry/context/context.h"
# include "opentelemetry/sdk/metrics/data/exemplar_data.h"
Expand Down Expand Up @@ -40,6 +41,20 @@ class NoExemplarReservoir final : public ExemplarReservoir
// Stores nothing.
}

void OfferMeasurement(int64_t /* value */,
const opentelemetry::common::KeyValueIterable & /* attributes */,
const opentelemetry::context::Context & /* context */) noexcept override
{
// Stores nothing.
}

void OfferMeasurement(double /* value */,
const opentelemetry::common::KeyValueIterable & /* attributes */,
const opentelemetry::context::Context & /* context */) noexcept override
{
// Stores nothing.
}

std::vector<std::shared_ptr<ExemplarData>> CollectAndReset(
const MetricAttributes & /* pointAttributes */) noexcept override
{
Expand Down
11 changes: 11 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/exemplar/reservoir.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# include <memory>
# include <vector>

# include "opentelemetry/common/key_value_iterable.h"
# include "opentelemetry/sdk/metrics/exemplar/reservoir_cell_selector.h"
# include "opentelemetry/version.h"

Expand Down Expand Up @@ -68,6 +69,16 @@ class ExemplarReservoir
virtual std::vector<std::shared_ptr<ExemplarData>> CollectAndReset(
const MetricAttributes &pointAttributes) noexcept = 0;

/** Offers a long measurement to be sampled. */
virtual void OfferMeasurement(int64_t value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept = 0;

/** Offers a long measurement to be sampled. */
virtual void OfferMeasurement(double value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) noexcept = 0;

static nostd::shared_ptr<ExemplarReservoir> GetSimpleFixedSizeExemplarReservoir(
size_t size,
const std::shared_ptr<ReservoirCellSelector> &reservoir_cell_selector,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# include <cstddef>
# include <vector>

# include "opentelemetry/common/key_value_iterable.h"
# include "opentelemetry/sdk/metrics/exemplar/filter_type.h"
# include "opentelemetry/sdk/metrics/exemplar/reservoir_cell.h"
# include "opentelemetry/version.h"
Expand Down Expand Up @@ -49,6 +50,18 @@ class ReservoirCellSelector

/** Called when {@link FixedSizeExemplarReservoir#CollectAndReset(Attributes)}. */
virtual void reset() = 0;

/** Determine the index of the {@code cells} to record the measurement to. */
virtual int ReservoirCellIndexFor(const std::vector<ReservoirCell> &cells,
int64_t value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) = 0;

/** Determine the index of the {@code cells} to record the measurement to. */
virtual int ReservoirCellIndexFor(const std::vector<ReservoirCell> &cells,
double value,
const opentelemetry::common::KeyValueIterable &attributes,
const opentelemetry::context::Context &context) = 0;
};

} // namespace metrics
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# include <memory>
# include <vector>

# include "opentelemetry/common/key_value_iterable.h"
# include "opentelemetry/sdk/metrics/data/exemplar_data.h"
# include "opentelemetry/sdk/metrics/exemplar/filter_type.h"
# include "opentelemetry/sdk/metrics/exemplar/fixed_size_exemplar_reservoir.h"
Expand Down Expand Up @@ -50,18 +51,42 @@ class SimpleFixedSizeExemplarReservoir : public FixedSizeExemplarReservoir
public:
SimpleFixedSizeCellSelector(size_t size) : size_(size) {}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> &cells,
int64_t value,
const MetricAttributes &attributes,
const opentelemetry::context::Context &context) override
int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
int64_t /* value */,
const MetricAttributes & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return ReservoirCellIndexFor(cells, static_cast<double>(value), attributes, context);
return SelectCell();
}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
double /* value */,
const MetricAttributes & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return SelectCell();
}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
int64_t /* value */,
const opentelemetry::common::KeyValueIterable & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return SelectCell();
}

int ReservoirCellIndexFor(const std::vector<ReservoirCell> & /* cells */,
double /* value */,
const opentelemetry::common::KeyValueIterable & /* attributes */,
const opentelemetry::context::Context & /* context */) override
{
return SelectCell();
}

void reset() override {}

private:
int SelectCell()
{
//
// The simple reservoir sampling algorithm from the spec below is used.
Expand All @@ -88,9 +113,6 @@ class SimpleFixedSizeExemplarReservoir : public FixedSizeExemplarReservoir
return static_cast<int>(index);
}

void reset() override {}

private:
size_t measurements_seen_ = 0;
size_t size_;
}; // class SimpleFixedSizeCellSelector
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
if (ExemplarFilterEnabled(exemplar_filter_type_, context))
{
exemplar_reservoir_->OfferMeasurement(value, {}, context);
exemplar_reservoir_->OfferMeasurement(value, opentelemetry::common::NoopKeyValueIterable{},
context);
}
#endif
static MetricAttributes attr = MetricAttributes{};
Expand Down Expand Up @@ -144,7 +145,8 @@ class SyncMetricStorage : public MetricStorage, public SyncWritableMetricStorage
#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
if (ExemplarFilterEnabled(exemplar_filter_type_, context))
{
exemplar_reservoir_->OfferMeasurement(value, {}, context);
exemplar_reservoir_->OfferMeasurement(value, opentelemetry::common::NoopKeyValueIterable{},
context);
}
#endif
static MetricAttributes attr = MetricAttributes{};
Expand Down
35 changes: 35 additions & 0 deletions sdk/test/metrics/exemplar/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,24 @@
# SPDX-License-Identifier: Apache-2.0

load("@rules_cc//cc:cc_test.bzl", "cc_test")
load("//bazel:otel_cc_benchmark.bzl", "otel_cc_benchmark")

otel_cc_benchmark(
name = "exemplar_offer_benchmark",
srcs = [
"exemplar_offer_benchmark.cc",
],
tags = [
"benchmark",
"metrics",
"test",
],
deps = [
"//api",
"//sdk:headers",
"//sdk/src/metrics",
],
)

cc_test(
name = "no_exemplar_reservoir_test",
Expand Down Expand Up @@ -37,6 +55,23 @@ cc_test(
],
)

cc_test(
name = "reservoir_attribute_offer_test",
srcs = [
"reservoir_attribute_offer_test.cc",
],
tags = [
"metrics",
"test",
],
deps = [
"//api",
"//sdk:headers",
"//sdk/src/metrics",
"@com_google_googletest//:gtest_main",
],
)

cc_test(
name = "aligned_histogram_bucket_exemplar_reservoir_test",
srcs = [
Expand Down
8 changes: 7 additions & 1 deletion sdk/test/metrics/exemplar/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
foreach(
testname
no_exemplar_reservoir_test aligned_histogram_bucket_exemplar_reservoir_test
reservoir_cell_test filter_predicate_test)
reservoir_attribute_offer_test reservoir_cell_test filter_predicate_test)
add_executable(${testname} "${testname}.cc")
target_link_libraries(
${testname} ${GTEST_BOTH_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT}
Expand All @@ -14,3 +14,9 @@ foreach(
TEST_PREFIX metrics.
TEST_LIST ${testname})
endforeach()

if(OTELCPP_WITH_BENCHMARK)
add_executable(exemplar_offer_benchmark exemplar_offer_benchmark.cc)
target_link_libraries(exemplar_offer_benchmark benchmark::benchmark
${CMAKE_THREAD_LIBS_INIT} opentelemetry_metrics)
endif()
Loading
Loading