Skip to content

[SDK] support aggregation of identical instruments #3358

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
67ffb21
use the existing storage for sync or async instruments of the same na…
dbarker Apr 9, 2025
6ed9e02
add hash and name case insensitive hash for InstrumentDescriptor. Upd…
dbarker Apr 10, 2025
87d335b
don't allocate heap in the hash. fix some ci failures
dbarker Apr 10, 2025
0cbf6d2
fix a few more ci failures.
dbarker Apr 11, 2025
a85c892
move the instrument descriptor ostream operator to meter.cc to not le…
dbarker Apr 11, 2025
b8cec04
adds instrument descriptor tests
dbarker Apr 11, 2025
9383259
add comments
dbarker Apr 11, 2025
4e42598
Move case-insensitive equals method and IsDuplicate method into Instr…
dbarker Apr 14, 2025
8923de6
Merge remote-tracking branch 'origin/main' into fix_aggregate_identic…
dbarker Apr 14, 2025
3f311b8
fix iwyu errors
dbarker Apr 14, 2025
aa98843
duplicate instrument log message improvements to match spec. minor te…
dbarker Apr 15, 2025
812522d
Merge branch 'main' into fix_aggregate_identical_instruments
ThomsonTan Apr 16, 2025
a3c5fe3
Merge remote-tracking branch 'origin/main' into fix_aggregate_identic…
dbarker Apr 25, 2025
5cce7e8
changelog entry
dbarker Apr 25, 2025
22ed928
Merge branch 'main' into fix_aggregate_identical_instruments
ThomsonTan Apr 30, 2025
cb9c08f
address review feedback. Add Ascii to the name of the instrument util…
dbarker Apr 30, 2025
a5c5f46
address feedback: short circuit the instrument descriptor CaseInsensi…
dbarker Apr 30, 2025
8b7ed04
fix comments
dbarker Apr 30, 2025
53538dc
Merge branch 'main' into fix_aggregate_identical_instruments
marcalff May 3, 2025
6b60a17
Merge remote-tracking branch 'origin/main' into fix_aggregate_identic…
dbarker May 6, 2025
53b98c5
Merge branch 'main' into fix_aggregate_identical_instruments
lalitb May 9, 2025
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ Increment the:
* [SDK] Optimize PeriodicExportingMetricReader thread usage
[#3383](https://github.com/open-telemetry/opentelemetry-cpp/pull/3383)

* [SDK] Aggregate identical metrics instruments and detect duplicates
[#3358](https://github.com/open-telemetry/opentelemetry-cpp/pull/3358)

## [1.20 2025-04-01]

* [BUILD] Update opentelemetry-proto version
Expand Down
148 changes: 148 additions & 0 deletions sdk/include/opentelemetry/sdk/metrics/instruments.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

#pragma once

#include <algorithm>
#include <cctype>
#include <functional>
#include <string>

Expand Down Expand Up @@ -65,6 +67,152 @@ struct InstrumentDescriptor
InstrumentValueType value_type_;
};

struct InstrumentDescriptorUtil
{
// Case-insensitive comparison of two ASCII strings used to evaluate equality of instrument names
static bool CaseInsensitiveAsciiEquals(const std::string &lhs, const std::string &rhs) noexcept
{
return lhs.size() == rhs.size() &&
std::equal(lhs.begin(), lhs.end(), rhs.begin(), [](char a, char b) {
return std::tolower(static_cast<unsigned char>(a)) ==
std::tolower(static_cast<unsigned char>(b));
});
}

// Implementation of the specification requirements on duplicate instruments
// An instrument is a duplicate if it has the same name (case-insensitive) as another instrument,
// but different instrument kind, unit, or description.
// https://github.com/open-telemetry/opentelemetry-specification/blob/9c8c30631b0e288de93df7452f91ed47f6fba330/specification/metrics/sdk.md?plain=1#L869
static bool IsDuplicate(const InstrumentDescriptor &lhs, const InstrumentDescriptor &rhs) noexcept
{
// Not a duplicate if case-insensitive names are not equal
if (!InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals(lhs.name_, rhs.name_))
{
return false;
}

// Duplicate if names equal and kinds (Type and ValueType) are not equal
if (lhs.type_ != rhs.type_ || lhs.value_type_ != rhs.value_type_)
{
return true;
}

// Duplicate if names equal and units (case-sensitive) are not equal
if (lhs.unit_ != rhs.unit_)
{
return true;
}

// Duplicate if names equal and descriptions (case-sensitive) are not equal
if (lhs.description_ != rhs.description_)
{
return true;
}

// All identifying fields are equal
// These are identical instruments or only have a name case conflict
return false;
}

static opentelemetry::nostd::string_view GetInstrumentTypeString(InstrumentType type) noexcept
{
switch (type)
{
case InstrumentType::kCounter:
return "Counter";
case InstrumentType::kUpDownCounter:
return "UpDownCounter";
case InstrumentType::kHistogram:
return "Histogram";
case InstrumentType::kObservableCounter:
return "ObservableCounter";
case InstrumentType::kObservableUpDownCounter:
return "ObservableUpDownCounter";
case InstrumentType::kObservableGauge:
return "ObservableGauge";
case InstrumentType::kGauge:
return "Gauge";
default:
return "Unknown";
}
}

static opentelemetry::nostd::string_view GetInstrumentValueTypeString(
InstrumentValueType value_type) noexcept
{
switch (value_type)
{
case InstrumentValueType::kInt:
return "Int";
case InstrumentValueType::kLong:
return "Long";
case InstrumentValueType::kFloat:
return "Float";
case InstrumentValueType::kDouble:
return "Double";
default:
return "Unknown";
}
}
};

struct InstrumentEqualNameCaseInsensitive
{
bool operator()(const InstrumentDescriptor &lhs, const InstrumentDescriptor &rhs) const noexcept
{
// Names (case-insensitive)
if (!InstrumentDescriptorUtil::CaseInsensitiveAsciiEquals(lhs.name_, rhs.name_))
{
return false;
}

// Kinds (Type and ValueType)
if (lhs.type_ != rhs.type_ || lhs.value_type_ != rhs.value_type_)
{
return false;
}

// Units (case-sensitive)
if (lhs.unit_ != rhs.unit_)
{
return false;
}

// Descriptions (case-sensitive)
if (lhs.description_ != rhs.description_)
{
return false;
}

// All identifying fields are equal
return true;
}
};

// Hash for InstrumentDescriptor
// Identical instruments must have the same hash value
// Two instruments are identical when all identifying fields (case-insensitive name , kind,
// description, unit) are equal.
struct InstrumentDescriptorHash
{
std::size_t operator()(const InstrumentDescriptor &instrument_descriptor) const noexcept
{
std::size_t hashcode{};

for (char c : instrument_descriptor.name_)
{
sdk::common::GetHash(hashcode,
static_cast<char>(std::tolower(static_cast<unsigned char>(c))));
}

sdk::common::GetHash(hashcode, instrument_descriptor.description_);
sdk::common::GetHash(hashcode, instrument_descriptor.unit_);
sdk::common::GetHash(hashcode, static_cast<uint32_t>(instrument_descriptor.type_));
sdk::common::GetHash(hashcode, static_cast<uint32_t>(instrument_descriptor.value_type_));
return hashcode;
}
};

using MetricAttributes = opentelemetry::sdk::metrics::FilteredOrderedAttributeMap;
using MetricAttributesHash = opentelemetry::sdk::metrics::FilteredOrderedAttributeMapHash;
using AggregationTemporalitySelector = std::function<AggregationTemporality(InstrumentType)>;
Expand Down
21 changes: 19 additions & 2 deletions sdk/include/opentelemetry/sdk/metrics/meter.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,12 @@ class Meter final : public opentelemetry::metrics::Meter
// meter-context.
std::unique_ptr<sdk::instrumentationscope::InstrumentationScope> scope_;
std::weak_ptr<sdk::metrics::MeterContext> meter_context_;
// Mapping between instrument-name and Aggregation Storage.
std::unordered_map<std::string, std::shared_ptr<MetricStorage>> storage_registry_;
// Mapping between instrument descriptor and Aggregation Storage.
using MetricStorageMap = std::unordered_map<InstrumentDescriptor,
std::shared_ptr<MetricStorage>,
InstrumentDescriptorHash,
InstrumentEqualNameCaseInsensitive>;
MetricStorageMap storage_registry_;
std::shared_ptr<ObservableRegistry> observable_registry_;
MeterConfig meter_config_;
std::unique_ptr<SyncWritableMetricStorage> RegisterSyncMetricStorage(
Expand All @@ -164,6 +168,19 @@ class Meter final : public opentelemetry::metrics::Meter
return instrument_validator.ValidateName(name) && instrument_validator.ValidateUnit(unit) &&
instrument_validator.ValidateDescription(description);
}

// This function checks if the instrument is a duplicate of an existing one
// and emits a warning through the internal logger.
static void WarnOnDuplicateInstrument(
const sdk::instrumentationscope::InstrumentationScope *scope,
const MetricStorageMap &storage_registry,
const InstrumentDescriptor &new_instrument);

// This function checks if the instrument has a name case conflict with an existing one
// and emits a warning through the internal logger.
static void WarnOnNameCaseConflict(const sdk::instrumentationscope::InstrumentationScope *scope,
const InstrumentDescriptor &existing_instrument,
const InstrumentDescriptor &new_instrument);
};
} // namespace metrics
} // namespace sdk
Expand Down
Loading
Loading