SDK Implement collection limits

[marcalff]

Implement the various limits defined in the specifications:

• events collection size limit
• attribute collection size limit
• links collection size limit
• https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#span-limits
• https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/README.md#attribute-limits

[github-actions]

This issue is available for anyone to work on. Make sure to reference this issue in your pull request.
✨ Thank you for your contribution! ✨

[XueSongTap]

Can I work on this？

[TheMenisFactor]

I don't understand what this means....

…

On Fri, Mar 21, 2025, 1:40 AM Xiaochuan Ye ***@***.***> wrote: Can I work on this？ — Reply to this email directly, view it on GitHub <#3303 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BNKC6HEZDN7XOUOADSSCRSL2VOX53AVCNFSM6AAAAABZOQNV7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONBSGQ4DCNZRGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***> [image: XueSongTap]*XueSongTap* left a comment (open-telemetry/opentelemetry-cpp#3303) <#3303 (comment)> Can I work on this？ — Reply to this email directly, view it on GitHub <#3303 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BNKC6HEZDN7XOUOADSSCRSL2VOX53AVCNFSM6AAAAABZOQNV7OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDONBSGQ4DCNZRGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[XueSongTap]

Implementation Proposal for Collection Limits in OpenTelemetry C++ SDK

Overview

To implement collection limits as specified in Issue #3303, I propose the following changes to the OpenTelemetry C++ SDK:

1. Create a new SpanLimits class to encapsulate all limit configurations
2. Extend the TracerConfig class to include span limits
3. Modify the Span implementation to track and enforce these limits

New Components

SpanLimits Class

Create a new class in sdk/include/opentelemetry/sdk/trace/span_limits.h with:

• Constants for default limit values:

  • kDefaultAttributeCountLimit = 128
  • kDefaultEventCountLimit = 128
  • kDefaultLinkCountLimit = 128
  • kDefaultAttributePerEventCountLimit = 128
  • kDefaultAttributePerLinkCountLimit = 128

• Getter/setter methods for each limit

• Factory method for default configuration

• Equality operator for comparison

Modifications to Existing Components

TracerConfig Class

Extend sdk/include/opentelemetry/sdk/trace/tracer_config.h to:

• Add a SpanLimits member variable
• Add getter/setter methods for span limits
• Update constructor to accept span limits
• Add a Create() factory method for custom configurations

Span Class

Modify sdk/src/trace/span.h and span.cc to:

• Add counters for tracking collection sizes:

  • attribute_count_
  • event_count_
  • link_count_

• Add flags to track warning status:

  • attribute_limit_warned_
  • event_limit_warned_
  • link_limit_warned_

• Add helper methods for limit checking and logging

Implementation Approach

1. In SetAttribute():

   • Check if attribute_count_ < GetSpanLimits().GetAttributeCountLimit()
   • If below limit: add attribute and increment counter
   • If at limit: discard and log warning (only once)

2. In AddEvent() methods:

   • Check event count against event limit
   • For events with attributes, check attribute count against per-event limit

3. In AddLink() methods:

   • Check link count against link limit
   • For links with attributes, check attribute count against per-link limit

Configuration

• All limits will have reasonable defaults based on the specification
• Limits will be configurable through the TracerConfig
• The implementation will be backward compatible with existing code

This approach ensures we meet the requirements in the specification while maintaining flexibility for users who need custom limit configurations.

[lalitb]

Thanks for the analysis.

The otel-cpp SDK does not copy or buffer attributes internally; instead, they are directly serialized from user memory by the exporter. As a result, it doesn't contribute to memory growth — one of the key concerns the spec addresses with recommended attribute limits.

Introducing limit checks in the SDK would require additional memory writes and branching on the hot path, which could impact performance — especially in high-throughput or non-batching concurrent/real-time processors where efficiency is critical.

As I understand the specs, while the spec encourages (SHOULD) applying limits, it doesn't make it mandatory and only requires them to be configurable if they are implemented. In the case of otel-cpp, it seems more appropriate for specific exporters to handle such enforcement if needed. Implementing limits in the SDK would impact all exporters and users, even those who do not face issues related to excessive attributes — introducing overhead where it's not needed.

[XueSongTap]

@lalitb Thank you for your review and valuable feedback. I fully understand and share your concerns regarding performance, especially considering the direct serialization architecture of the C++ SDK. Based on your suggestions, i'm planning to modify our proposal with the following key changes:

Primary Modifications

1. Limits Disabled by Default:

   • All limit checks will be disabled by default (using kNoLimit = UINT32_MAX value)
   • This ensures existing users experience no performance impact from this feature

2. Explicit Opt-in Mechanism:

   • Limit functionality will only be active when users explicitly enable it through configuration
   • We'll provide a clean API for users who need this feature to easily enable it

3. Optimized Code Path:

   • Implement a "fast path" design that requires only a single simple check in the default case (limits disabled)
   • Avoid additional branching or memory operations in the hot path

4. Flexible Configuration Options:

   • Allow users to adopt specification-recommended values or define custom limit values
   • Support selectively enabling specific limits (e.g., only limit event count without limiting attributes)

Implementation Approach

// Fast path example
void Span::SetAttribute(nostd::string_view key, const common::AttributeValue &value) noexcept {
  // Existing code...
  
  // First check if limits are enabled (default is disabled)
  const auto &limits = GetSpanLimits();
  uint32_t attribute_limit = limits.GetAttributeCountLimit();
  
  // Fast path - most users will use this path (no limits)
  if (attribute_limit == SpanLimits::kNoLimit) {
    recordable_->SetAttribute(key, value);
    return;  // Return directly, no additional operations
  }
  
  // Only users who explicitly enable limits will execute the following code
  // Limit check logic...
}

I completely agree that performance should be the primary consideration for the C++ SDK. This revised design ensures:

1. Compliance with the specification's SHOULD requirement (providing limits functionality without mandating its use)
2. Virtually no performance impact for users who choose not to use limits
3. A solution for users who need protection against excessive resource usage

This balanced approach should satisfy the needs of all users while maintaining the high-performance characteristics of the C++ SDK. Leave comment if you have any questions or concerns.

[lalitb]

@XueSongTap i would be still reluctant to add the support within the SDK, specifically adding any new member variables to span class, and increase the span object size.

[Joshua-Elsner]

@lalitb After reviewing this discussion I agree that increasing the Span object size with counters/flags is problematic, especially given that Span objects are frequently allocated and deallocated on the hot path. We definitely want to avoid penalizing the "default" (no limits) case with extra memory footprint or cache pressure.

However, since the specification recommends these limits to protect against memory exhaustion (e.g., accidental loops adding infinite attributes), we should ideally find a way to support it for users who strictly need that safety net, without touching the core Span layout. Instead of modifying the Span class, could we move the enforcement logic into the Recordable layer using a Decorator pattern?

In the OTel C++ architecture, the Span delegates SetAttribute calls to a Recordable interface. We can leverage this:

1. Keep Span Pristine: No new member variables or flags in sdk/src/trace/span.h. The Span object size remains exactly as it is today.
2. Conditional Logic in Tracer: The Tracer already holds the configuration. When StartSpan is called:

• Default (No Limits): The Tracer obtains a standard Recordable from the processor and passes it to the Span. Zero overhead added.
• Opt-In (Limits Enabled): If limits are configured, the Tracer wraps the standard Recordable in a LimitingRecordable wrapper before passing it to the Span.

Implementation Sketch

We introduce a helper class (only instantiated when limits are active):

// sdk/src/trace/limiting_recordable.h
class LimitingRecordable : public Recordable {
public:
  LimitingRecordable(std::unique_ptr<Recordable> delegate, const SpanLimits& limits)
      : delegate_(std::move(delegate)), limits_(limits), attr_count_(0) {}

  void SetAttribute(nostd::string_view key, const common::AttributeValue &value) noexcept override {
    // 1. Check limit
    if (attr_count_ >= limits_.max_attributes) {
       // Optional: Log warning once
       return; 
    }
    
    // 2. Pass through to actual storage
    delegate_->SetAttribute(key, value);
    attr_count_++;
  }
  
  // Forward other methods (AddEvent, SetStatus, etc.) similarly...

private:
  std::unique_ptr<Recordable> delegate_;
  const SpanLimits& limits_;
  uint32_t attr_count_;
  // ... other counters
};

Why this solves the conflict:

1. Zero-Cost Abstraction: Users who stick to defaults (which you noted is the preferred path) use the raw Recordable. They pay 0 bytes of extra memory per Span and execute 0 extra instructions.
2. Separation of Concerns: The Span logic doesn't get cluttered with policy enforcement. The Span just writes to an interface; it doesn't care if that interface is counting writes or not.
3. Spec Compliance: We satisfy the "SHOULD" requirement by making it possible, but we adhere to the C++ philosophy of "you don't pay for what you don't use."

This approach respects the unique architecture of the C++ SDK (direct serialization) while offering the safety mechanism solely to those willing to pay the slight vtable/indirection cost for it.

Thoughts?

[lalitb]

Thanks @Joshua-Elsner for the opt-in suggestion with the LimitingRecordable decorator. I appreciate the effort to find a middle ground that keeps the Span class untouched.

Looking at the specs again - the language is "SDK SHOULD apply truncation", not MUST. The MUST requirements only kick in if an SDK chooses to implement limits. So there's room for SDKs to make architecture-appropriate decisions here.

In otel-cpp, the Span class doesn't buffer or copy attributes. It's just a thin wrapper that passes everything straight through to the Recordable, which is owned and created by the exporter. The exporter decides how to store the data - OTLP serializes directly to protobuf, others may do something different. The SDK itself doesn't contribute to memory growth from attributes.

On the decorator approach - I get the appeal, but it still means maintaining a wrapper class with counters, allocation logic for when limits are enabled, and the configuration wiring to toggle it on. That's ongoing maintenance for something that solves a problem at the wrong layer. The memory lives in the exporter's recordable, not the SDK, so we'd be adding complexity to guard against growth we don't cause. And even when "disabled", the code paths and configuration options still need to be tested and maintained.

The cleaner path is letting exporters handle this where the memory actually lives. If OTLP exporter users want limits, we can add enforcement directly in OtlpRecordable - that's straightforward and self-contained. Different exporters can then make their own choices based on their backend requirements. And since the exporter runs on separate thread in case of batch processor, these opt-in conditional check won't be executed in the application hot-path.

Happy to review a PR that adds limit support to the OTLP exporter.

[Joshua-Elsner]

@lalitb Thanks for the clear explanation. I completely agree with the "pipe vs. bucket" distinction here. It makes much more sense to enforce these limits in the Exporter where the data is actually stored, especially given the threading benefits you mentioned for batch processing.

I will pivot to that approach. My plan for the PR is:

Modify oltp_recordable.cc. Add the limit counters and enforcement checks (drop & log/count) in SetAttribute, AddEvent, and AddLink.

Update Options: Add the limit configuration fields to oltp_http_exporter_options.cc and oltp_grpc_exporter_options.cc.

Wire it up: Pass these options through MakeRecordable in the respective exporters.

I’ll get started on this and open a PR soon. Thanks for the guidance