Skip to content

Add preview support for bound instruments #4042

Open
lalitb wants to merge 4 commits into
open-telemetry:mainfrom
lalitb:lalitb/bound-sync-instruments-preview
Open

Add preview support for bound instruments #4042
lalitb wants to merge 4 commits into
open-telemetry:mainfrom
lalitb:lalitb/bound-sync-instruments-preview

Conversation

@lalitb
Copy link
Copy Markdown
Member

@lalitb lalitb commented May 2, 2026

Summary

Related specs PR - open-telemetry/opentelemetry-specification#5050

Adds experimental bound synchronous metric instruments for metrics ABI v2. Bound instruments let callers bind a fixed attribute set once, then record repeated measurements through a bound handle:

auto counter = meter->CreateDoubleCounter("http.server.requests");

std::map<std::string, std::string> checkout_200_attrs = {
    {"route", "/checkout"},
    {"status", "200"},
};

std::map<std::string, std::string> checkout_500_attrs = {
    {"route", "/checkout"},
    {"status", "500"},
};

auto checkout_200 = counter->Bind(
    opentelemetry::common::KeyValueIterableView<std::map<std::string, std::string>>(
        checkout_200_attrs));

auto checkout_500 = counter->Bind(
    opentelemetry::common::KeyValueIterableView<std::map<std::string, std::string>>(
        checkout_500_attrs));

checkout_200->Add(1);
checkout_500->Add(1);

Both bound handles record to the same metric instrument, http.server.requests, with different pre-bound attribute sets. This avoids repeated per-call attribute processing and hashmap lookup for hot paths that reuse the same attribute set.

Scope

This PR intentionally keeps the preview surface narrow:

  • Counter: BoundCounter + Counter::Bind(...)
  • Histogram: BoundHistogram + Histogram::Bind(...)
  • SDK support for bound sync metric storage
  • multi-storage fanout support
  • noop support
  • bound-vs-unbound benchmark coverage
  • Bazel and CMake feature-gate support

Not included in this preview:

  • UpDownCounter bind support
  • Gauge bind support
  • Context-bearing bound recording overloads
  • exemplar parity for bound recordings

Those are left as follow-ups while the spec is still being finalized.

Feature Gate

The feature is disabled by default.

CMake:

  -DWITH_ABI_VERSION_2=ON
  -DWITH_METRICS_BOUND_INSTRUMENTS_PREVIEW=ON

Bazel:

  --//api:abi_version_no=2
  --//api:with_metrics_bound_instruments_preview=true

CMake rejects enabling WITH_METRICS_BOUND_INSTRUMENTS_PREVIEW without ABI v2.

Bound code is guarded by: OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW

Implementation Notes

  • Bound handles use the same metric name, description, unit, views, and export pipeline as the original instrument.
  • Attributes are processed once at bind time, then reused for later recordings.
  • Bound and unbound recordings with the same attributes are merged into the same exported datapoint.
  • Bound and unbound recordings share the same cardinality limit and overflow behavior.
  • Delta and cumulative temporality are handled by the existing metrics collection path.
  • Bound handles are safe to call after SDK storage is gone, but those late recordings are not exported.

Benchmark

Benchmark Mean Median StdDev
BM_UnboundFixedAttrsCounter 262 ns 262 ns 1.19 ns
BM_BoundFixedAttrsCounter 14.3 ns 14.9 ns 0.74 ns

Bound recording is about 18.3x faster by mean for repeated recordings with the same fixed attribute set.
Env: Ubuntu 24.04 · GCC 13.3 · AMD EPYC 9V45 (32C/64T) · 251 GiB RAM.

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

@lalitb lalitb changed the title Lalitb/bound sync instruments preview Add preview support for bound sync metric instruments May 2, 2026
@lalitb lalitb changed the title Add preview support for bound sync metric instruments [WIP] Add preview support for bound sync metric instruments May 2, 2026
@lalitb lalitb changed the title [WIP] Add preview support for bound sync metric instruments [WIP] Add preview support for bound instruments May 2, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 2, 2026

Codecov Report

❌ Patch coverage is 81.38528% with 43 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.05%. Comparing base (5533a30) to head (4f9b08c).

Files with missing lines Patch % Lines
sdk/src/metrics/sync_instruments.cc 62.50% 30 Missing ⚠️
sdk/src/metrics/state/sync_metric_storage.cc 94.12% 5 Missing ⚠️
...telemetry/sdk/metrics/state/multi_metric_storage.h 78.95% 4 Missing ⚠️
...e/opentelemetry/sdk/metrics/state/metric_storage.h 50.00% 2 Missing ⚠️
...ntelemetry/sdk/metrics/state/sync_metric_storage.h 93.55% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4042      +/-   ##
==========================================
+ Coverage   82.00%   82.05%   +0.06%     
==========================================
  Files         385      385              
  Lines       16031    16255     +224     
==========================================
+ Hits        13144    13336     +192     
- Misses       2887     2919      +32     
Files with missing lines Coverage Δ
api/include/opentelemetry/metrics/noop.h 74.03% <100.00%> (+3.02%) ⬆️
...i/include/opentelemetry/metrics/sync_instruments.h 100.00% <100.00%> (ø)
...clude/opentelemetry/sdk/metrics/sync_instruments.h 100.00% <ø> (ø)
...e/opentelemetry/sdk/metrics/state/metric_storage.h 80.00% <50.00%> (-20.00%) ⬇️
...ntelemetry/sdk/metrics/state/sync_metric_storage.h 82.28% <93.55%> (+4.51%) ⬆️
...telemetry/sdk/metrics/state/multi_metric_storage.h 90.25% <78.95%> (-9.75%) ⬇️
sdk/src/metrics/state/sync_metric_storage.cc 94.69% <94.12%> (-5.31%) ⬇️
sdk/src/metrics/sync_instruments.cc 66.47% <62.50%> (+0.06%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@lalitb lalitb force-pushed the lalitb/bound-sync-instruments-preview branch 5 times, most recently from 1fc14cf to 3690e7e Compare May 6, 2026 02:27
// This macro affects SDK class layout/vtables and MUST match between the SDK
// library build and consumer translation units.
#if OPENTELEMETRY_ABI_VERSION_NO >= 2 && defined(ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW)
# define OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW 1
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I kept this derived preview macro in version.h so API and SDK headers use one consistent guard for the ABI-v2 + preview-flag combination. This is temporary while bound instruments are experimental; once the API is stabilized, we can remove the preview flag and keep the surface under the normal ABI rules.

@lalitb lalitb marked this pull request as ready for review May 6, 2026 02:33
@lalitb lalitb requested a review from a team as a code owner May 6, 2026 02:33
@lalitb
Copy link
Copy Markdown
Member Author

lalitb commented May 6, 2026

this is ready for review now. This is preview-only and gated behind ABI v2 plus ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW, so existing users should not see the new API unless they opt in. The scope is Counter/Histogram bound handles for now, with the preview flag left in place while the API/spec settles.

@lalitb lalitb changed the title [WIP] Add preview support for bound instruments Add preview support for bound instruments May 6, 2026
@lalitb
Copy link
Copy Markdown
Member Author

lalitb commented May 6, 2026

I also did a local throwaway stress test on a 64-core test VM to sanity-check the expected performance shape using the stress framework #3241

The result matches the design expectation: bound instruments help most when the workload records repeatedly to stable attribute sets, because the hot path avoids repeated attribute processing and hashmap lookup. The benefit is small when all threads record to the same single series, since the same bound cell becomes the contention point. With many pre-bound series, the bound path scaled much better because contention was spread across many handles.

So I would not describe the largest speedup number as a general result, but the trend is encouraging and supports the usefulness of the preview API for high-concurrency, stable-attribute workloads.

For example, with 2 attributes per series and 64 threads, the local stress test showed roughly:

  • 1 series: bound was about 2x faster
  • 1,024 series: bound was about 70x faster
  • 16,384 series: bound was much faster still, but I would treat that as a stress upper bound rather than a general claim

Copy link
Copy Markdown
Member

@dbarker dbarker left a comment

Choose a reason for hiding this comment

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

Thanks for the feature. I'm excited to try it. Please see some initial feedback/questions below.

Comment thread CMakeLists.txt Outdated
Comment thread test_common/cmake/preview-options.cmake
Comment thread sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h Outdated
Comment thread sdk/src/metrics/state/sync_metric_storage.cc
Comment thread sdk/src/metrics/state/sync_metric_storage.cc
Comment thread sdk/src/metrics/sync_instruments.cc Outdated
Comment thread sdk/src/metrics/sync_instruments.cc
Comment thread sdk/test/metrics/CMakeLists.txt Outdated
Comment thread sdk/test/metrics/measurements_benchmark.cc
@lalitb lalitb force-pushed the lalitb/bound-sync-instruments-preview branch 2 times, most recently from 2784346 to d64810f Compare June 2, 2026 00:06
@lalitb
Copy link
Copy Markdown
Member Author

lalitb commented Jun 2, 2026

@dbarker Thanks for the review. I’ve addressed the comments and pushed the updates. Please let me know if you spot anything else.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces an experimental, feature-gated “bound synchronous instruments” preview for metrics ABI v2 in OpenTelemetry C++. It adds API and SDK support for binding a fixed attribute set once and recording repeated measurements via a bound handle, reducing per-call attribute processing overhead.

Changes:

  • Adds new API surface for Counter<T>::Bind(...) / Histogram<T>::Bind(...) and corresponding BoundCounter<T> / BoundHistogram<T> interfaces (plus noop implementations), guarded by OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW.
  • Implements SDK-side bound storage (SyncMetricStorage::Bind) with unified cardinality policy and multi-storage fanout (SyncMultiMetricStorage::Bind).
  • Adds targeted unit tests and benchmarks for bound vs unbound hot-path recording, and wires the preview behind CMake/Bazel feature gates.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test_common/cmake/preview-options.cmake Enables the preview option in test preview configurations when ABI v2 is on.
sdk/test/metrics/measurements_benchmark.cc Adds bound vs unbound fixed-attribute counter benchmarks (feature-gated).
sdk/test/metrics/CMakeLists.txt Adds a preview-gated unit test target for bound sync instruments under ABI v2.
sdk/test/metrics/bound_sync_instruments_test.cc Adds comprehensive unit tests for bound sync storage behavior (merge, overflow, GC, noop, multi-storage fanout, etc.).
sdk/src/metrics/sync_instruments.cc Implements instrument-layer bound handles for counters/histograms and binds them to bound storage handles.
sdk/src/metrics/state/sync_metric_storage.cc Adds bound-entry lifecycle, rotation, and unified cardinality handling integrated with collection.
sdk/include/opentelemetry/sdk/metrics/sync_instruments.h Declares SDK instrument Bind(...) overrides under the preview macro.
sdk/include/opentelemetry/sdk/metrics/state/sync_metric_storage.h Extends sync storage with Bind(...), BoundEntry, and unified cardinality tracking (preview-gated).
sdk/include/opentelemetry/sdk/metrics/state/multi_metric_storage.h Adds bound-handle fanout support for multi-storage sync pipelines (preview-gated).
sdk/include/opentelemetry/sdk/metrics/state/metric_storage.h Introduces BoundSyncWritableMetricStorage and adds optional SyncWritableMetricStorage::Bind(...) (preview-gated).
CMakeLists.txt Adds the CMake feature gate option for the preview and prints its status.
api/include/opentelemetry/version.h Defines OPENTELEMETRY_HAVE_METRICS_BOUND_INSTRUMENTS_PREVIEW when ABI v2 + preview define are enabled.
api/include/opentelemetry/metrics/sync_instruments.h Adds BoundCounter/BoundHistogram and Bind(...) API methods (preview-gated).
api/include/opentelemetry/metrics/noop.h Adds noop bound handle implementations and hooks them into noop instruments.
api/CMakeLists.txt Propagates ENABLE_METRICS_BOUND_INSTRUMENTS_PREVIEW to API consumers when the CMake option is enabled.
api/BUILD Adds Bazel flag/config plumbing to enable the preview define under ABI v2.

Comment thread CMakeLists.txt
Comment thread api/include/opentelemetry/metrics/sync_instruments.h Outdated
Comment thread api/include/opentelemetry/metrics/sync_instruments.h
Comment thread api/include/opentelemetry/metrics/sync_instruments.h Outdated
Comment thread sdk/src/metrics/sync_instruments.cc Outdated
@lalitb lalitb force-pushed the lalitb/bound-sync-instruments-preview branch from d64810f to 1bdd807 Compare June 2, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants