Repository navigation
Conversation
Comment on lines
+306
to
+314
| // resource_logs + scope_logs + one op per record. | ||
| let payload = logs_with_records(1_001).encode_to_vec(); | ||
|
|
||
| assert!(matches!( | ||
| parse_logs_data(OtelFormat::Protobuf, &payload, 1000), | ||
| Err(Error::Invalid(DiscardReason::InvalidProtobuf)) | ||
| )); | ||
| assert!(parse_logs_data(OtelFormat::Protobuf, &payload, 1003).is_ok()); | ||
| } |
There was a problem hiding this comment.
Resource/scope attributes re-cloned per log record with no memoization
After bounded decode, expand still fans out shared resource/scope attributes onto every log record via otel_to_sentry_log, so a small payload (one large shared attr + many tiny records) under max_ops materializes multi-GB output before any size check.
Evidence
expand(otel.rs:87-105) loops everylog_recordand callsotel_to_sentry_log(log_record, resource, scope)with the same sharedresource/scope.otel_scope_into_attributes(relay-otel/src/lib.rs:112-128).clone()s every resource/scope attribute into each log'sAttributesmap with no memoization.parse_logs_data/decode_boundedonly meters input field occurrences (max_ops); empty records cost 1 op each (seetest_abusive_protobufand bounded_decodetest_logs_many_records_exceed_budget), while a large string attr still costs a few ops regardless of byte size.max_log_size(2 MiB) is enforced only later inLogsProcessor(validate::size), after OTEL expand has already built and serialized the full intermediate container viacreate_intermediates.
Identified by Warden · wrdn-dos-review · DZG-CT3
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Actually hook up the proto bounding code to our otel log processor. Note that there is some noise in this PR due to upgrading otel (in relay-otel) to latest in order to get the proto descriptor changes, hence the "Strindex" lines. Setting them to default values is the right thing to do, here (the index refers to a new capability to intern strings in a proto message and just refer to them by index.)
Tests by Claude, but they look sane to me.