[SDK] LogRecord attribute limits enforcement#4157
Conversation
thc1006
force-pushed
the
feat/log-record-limits-4126
branch
from
7bdc3c5 to
1769757
Compare
Apply attribute count and value length limits described by the logs SDK spec (https://opentelemetry.io/docs/specs/otel/logs/sdk/#logrecord-limits) during attribute writes. Limits flow from LoggerProvider through LoggerContext to each LogRecord created by Logger::CreateLogRecord. * Add LogRecordLimits struct with spec defaults: attribute_count_limit = 128 and attribute_value_length_limit = SIZE_MAX (unlimited). * Recordable gains a virtual SetLogRecordLimits with a no-op default so existing implementations need not change. ReadableLogRecord gains a virtual GetDroppedAttributesCount returning zero by default. * ReadWriteLogRecord and OtlpLogRecordable enforce the limits in SetAttribute. An attribute beyond attribute_count_limit is dropped and counted as dropped; string and string-array values whose byte length exceeds attribute_value_length_limit are truncated. Truncation is byte-level, mirroring the existing Span attribute behavior. The OTLP path also populates dropped_attributes_count on the proto LogRecord. * MultiRecordable propagates the limits to every wrapped recordable. * LoggerContext owns a LogRecordLimits value; Logger calls SetLogRecordLimits on the recordable returned by MakeRecordable before any user attribute writes. A new LoggerProviderFactory::Create overload accepts LogRecordLimits. * The declarative configuration path (SdkBuilder) wires LogRecordLimitsConfiguration to the runtime LogRecordLimits. Tests cover ReadWriteLogRecord and OtlpLogRecordable: defaults, count enforcement (including the "replace existing key while at limit must not drop" case), length truncation of strings and string arrays, type selectivity (only string and array-of-string are truncated), the combined count plus length case, and a Logger-level wiring test that verifies the limits configured on LoggerProvider reach the recordable returned by Logger::CreateLogRecord. Fixes open-telemetry#4126 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
thc1006
force-pushed
the
feat/log-record-limits-4126
branch
from
1769757 to
e244122
Compare
|
@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 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. |
|
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? |
|
@proost You're right and that one's on me. The honest reason I missed #4132: I was searching open issues by If you're comfortable stepping aside, yes I'd like to continue with #4157. After #4157 lands I'd open these as follow-ups:
If you'd rather author any of those follow-ups yourself and have me review, that works equally well. Whatever keeps the work landing. |
|
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. |
| const opentelemetry::sdk::resource::Resource *resource_ = nullptr; | ||
| const opentelemetry::sdk::instrumentationscope::InstrumentationScope *instrumentation_scope_ = | ||
| nullptr; | ||
| const opentelemetry::sdk::logs::LogRecordLimits *limits_ = nullptr; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
…runcate) Address three review comments from @lalitb on PR open-telemetry#4157: 1. Store LogRecordLimits by value inside ReadWriteLogRecord and OtlpLogRecordable instead of a raw pointer to a LoggerContext-owned object. A LogRecord is handed out as a unique_ptr, so a record outliving the context that produced it would otherwise dereference a dangling pointer when the user calls SetAttribute() later. The default-constructed value already carries the spec defaults (count=128, length=unlimited), so a fresh recordable enforces the spec count cap from construction, matching the PR's "enforcement" contract. 2. Drop the now-redundant `limits_ != nullptr` short-circuit at every enforcement site (4 in total). This also closes the Codecov-reported uncovered branch in otlp_log_recordable.cc. 3. Truncate UTF-8 string attributes at a code-point boundary instead of a raw byte boundary, so an OTLP protobuf string_value produced by truncation stays valid UTF-8 when the input was. Malformed UTF-8 and trailing lead bytes degrade to plain byte truncation. Logic adapted from open-telemetry#4132 with attribution. While in the same truncation paths, also apply the byte-length cap to raw bytes attributes (`vector<uint8_t>` on the SDK side, AnyValue `bytes_value` on the OTLP side). Both were previously passing through any size, even though the spec applies `attribute_value_length_limit` to bytes attributes as well. Test changes: - Rename DefaultsPassThroughWithoutLimitsObject to DefaultRecordEnforcesSpecCountCap (200 attrs in, 128 stored, 72 dropped) to reflect the new spec-correct default behavior. - Add 3 UTF-8 regression tests on the SDK side (split prevention, exact fit at sequence boundary, malformed input falls back) plus a bytes-truncation test. - Add 3 mirror tests on the OTLP side, a default-cap test, and a bytes-truncation test. Refs: open-telemetry#4126 Co-authored-by: Hyeonho Kim <proost@apache.org> Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
…are truncate) Address three review comments from @lalitb on PR open-telemetry#4157, plus a follow-up from @owent (r3434178971). 1. Store LogRecordLimits by value inside ReadWriteLogRecord and OtlpLogRecordable instead of a raw pointer to a LoggerContext-owned object. A LogRecord is handed out as a unique_ptr, so a record outliving the context that produced it would otherwise dereference a dangling pointer when the user calls SetAttribute() later. The default-constructed value already carries the spec defaults (count=128, length=unlimited), so a fresh recordable enforces the spec count cap from construction, matching the PR's "enforcement" contract. 2. Drop the now-redundant `limits_ != nullptr` short-circuit at every enforcement site (4 in total). This also closes the Codecov-reported uncovered branch in otlp_log_recordable.cc. 3. Truncate OTLP string attributes at a UTF-8 code-point boundary instead of a raw byte boundary, so the protobuf string_value produced by truncation stays valid UTF-8 when the input was. Malformed UTF-8 and trailing lead bytes degrade to plain byte truncation. Logic adapted from open-telemetry#4132 with attribution. The SDK-side ReadWriteLogRecord truncation stays as plain byte cut. The in-memory `OwnedAttributeValue::std::string` variant may legitimately carry raw bytes when constructed from a non-UTF-8 source, so forcing UTF-8 boundary semantics there would over-truncate that case (per @owent's r3434178971, echoing the same point on open-telemetry#4132 r3409677314). Each recordable's truncation strategy now matches its own consumer's wire-format requirement: SDK in-memory has no wire requirement, OTLP protobuf requires valid UTF-8. While in the same truncation paths, also apply the byte-length cap to raw bytes attributes (`vector<uint8_t>` on the SDK side, AnyValue `bytes_value` on the OTLP side). Both were previously passing through any size, even though the spec applies `attribute_value_length_limit` to bytes attributes as well. Test changes: - Rename DefaultsPassThroughWithoutLimitsObject to DefaultRecordEnforcesSpecCountCap (200 attrs in, 128 stored, 72 dropped) to reflect the new spec-correct default behavior. - Add bytes-truncation tests on both SDK and OTLP sides. - Add 3 UTF-8 regression tests on the OTLP side only (split prevention, exact fit at sequence boundary; malformed-fallback omitted since the algorithm's seq=1 fallback for invalid continuations is implementation detail rather than wire contract). - Add a default-cap test on the OTLP side. Refs: open-telemetry#4126 Co-authored-by: Hyeonho Kim <proost@apache.org> Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
thc1006
force-pushed
the
feat/log-record-limits-4126
branch
from
ba8f904 to
1612edc
Compare
dbarker
left a comment
dbarker
left a comment
There was a problem hiding this comment.
Thanks for the PR. Please see comments below.
…dupe SetAttribute lookup) Address @dbarker's three review comments on PR open-telemetry#4157. * r3442854800: Extract Utf8SafePrefixLength and TruncateProtoAttributeValue from the anonymous namespace in otlp_log_recordable.cc into OtlpPopulateAttributeUtils as static methods, with new direct unit tests in otlp_populate_attribute_utils_test.cc. The upcoming SpanLimits PR will reuse these from the OTLP trace recordable. The OTLP helper that was previously TruncateProtoStringValue is renamed TruncateProtoAttributeValue to reflect that it covers string_value, bytes_value, and array_value branches. * r3443005793: Extract the SDK byte-length truncation helper into sdk::common::TruncateAttributeValueByteLength (inline, declared in the existing sdk/common/attribute_utils.h), with new direct unit tests in attribute_utils_test.cc. The upcoming SpanLimits PR will reuse this from ReadWriteSpanData. The new name reflects that the helper covers string, string-array, AND bytes variants rather than only strings. * r3443034336: Rewrite ReadWriteLogRecord::SetAttribute to use a single unordered_map lookup. The previous code did .find() to gate the count cap, then operator[] to fetch-or-insert; the new code does .find() followed by conditional .emplace(), so existing-key replacement and new-key insertion each cost one hash lookup. Refs: open-telemetry#4126 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
Two small fixes on top of c4532cd: * Address @dbarker's r3444034234: rewrite sdk::common::TruncateAttributeValueByteLength to dispatch on the variant via nostd::get_if (returns nullptr if the alternative does not hold) instead of nostd::holds_alternative + nostd::get. The helper is declared noexcept; nostd::get throws when the alternative does not match, which would invoke std::terminate even though the preceding holds_alternative check makes that path unreachable in practice. The get_if rewrite removes the throwing call entirely so the noexcept contract is statically honored. * Restore the Bazel link of the new otlp_populate_attribute_utils_test target by adding //sdk/src/metrics to its deps, matching the existing otlp_log_recordable_test target. The new test target links against :otlp_recordable, which transitively references sdk::metrics::AdaptingCircularBufferCounter symbols; Bazel's strict layering requires the dep to be declared at the cc_test level. CMake did not catch this because its default link aggregates the whole library. Refs: open-telemetry#4126 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
… + clang-tidy NOLINT Three CI failure classes surfaced after the branch-update merge of main into the PR branch (ccdd5e9). Address each: * MSVC C2220 (warning-as-error) on attribute_utils.h:93 — the `for (auto &s : *vec)` loop inside TruncateAttributeValueByteLength shadowed the `if (auto *s = get_if<std::string>(...))` on line 84. GCC/Clang accept if-init scoping for the two `s`, MSVC /W4 /WX treats C4456 (declaration hides previous local) as an error. Rename the loop variable to `element` to remove the shadow. * IWYU drop four includes that the v3+v4 utility-extraction refactor made redundant: - sdk/src/logs/read_write_log_record.cc: <vector> - exporters/otlp/src/otlp_log_recordable.cc: <string> - exporters/otlp/test/otlp_populate_attribute_utils_test.cc: <cstddef> and <limits> * clang-tidy abiv2-preview misc-no-recursion on OtlpPopulateAttributeUtils::TruncateProtoAttributeValue — the recursive descent into AnyValue::kArrayValue children is intentional and bounded by the SDK-side AttributeValue variant depth. Suppress with a NOLINTBEGIN/END(misc-no-recursion) block, matching the codebase precedent in sdk/src/configuration/configuration_parser.cc. The remaining clang-tidy bugprone-exception-escape warnings in the same config (read_write_log_record.cc:68 SetBody, :158 SetAttribute and otlp_populate_attribute_utils.cc:62/220 PopulateAnyValue) trace through nostd::visit / nostd::get into bad_alloc paths that pre-date this PR; they are out of scope for the LogRecord limits change. Refs: open-telemetry#4126 Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
|
@dbarker the only failing CI on 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
left a comment
dbarker
left a comment
There was a problem hiding this comment.
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?
|
The introduced limit changes the current default behavior, a default-constructed |
CreateLogRecord() called SetLogRecordLimits() on every record unconditionally, paying a virtual dispatch even for recordables that do not enforce limits. Add a RecordableEnforcesLogRecordLimits() capability query on LogRecordExporter and LogRecordProcessor (default false; OTLP and ostream exporters return true; Simple and Batch processors delegate to their exporter; Multi returns true if any child does). LoggerContext caches the result at construction and refreshes it in AddProcessor, so the Logger hot path reads a plain bool and only calls SetLogRecordLimits() when a processor actually enforces limits. The new virtuals are appended at the end of their vtables to keep the change additive. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
The in-memory AttributeConverter cut string values at a raw byte boundary, which can split a multi-byte UTF-8 sequence and leave an invalid std::string. Move Utf8SafePrefixLength from the OTLP recordable into sdk::common so the converter can share it, and truncate the std::string, string_view, const char*, and string-array alternatives at a UTF-8 code-point boundary. Raw bytes (span<const uint8_t>) keep the exact byte cut since they carry no encoding. The OTLP helper now forwards to the shared implementation, leaving its public surface and tests unchanged. Also fold in review nits: delegate the const char* overload to the string_view overload to avoid copying an oversized C string before truncating, drop a redundant pointer cast in the bytes overload, and use std::strlen. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
|
BTW the |
A recent merge of main lowered the abiv1-preview and abiv2-preview clang-tidy ceilings to 331 and 341. Two warnings need addressing to stay within them: - Initialize LoggerContext::recordable_enforces_limits_ in the member initializer list instead of the constructor body (cppcoreguidelines-prefer-member-initializer). - Move the rvalue reference parameter in the test TrackingProcessor OnEmit stub (cppcoreguidelines-rvalue-reference-param-not-moved). Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
|
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:
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. |
…rovider A default-constructed ReadWriteLogRecord / OtlpLogRecordable previously carried the spec-default LogRecordLimits (count 128), so a recordable used without any LoggerProvider wiring enforced the 128 cap. That changed behavior for code that constructs a recordable directly. Default the recordable to no limits and let the LoggerProvider wiring inject the configured limits through SetLogRecordLimits, which it already does only when a processor enforces them. LoggerContext keeps the spec default, so a record created through the standard provider path still gets the 128 cap (or the configured value), while a bare recordable no longer caps on its own. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
dbarker
left a comment
dbarker
left a comment
There was a problem hiding this comment.
Thanks for the revision. Please see feedback below. Given the changes please review the code comments and update if needed.
…shortcut, test) - Correct the SetLogRecordLimits comments on the base Recordable, ReadWriteLogRecord, and OtlpLogRecordable: the limits are copied, so the caller does not need to keep the supplied object alive (the previous "must outlive" wording was stale). - Drop the vtable-append implementation-detail paragraph from the RecordableEnforcesLogRecordLimits comments on LogRecordExporter and LogRecordProcessor. - Short-circuit Utf8SafePrefixLength when the whole value fits within the budget (max_bytes >= size, including the unbounded default), so the common path skips the per-byte scan. - Add a const char* attribute value truncation test. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
dbarker
left a comment
dbarker
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks. Approved, but we need to hold on merging until benchmarks are added and compared with main.
Please run the otlp_recordable_benchmark to verify changes to the attribute utils did not introduce a regression for the otlp span recording path. There is no benchmark for log recordables now. At least the oltp log recordable benchmark needs to be added in a separate PR.
Remove the second paragraph of the Recordable::SetLogRecordLimits doc comment (the vtable-append note) per review. It is an implementation detail and matches the paragraphs already dropped from the exporter and processor headers. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
|
Heads up: the red |
owent
left a comment
owent
left a comment
There was a problem hiding this comment.
LGTM on my end. Given the scope, let's get one more reviewer to double-check before merging.
lalitb
left a comment
lalitb
left a comment
There was a problem hiding this comment.
LGTM from my side. I agree with @dbarker we should at least compare the existing otlp_recordable_benchmark against main for this PR, since the shared attribute utils changed. A dedicated otlp_log_recordable_benchmark can be a follow-up PR.
|
Ran Built it from both
Every interval spans zero, so no measurable regression on the span recording path. I'll add the dedicated (Local Release build in the otelcpp-dev image.) |
There was a problem hiding this comment.
Pull request overview
Implements OpenTelemetry Logs SDK LogRecord attribute limits (count + value length) end-to-end: introduces a runtime LogRecordLimits type, wires limits from LoggerProvider/LoggerContext into each newly created recordable, and enforces limits in both in-memory (ReadWriteLogRecord) and OTLP (OtlpLogRecordable) recordables. Also adds UTF-8-safe truncation utilities in the common attribute conversion layer and expands test coverage across SDK and OTLP exporter paths.
Changes:
- Add
LogRecordLimits(spec defaults) and wire it throughLoggerProviderFactory→LoggerContext→Logger::CreateLogRecord()→Recordable::SetLogRecordLimits(). - Enforce attribute count dropping and value-length truncation in
ReadWriteLogRecordandOtlpLogRecordable, including protodropped_attributes_count. - Add/extend unit tests and register new tests in both CMake and Bazel builds.
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/test/logs/log_record_limits_test.cc | New SDK tests for count/length enforcement and logger wiring. |
| sdk/test/logs/CMakeLists.txt | Registers new log_record_limits_test. |
| sdk/test/logs/BUILD | Adds Bazel target for log_record_limits_test. |
| sdk/test/common/attribute_utils_test.cc | Adds tests for UTF-8-safe truncation behavior in AttributeConverter. |
| sdk/src/logs/read_write_log_record.cc | Enforces count/length limits in in-memory log record attributes. |
| sdk/src/logs/multi_recordable.cc | Propagates SetLogRecordLimits to wrapped recordables. |
| sdk/src/logs/multi_log_record_processor.cc | Aggregates “enforces limits” capability across processors. |
| sdk/src/logs/logger.cc | Pushes limits onto recordables on creation when needed. |
| sdk/src/logs/logger_provider_factory.cc | Adds Create(..., LogRecordLimits) overload. |
| sdk/src/logs/logger_context.cc | Stores limits and caches whether recordables enforce them. |
| sdk/src/configuration/sdk_builder.cc | Maps declarative LogRecordLimitsConfiguration to runtime limits. |
| sdk/include/opentelemetry/sdk/logs/simple_log_record_processor.h | Delegates “enforces limits” capability to exporter. |
| sdk/include/opentelemetry/sdk/logs/recordable.h | Adds no-op default SetLogRecordLimits hook. |
| sdk/include/opentelemetry/sdk/logs/readable_log_record.h | Adds default GetDroppedAttributesCount() API. |
| sdk/include/opentelemetry/sdk/logs/read_write_log_record.h | Declares limits/drop-count support for ReadWriteLogRecord. |
| sdk/include/opentelemetry/sdk/logs/processor.h | Adds RecordableEnforcesLogRecordLimits() capability flag. |
| sdk/include/opentelemetry/sdk/logs/multi_recordable.h | Declares SetLogRecordLimits propagation API. |
| sdk/include/opentelemetry/sdk/logs/multi_log_record_processor.h | Declares aggregated capability method. |
| sdk/include/opentelemetry/sdk/logs/logger_provider_factory.h | Declares factory overload taking LogRecordLimits. |
| sdk/include/opentelemetry/sdk/logs/logger_context.h | Adds limits storage and capability accessor APIs. |
| sdk/include/opentelemetry/sdk/logs/log_record_limits.h | New header defining LogRecordLimits defaults + NoLimits(). |
| sdk/include/opentelemetry/sdk/logs/exporter.h | Adds exporter capability method for limit enforcement. |
| sdk/include/opentelemetry/sdk/logs/batch_log_record_processor.h | Delegates “enforces limits” capability to exporter. |
| sdk/include/opentelemetry/sdk/common/attribute_utils.h | Adds UTF-8-safe prefix helper and truncating AttributeConverter. |
| exporters/otlp/test/otlp_populate_attribute_utils_test.cc | New tests for UTF-8-safe prefix behavior in OTLP utils. |
| exporters/otlp/test/otlp_log_recordable_test.cc | Adds OTLP log recordable tests for count/length enforcement. |
| exporters/otlp/src/otlp_populate_attribute_utils.cc | Adds max-length truncation support to OTLP AnyValue population. |
| exporters/otlp/src/otlp_log_recordable.cc | Enforces limits in OTLP log recordable and sets dropped count. |
| exporters/otlp/include/opentelemetry/exporters/otlp/otlp_populate_attribute_utils.h | Extends APIs with max-length and documents truncation semantics. |
| exporters/otlp/include/opentelemetry/exporters/otlp/otlp_log_recordable.h | Adds SetLogRecordLimits override + stored limits member. |
| exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_log_record_exporter.h | Marks OTLP HTTP exporter recordable as enforcing limits. |
| exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_log_record_exporter.h | Marks OTLP gRPC exporter recordable as enforcing limits. |
| exporters/otlp/include/opentelemetry/exporters/otlp/otlp_file_log_record_exporter.h | Marks OTLP file exporter recordable as enforcing limits. |
| exporters/otlp/CMakeLists.txt | Adds otlp_populate_attribute_utils_test executable. |
| exporters/otlp/BUILD | Adds Bazel target for otlp_populate_attribute_utils_test. |
| exporters/ostream/include/opentelemetry/exporters/ostream/log_record_exporter.h | Marks ostream exporter recordable as enforcing limits. |
| CHANGELOG.md | Documents the new LogRecord limits enforcement feature. |
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
LoggerProviderthroughLoggerContextto eachLogRecordcreated by
Logger::CreateLogRecord. The infrastructure existed in thedeclarative configuration tier (
LogRecordLimitsConfiguration) but wasnever wired into the runtime pipeline.
What changed
New header
sdk/include/opentelemetry/sdk/logs/log_record_limits.h:LogRecordLimitsstruct with spec defaultsattribute_count_limit = 128andattribute_value_length_limit = SIZE_MAX(unlimited).SDK Recordable hierarchy:
Recordablegains a non-pure virtualSetLogRecordLimitswith a no-opdefault 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.
ReadableLogRecordgains a non-pure virtualGetDroppedAttributesCountreturning zero by default.
ReadWriteLogRecordoverrides both.SetAttributechecks the countlimit 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.
MultiRecordablepropagatesSetLogRecordLimitsto every wrappedrecordable.
OTLP exporter:
OtlpLogRecordableoverridesSetLogRecordLimits.SetAttributedrops attributes beyond the count limit and increments the proto
LogRecord.dropped_attributes_countfield. Strings and string-arrayvalues are truncated after
OtlpPopulateAttributeUtils::PopulateAttributepopulates the proto
AnyValue.Wiring:
LoggerContextowns aLogRecordLimitsvalue and exposesGetLogRecordLimits(). A new fourth parameter is appended to theexisting constructor with a default-constructed default, so existing
call sites compile unchanged.
Logger::CreateLogRecord(both ABI v1 and ABI v2 variants) callsrecordable->SetLogRecordLimits(context_->GetLogRecordLimits())immediately after
MakeRecordable, before any user attribute write.LoggerProviderFactory::Createoverload acceptsLogRecordLimitsand builds aLoggerContextwith those limitsinternally.
Declarative configuration:
SdkBuilder::CreateLoggerProvidernow mapsLogRecordLimitsConfigurationfrom the parsedLoggerProviderConfigurationto the runtimeLogRecordLimitsandpasses 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):limits object is supplied.
counted. Replacing an existing key while at the limit must not drop.
int / double / bool pass through.
TrackingRecordableproduced by aTrackingProcessorconfirms that the limits configured onLoggerProviderreach the recordable returned byLogger::CreateLogRecord.exporters/otlp/test/otlp_log_recordable_test.ccaugmented with fourcases that verify the proto
dropped_attributes_countfield and thestring / array truncation logic.
Verification
Built and tested in the
otelcpp-devcontainer:log_record_limits_test9/9,otlp_log_recordable_test23/23,logger_sdk_test9/9,logger_provider_sdk_test9/9,simple_log_record_processor_test10/10,
batch_log_record_processor_test13/13. No regression.log_record_limits_test9/9.-fno-rtti(Bazel nortti CI mirror):log_record_limits_test9/9,otlp_log_recordable_test23/23.from a local clang-22 run.
git diff --checkwhitespace: clean.Known limitations
ElasticSearchRecordableis not modified in this PR. It inheritsthe no-op default and does not enforce limits. A follow-up PR can
add enforcement if desired.
a multibyte UTF-8 sequence will be truncated mid-sequence, matching
the existing Span attribute behavior in
OtlpRecordable.(
ParseLogRecordLimitsConfiguration) keeps the existingattribute_value_length_limit = 4096fallback for callers thatopt into the
limits:block without specifying a length. Aligningthat fallback with the spec default (unlimited) is left to a follow-up.