[SDK] LogRecord attribute limits enforcement

[thc1006]

Fixes #4126.

Implements the LogRecord attribute count and value length limits described
by the logs SDK spec
(https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecord-limits).
Limits flow from LoggerProvider through LoggerContext to each LogRecord
created by Logger::CreateLogRecord. The infrastructure existed in the
declarative configuration tier (LogRecordLimitsConfiguration) but was
never wired into the runtime pipeline.

What changed

New header sdk/include/opentelemetry/sdk/logs/log_record_limits.h:

• LogRecordLimits struct with spec defaults
  attribute_count_limit = 128 and
  attribute_value_length_limit = SIZE_MAX (unlimited).

SDK Recordable hierarchy:

• Recordable gains a non-pure virtual SetLogRecordLimits with a no-op
  default body. Existing implementations that do not enforce limits
  inherit the no-op and compile unchanged. The virtual is appended at
  the end of the vtable to keep the change additive.
• ReadableLogRecord gains a non-pure virtual GetDroppedAttributesCount
  returning zero by default.
• ReadWriteLogRecord overrides both. SetAttribute checks the count
  limit before inserting and truncates string / array-of-string values
  whose length exceeds the configured limit. Truncation is byte level,
  mirroring the existing Span attribute behavior.
• MultiRecordable propagates SetLogRecordLimits to every wrapped
  recordable.

OTLP exporter:

• OtlpLogRecordable overrides SetLogRecordLimits. SetAttribute
  drops attributes beyond the count limit and increments the proto
  LogRecord.dropped_attributes_count field. Strings and string-array
  values are truncated after OtlpPopulateAttributeUtils::PopulateAttribute
  populates the proto AnyValue.

Wiring:

• LoggerContext owns a LogRecordLimits value and exposes
  GetLogRecordLimits(). A new fourth parameter is appended to the
  existing constructor with a default-constructed default, so existing
  call sites compile unchanged.
• Logger::CreateLogRecord (both ABI v1 and ABI v2 variants) calls
  recordable->SetLogRecordLimits(context_->GetLogRecordLimits())
  immediately after MakeRecordable, before any user attribute write.
• A new LoggerProviderFactory::Create overload accepts
  LogRecordLimits and builds a LoggerContext with those limits
  internally.

Declarative configuration:

• SdkBuilder::CreateLoggerProvider now maps
  LogRecordLimitsConfiguration from the parsed
  LoggerProviderConfiguration to the runtime LogRecordLimits and
  passes them to the new factory overload. The existing FIXME-SDK
  comment about wiring limits is removed.

Tests

sdk/test/logs/log_record_limits_test.cc (new):

• Default behavior: 200 attributes accepted, zero dropped when no
  limits object is supplied.
• Count enforcement: attributes beyond the limit are dropped and
  counted. Replacing an existing key while at the limit must not drop.
• Length truncation for strings and string arrays.
• Type selectivity: only string and array-of-string are truncated;
  int / double / bool pass through.
• Combined count + length case.
• Logger-level wiring test: a TrackingRecordable produced by a
  TrackingProcessor confirms that the limits configured on
  LoggerProvider reach the recordable returned by
  Logger::CreateLogRecord.

exporters/otlp/test/otlp_log_recordable_test.cc augmented with four
cases that verify the proto dropped_attributes_count field and the
string / array truncation logic.

Verification

Built and tested in the otelcpp-dev container:

• Release (gcc, abiv1): log_record_limits_test 9/9,
  otlp_log_recordable_test 23/23, logger_sdk_test 9/9,
  logger_provider_sdk_test 9/9, simple_log_record_processor_test
  10/10, batch_log_record_processor_test 13/13. No regression.
• ABI v2 (clang): log_record_limits_test 9/9.
• -fno-rtti (Bazel nortti CI mirror): log_record_limits_test 9/9,
  otlp_log_recordable_test 23/23.
• clang-format fixed point verified for all touched files.
• IWYU (abiv1-preview): include-list reconciled with diagnostics
  from a local clang-22 run.
• git diff --check whitespace: clean.

Known limitations

• ElasticSearchRecordable is not modified in this PR. It inherits
  the no-op default and does not enforce limits. A follow-up PR can
  add enforcement if desired.
• Truncation is byte level. Strings whose configured limit falls inside
  a multibyte UTF-8 sequence will be truncated mid-sequence, matching
  the existing Span attribute behavior in OtlpRecordable.
• The default branch in the YAML parser
  (ParseLogRecordLimitsConfiguration) keeps the existing
  attribute_value_length_limit = 4096 fallback for callers that
  opt into the limits: block without specifying a length. Aligning
  that fallback with the spec default (unlimited) is left to a follow-up.

[codecov]

Codecov Report

❌ Patch coverage is 93.28358% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.93%. Comparing base (82a2ada) to head (980ef8e).

Files with missing lines	Patch %	Lines
...xporters/otlp/src/otlp_populate_attribute_utils.cc	80.77%	5 Missing ⚠️
...include/opentelemetry/sdk/common/attribute_utils.h	97.50%	1 Missing ⚠️
sdk/include/opentelemetry/sdk/logs/exporter.h	0.00%	1 Missing ⚠️
...clude/opentelemetry/sdk/logs/readable_log_record.h	0.00%	1 Missing ⚠️
sdk/include/opentelemetry/sdk/logs/recordable.h	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4157      +/-   ##
==========================================
- Coverage   82.98%   82.93%   -0.05%     
==========================================
  Files         406      416      +10     
  Lines       17264    17400     +136     
==========================================
+ Hits        14325    14429     +104     
- Misses       2939     2971      +32     

Files with missing lines	Coverage Δ
...ntelemetry/exporters/ostream/log_record_exporter.h	100.00% <100.00%> (ø)
...try/exporters/otlp/otlp_file_log_record_exporter.h	100.00% <100.00%> (ø)
...try/exporters/otlp/otlp_grpc_log_record_exporter.h	100.00% <100.00%> (ø)
...try/exporters/otlp/otlp_http_log_record_exporter.h	100.00% <100.00%> (ø)
...opentelemetry/exporters/otlp/otlp_log_recordable.h	100.00% <ø> (ø)
...try/exporters/otlp/otlp_populate_attribute_utils.h	100.00% <100.00%> (ø)
exporters/otlp/src/otlp_log_recordable.cc	38.16% <100.00%> (+2.11%)	⬆️
...pentelemetry/sdk/logs/batch_log_record_processor.h	100.00% <100.00%> (ø)
...include/opentelemetry/sdk/logs/log_record_limits.h	100.00% <100.00%> (ø)
sdk/include/opentelemetry/sdk/logs/processor.h	100.00% <100.00%> (ø)
... and 12 more

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[proost]

@thc1006
I'm working on the feature in here(#4132).
Do you want to implement this feature?

[thc1006]

@proost Hey, thanks for the ping. I went through #4132 and the review thread before pushing #4157. There's a fair bit of substantive work in yours that mine doesn't have: the OTEL_LOGRECORD_* env var integration, the benchmark file, ElasticSearchRecordable enforcement, and the yaml count_limit plumbing.

The reason I went ahead with #4157 even after seeing yours is lalitb's comment from yesterday (#4132 (comment)), which was asking to move enforcement out of the base Recordable hot path and let each recordable handle it the way that fits. That's the shape #4157 takes. Recordable just gains a SetLogRecordLimits virtual with a no-op default body, and the actual enforcement lives in ReadWriteLogRecord and OtlpLogRecordable. Non-enforcing exporters inherit the no-op so they pay nothing.

If the maintainers think #4157 is the better base, I'd like to layer the env vars, benchmark, and ES enforcement on top in follow-ups with attribution to your work. If they prefer the #4132 redesign route, happy to close #4157 and review the new version there.

@marcalff @lalitb would appreciate your call on which to drive forward.

[proost]

@thc1006

Thanks for the explanation.

I'm always open to feedback, and if there were concerns about the direction of #4132, I would have been happy to discuss and adjust it. I do wish those concerns had been raised directly on the PR earlier, as it would have helped avoid duplicated work.

That said, I don't have a strong preference on whose implementation lands. If you would like to continue driving this work, I'm happy to step aside and close #4132 so we don't spend effort maintaining two competing PRs.

So would you like to continue driving this work?

[thc1006]

@proost You're right and that one's on me. The honest reason I missed #4132: I was searching open issues by help wanted label and recent merged PRs for landscape, not by fixes #4126 against open PRs. #4132 has no labels and the title in the open-PR list truncates around "attribute count l...", so I read it as a narrower length-only fix and moved on. That was a diligence gap on my part, not a deliberate decision to bypass your work. Sorry about that.

If you're comfortable stepping aside, yes I'd like to continue with #4157.

After #4157 lands I'd open these as follow-ups:

1. OTEL_LOGRECORD_ATTRIBUTE_VALUE_LENGTH_LIMIT and OTEL_LOGRECORD_ATTRIBUTE_COUNT_LIMIT env-var integration through LogRecordLimits and the SdkBuilder path, modeled on your feat: log record attribute value length limit & attribute count limit #4132 work and adding you as a Co-authored-by on the commit.
2. The benchmark file from feat: log record attribute value length limit & attribute count limit #4132, ported onto the per-recordable enforcement so the numbers reflect the new architecture, attribution in the commit body.
3. ElasticSearchRecordable enforcement following the same SetLogRecordLimits pattern.
4. YAML attribute_count_limit parsing along the lines feat: log record attribute value length limit & attribute count limit #4132 did.

If you'd rather author any of those follow-ups yourself and have me review, that works equally well. Whatever keeps the work landing.

[proost]

Thanks for the clarification and the apology.

I’m happy for you to continue driving this work, so I’ll close #4132 to avoid duplicate effort.

[thc1006]

@proost Thanks for the clean handoff. The reviewer feedback from owent, lalitb, and ThomsonTan on #4132 still applies here, and I'll reference the specific discussions when each follow-up goes up. I'll ping you on each so you can shape the attribution however works for you.

[lalitb]

I think we should avoid storing a pointer to the limits object here. In the normal logger path, CreateLogRecord() sets this from LoggerContext, but the returned LogRecord is an independent unique_ptr. User code can keep that record and call SetAttribute() after the logger/provider/context is gone, which would leave limits_ dangling.

Can we instead copy LogRecordLimits into the recordable instead of storing &limits? The struct is tiny, and it avoids adding a lifetime requirement to LogRecord.

[thc1006]

Agreed. Switched to by-value storage in ba8f9047. The default-constructed value carries the spec defaults (count=128, length=unlimited), so a fresh recordable now enforces the spec count cap from construction — happy to revisit that semantic if you'd prefer a no-op-when-unset sentinel instead.

[dbarker]

Thanks for the PR. Please see comments below.

[thc1006]

@dbarker the only failing CI on 8c965e7e is DocFX check and it's a transient infrastructure failure unrelated to this PR — the GitHub Releases download for docfx.zip returned HTTP 500:

Attempt to get headers for https://github.com/dotnet/docfx/releases/download/v2.58.5/docfx.zip failed.
The remote server returned an error: (500) Internal Server Error.
Chocolatey installed 0/1 packages. 1 packages failed.
docfx (exited 404)

The job failed in 40 seconds during the docfx download step before any of the PR's content was even read. Would you mind re-running just this job (I don't have admin rights to do it myself)? All other 31 jobs on this head are SUCCESS / 2 SKIPPED, no other failures.

[dbarker]

Thanks for the cleanup. Please see comments below. The main question is: Can the truncation be applied on creation of the owned type instead of after in order to prevent allocating the overflowing value?

[lalitb]

Can we avoid adding this unconditional virtual call on every CreateLogRecord()? This is now paid by all SDK log paths, including custom recordables that do not enforce limits.

One cleaner option would be passing LogRecordLimits through MakeRecordable(...), but that looks API-breaking for existing exporters. A safer incremental option may be to compute/store a config flag once and only call SetLogRecordLimits() for exporters/recordables that actually enforce limits, like OTLP and debug/ostream.

That would keep the extra limit work out of unrelated SDK hot paths.

[thc1006]

Thanks for catching this. v7 moved truncation into the recordable (stateful AttributeConverter), so the SetLogRecordLimits setter is what carries that state, and you are right that paying for the call on the common path is not justified when the recordable ignores it.

The "compute/store a config flag once" path lines up well: a RecordableEnforcesLogRecordLimits() virtual on LogRecordProcessor (and LogRecordExporter, defaulting false), cached by Logger at construction, and used to gate the existing call:

if (recordable_enforces_limits_) {
  recordable->SetLogRecordLimits(context_->GetLogRecordLimits());
}

OTLP and ostream exporters override to return true. MultiLogRecordProcessor returns true if any child does. Custom processors keep the default and pay only a bool load plus a predicted branch. The new virtuals append to the existing vtable, so subclass layout is unchanged.

If that lines up with what you had in mind, I will push v8 with this gate.

[lalitb]

Yes, that direction makes sense to me. One small refinement: since LogRecordLimits and the processor/exporter chain are provider/context-level configuration, can we cache this capability on LoggerContext rather than on each Logger?

That would still avoid the unconditional SetLogRecordLimits() virtual call in CreateLogRecord(), but it also avoids adding per-logger state for a provider-level setting. The hot path would just read the context-level flag and call SetLogRecordLimits() only when some processor/exporter actually enforces limits.

For example:

  LoggerContext {
    LogRecordLimits limits_;
    bool recordable_enforces_limits_;
  }

Then each logger can gate the setup with:

  if (context_->RecordableEnforcesLogRecordLimits())
  {
    recordable->SetLogRecordLimits(context_->GetLogRecordLimits());
  }

[thc1006]

Agreed, that's a cleaner placement. LoggerContext is where the processor chain and LogRecordLimits already live, so the capability flag belongs there. I'll add the cached recordable_enforces_limits_ member and the RecordableEnforcesLogRecordLimits() getter on LoggerContext, populated at construction from the processor chain. Both gating call sites in Logger::CreateLogRecord() will route through the context getter as you showed.

[owent]

I suggest keeping UTF-8-aware truncation here. The reasoning:

1. For UTF-8 strings: If a user stores a UTF-8 string in std::string, they would expect the truncated result to remain valid UTF-8. A naive byte-length truncation risks splitting a multi-byte code point, which could cause the OTLP exporter to fall back to bytes_value instead of the intended string_value.
2. For raw bytes: If a user stores raw binary data in std::string, they would expect simple byte-length truncation without any encoding semantics.

[thc1006]

Thanks, agreed. The v7 docstring defends byte cut on the grounds that std::string may carry raw bytes from a non-UTF-8 source, but the OTel convention that std::string means UTF-8 (with nostd::span<const uint8_t> for raw bytes) is the stronger signal here.

The v8 plan:

1. Move Utf8SafePrefixLength into sdk/include/opentelemetry/sdk/common/attribute_utils.h alongside AttributeConverter (inline, no new file; the OTLP-side wrapper forwards into it).
2. Route the AttributeConverter std::string / nostd::string_view / const char* overloads through it, plus the per-element cut in the nostd::span<const nostd::string_view> array overload; nostd::span<const uint8_t> keeps byte cut as you describe.
3. The existing 8 OTLP-side Utf8SafePrefixLength tests stay; sdk/test/common/attribute_utils_test.cc gets equivalent SDK-side coverage.
4. Docstring updates to match.

This composes with the gating ask in r3468037047; both land in the same v8 push as separate commits.

[ThomsonTan]

The introduced limit changes the current default behavior, a default-constructed ReadWriteLogRecord / OtlpLogRecordable now enforces the 128 attribute cap even when no limits were configured and the logger wiring path was never used. Suggest iither make "no explicit SetLogRecordLimits = unlimited" the recordable default and let the wiring layer inject 128, or keep the spec default but call this out in CHANGELOG as a breaking change.

[thc1006]

BTW the v+num is Number of fixed commit (revisions)

[thc1006]

Good catch, you're right. A default-constructed ReadWriteLogRecord / OtlpLogRecordable starts with limits_{} = {128, unlimited}, so it caps at 128 even when no provider wired it up. That goes against the intent that these limits are provider-level.

I'll go with option 1: the recordable defaults to no limits, and the wiring injects the configured ones. This lines up with the RecordableEnforcesLogRecordLimits gating in the latest revision: LoggerContext keeps the spec default ({128, unlimited}) and the Logger injects it through SetLogRecordLimits only when a processor enforces. The result:

• a bare recordable, or one behind a non-enforcing processor: no limits, matching the pre-PR behavior.
• a recordable created through the standard provider path: the spec default of 128 (or whatever the provider was configured with) applies.

Concretely I'll default limits_ to unbounded in both recordables, keep the LoggerContext default at the spec values, and update the DefaultRecordEnforcesSpecCountCap test to match. The CHANGELOG will describe the provider-path enforcement as the new behavior.

Happy to do option 2 instead (keep the spec default on the recordable and just document it) if you'd rather, but option 1 looks closer to the provider-level intent.

[dbarker]

Thanks for the revision. Please see feedback below. Given the changes please review the code comments and update if needed.

[dbarker]

Please also test truncation for the const char* and span paths.

[thc1006]

Added a const char* truncation test (LengthLimitTruncatesConstCharPtr). The span paths are already covered: LengthLimitTruncatesEachStringInArray for the string array and LengthLimitTruncatesBytesAttribute for the bytes span.

[dbarker]

please update the comment now that recordable should copy the limits

[thc1006]

Updated: the comment now says implementations copy the supplied limits, so the caller does not need to keep the object alive.

[dbarker]

please update the comment to account for limits being copied to recordables.

[thc1006]

Updated to say the limits are copied into the recordable.

[dbarker]

this second paragraph in the comment can be removed as an implementation detail.

[thc1006]

Removed. I also removed the same paragraph from the LogRecordProcessor version in processor.h for consistency.

[dbarker]

are there worthwhile short cuts that can be taken if the max_length limit is not set (or is the default max size_t) ?

[thc1006]

Good point. Added an early return in Utf8SafePrefixLength: when max_bytes >= size (which includes the default unbounded SIZE_MAX), the whole value fits so there is nothing to truncate and the per-byte scan is skipped. It lives in the shared helper, so both this OTLP path and the in-memory AttributeConverter benefit.