Skip to content

Commit de059ec

Browse files
authored
[SDK] Delta temporality: use per-instrument creation time for first interval start_ts (#4144)
1 parent 28b623b commit de059ec

4 files changed

Lines changed: 134 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,9 @@ Increment the:
9090
* [CI] iwyu and clang-tidy: use install_thirdparty.sh for third-party
9191
[#4136](https://github.com/open-telemetry/opentelemetry-cpp/pull/4136)
9292

93+
* [SDK] Per-instrument creation time as delta first-interval start_ts
94+
[#4144](https://github.com/open-telemetry/opentelemetry-cpp/pull/4144)
95+
9396
## [1.27.0] 2026-05-13
9497

9598
* [RELEASE] Bump main branch to 1.27.0-dev

sdk/include/opentelemetry/sdk/metrics/state/temporal_metric_storage.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ class TemporalMetricStorage
5858
const AggregationConfig *aggregation_config_;
5959
opentelemetry::common::SystemTimestamp last_delta_collection_ts_;
6060
bool has_last_delta_collection_ts_ = false;
61+
// Captured at this storage's construction time. Per the OpenTelemetry
62+
// specification, the start_ts of the first delta collection interval MUST be
63+
// the creation time of the instrument, NOT the MeterProvider creation time.
64+
// See https://github.com/open-telemetry/opentelemetry-specification (logs/metrics
65+
// SDK specs) and issue #4062.
66+
const opentelemetry::common::SystemTimestamp instrument_creation_ts_;
6167
};
6268
} // namespace metrics
6369
} // namespace sdk

sdk/src/metrics/state/temporal_metric_storage.cc

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright The OpenTelemetry Authors
22
// SPDX-License-Identifier: Apache-2.0
33

4+
#include <chrono>
45
#include <list>
56
#include <memory>
67
#include <mutex>
@@ -34,7 +35,8 @@ TemporalMetricStorage::TemporalMetricStorage(InstrumentDescriptor instrument_des
3435
const AggregationConfig *aggregation_config)
3536
: instrument_descriptor_(std::move(instrument_descriptor)),
3637
aggregation_type_(aggregation_type),
37-
aggregation_config_(aggregation_config)
38+
aggregation_config_(aggregation_config),
39+
instrument_creation_ts_(std::chrono::system_clock::now())
3840
{}
3941

4042
bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
@@ -45,9 +47,16 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
4547
nostd::function_ref<bool(MetricData)> callback) noexcept
4648
{
4749
std::lock_guard<opentelemetry::common::SpinLockMutex> guard(lock_);
48-
opentelemetry::common::SystemTimestamp last_collection_ts = sdk_start_ts;
4950
AggregationTemporality aggregation_temporarily =
5051
collector->GetAggregationTemporality(instrument_descriptor_.type_);
52+
// Per OTel spec (issue #4062): the start_ts for the first delta collection
53+
// interval must be the instrument creation time, not the MeterProvider
54+
// start time (which is what sdk_start_ts carries). For cumulative, sdk_start_ts
55+
// is acceptable per spec ("creation of the instrument or the timestamp from
56+
// when the SDK was started, whichever happened first").
57+
opentelemetry::common::SystemTimestamp last_collection_ts =
58+
(aggregation_temporarily == AggregationTemporality::kDelta) ? instrument_creation_ts_
59+
: sdk_start_ts;
5160

5261
// Fast path for single collector with delta temporality and counter, updown-counter, histogram
5362
// This path doesn't need to aggregated-with/contribute-to the unreported_metric_, as there is
@@ -56,7 +65,7 @@ bool TemporalMetricStorage::buildMetrics(CollectorHandle *collector,
5665
{
5766
if (!has_last_delta_collection_ts_)
5867
{
59-
last_delta_collection_ts_ = sdk_start_ts;
68+
last_delta_collection_ts_ = instrument_creation_ts_;
6069
has_last_delta_collection_ts_ = true;
6170
}
6271
// If no metrics, early return

sdk/test/metrics/sync_metric_storage_counter_test.cc

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "common.h"
1414

1515
#include "opentelemetry/common/key_value_iterable_view.h"
16+
#include "opentelemetry/common/timestamp.h"
1617
#include "opentelemetry/context/context.h"
1718
#include "opentelemetry/nostd/function_ref.h"
1819
#include "opentelemetry/nostd/span.h"
@@ -379,10 +380,121 @@ TEST(SyncMetricStorageTest, DeltaCounterStartTimestampTracksEmptyCycles)
379380
// Check that the fast path correctly preserved the timestamp from the empty
380381
// collection cycle (cycle 2)
381382
EXPECT_EQ(metric_cycle3.start_ts, collection_ts2);
382-
EXPECT_EQ(metric_cycle1.start_ts, sdk_start_ts);
383+
// Per OTel spec (#4062), the first delta interval's start_ts is the
384+
// instrument's creation time (captured at storage construction above), not
385+
// the sdk_start_ts that the MeterProvider would otherwise pass in. Storage
386+
// was constructed before sdk_start_ts was sampled, so the start_ts must be
387+
// less-or-equal to sdk_start_ts. SystemTimestamp does not define ordering
388+
// operators directly, so compare via time_since_epoch().
389+
EXPECT_LE(metric_cycle1.start_ts.time_since_epoch(), sdk_start_ts.time_since_epoch());
383390
EXPECT_EQ(metric_cycle1.end_ts, collection_ts1);
384391
EXPECT_EQ(metric_cycle3.end_ts, collection_ts3);
385392
}
393+
394+
TEST(SyncMetricStorageTest, DeltaCounterFirstIntervalUsesInstrumentCreationTime)
395+
{
396+
// Spec-compliance test for issue #4062: the start_ts of the first delta
397+
// collection interval MUST be the per-instrument creation time, NOT the
398+
// MeterProvider/SDK start time. Simulate a MeterProvider that was created
399+
// long before the instrument by passing an artificial sdk_start_ts that is
400+
// strictly earlier than the storage's construction time.
401+
InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter,
402+
InstrumentValueType::kLong};
403+
std::shared_ptr<DefaultAttributesProcessor> default_attributes_processor{
404+
new DefaultAttributesProcessor{}};
405+
406+
auto before_creation = std::chrono::system_clock::now();
407+
opentelemetry::sdk::metrics::SyncMetricStorage storage(
408+
instr_desc, AggregationType::kSum, default_attributes_processor,
409+
#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
410+
ExemplarFilterType::kAlwaysOff, ExemplarReservoir::GetNoExemplarReservoir(),
411+
#endif
412+
nullptr);
413+
auto after_creation = std::chrono::system_clock::now();
414+
415+
// sdk_start_ts is intentionally well before storage construction.
416+
auto sdk_start_ts = before_creation - std::chrono::seconds(60);
417+
auto collection_ts = after_creation + std::chrono::seconds(1);
418+
419+
std::map<std::string, std::string> attributes = {{"RequestType", "GET"}};
420+
std::shared_ptr<CollectorHandle> collector(
421+
new MockCollectorHandle(AggregationTemporality::kDelta));
422+
std::vector<std::shared_ptr<CollectorHandle>> collectors;
423+
collectors.push_back(collector);
424+
425+
storage.RecordLong(10, KeyValueIterableView<std::map<std::string, std::string>>(attributes),
426+
opentelemetry::context::Context{});
427+
428+
MetricData metric_data;
429+
bool collected = false;
430+
storage.Collect(collector.get(), collectors, sdk_start_ts, collection_ts,
431+
[&](const MetricData &md) {
432+
metric_data = md;
433+
collected = true;
434+
return true;
435+
});
436+
ASSERT_TRUE(collected);
437+
438+
// start_ts must be the instrument creation time (between the two clock
439+
// samples taken around the storage constructor), and strictly greater than
440+
// the simulated MeterProvider start time. SystemTimestamp does not define
441+
// ordering operators directly; compare via time_since_epoch() durations.
442+
EXPECT_GE(metric_data.start_ts.time_since_epoch(), before_creation.time_since_epoch());
443+
EXPECT_LE(metric_data.start_ts.time_since_epoch(), after_creation.time_since_epoch());
444+
EXPECT_GT(metric_data.start_ts.time_since_epoch(), sdk_start_ts.time_since_epoch());
445+
EXPECT_EQ(metric_data.end_ts, collection_ts);
446+
}
447+
448+
TEST(SyncMetricStorageTest, DeltaCounterMultiCollectorFirstIntervalUsesInstrumentCreationTime)
449+
{
450+
// Slow-path counterpart of DeltaCounterFirstIntervalUsesInstrumentCreationTime.
451+
// With more than one collector, buildMetrics takes the slow path; that path
452+
// must also use instrument_creation_ts_ as the start_ts for the first delta
453+
// collection interval (issue #4062 explicitly mentions both paths).
454+
InstrumentDescriptor instr_desc = {"name", "desc", "1unit", InstrumentType::kCounter,
455+
InstrumentValueType::kLong};
456+
std::shared_ptr<DefaultAttributesProcessor> default_attributes_processor{
457+
new DefaultAttributesProcessor{}};
458+
459+
auto before_creation = std::chrono::system_clock::now();
460+
opentelemetry::sdk::metrics::SyncMetricStorage storage(
461+
instr_desc, AggregationType::kSum, default_attributes_processor,
462+
#ifdef ENABLE_METRICS_EXEMPLAR_PREVIEW
463+
ExemplarFilterType::kAlwaysOff, ExemplarReservoir::GetNoExemplarReservoir(),
464+
#endif
465+
nullptr);
466+
auto after_creation = std::chrono::system_clock::now();
467+
468+
auto sdk_start_ts = before_creation - std::chrono::seconds(60);
469+
auto collection_ts = after_creation + std::chrono::seconds(1);
470+
471+
std::map<std::string, std::string> attributes = {{"RequestType", "GET"}};
472+
// Two collectors force collectors.size() > 1, which bypasses the fast path.
473+
std::shared_ptr<CollectorHandle> collector_a(
474+
new MockCollectorHandle(AggregationTemporality::kDelta));
475+
std::shared_ptr<CollectorHandle> collector_b(
476+
new MockCollectorHandle(AggregationTemporality::kDelta));
477+
std::vector<std::shared_ptr<CollectorHandle>> collectors{collector_a, collector_b};
478+
479+
storage.RecordLong(10, KeyValueIterableView<std::map<std::string, std::string>>(attributes),
480+
opentelemetry::context::Context{});
481+
482+
MetricData metric_data;
483+
bool collected = false;
484+
storage.Collect(collector_a.get(), collectors, sdk_start_ts, collection_ts,
485+
[&](const MetricData &md) {
486+
metric_data = md;
487+
collected = true;
488+
return true;
489+
});
490+
ASSERT_TRUE(collected);
491+
492+
// Same assertion shape as the fast-path test.
493+
EXPECT_GE(metric_data.start_ts.time_since_epoch(), before_creation.time_since_epoch());
494+
EXPECT_LE(metric_data.start_ts.time_since_epoch(), after_creation.time_since_epoch());
495+
EXPECT_GT(metric_data.start_ts.time_since_epoch(), sdk_start_ts.time_since_epoch());
496+
EXPECT_EQ(metric_data.end_ts, collection_ts);
497+
}
386498
INSTANTIATE_TEST_SUITE_P(WritableMetricStorageTestDouble,
387499
WritableMetricStorageTestFixture,
388500
::testing::Values(AggregationTemporality::kCumulative,

0 commit comments

Comments
 (0)