No limits for log record body

[pellared]

What are you trying to achieve?

According to the docs there are only limits for the log attributes. See: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md#logrecord-limits

LogRecord attributes MUST adhere to the common rules of attribute limits.

From https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/common/README.md#attribute-limits:

If there are no limits placed on attributes, they can quickly exhaust available memory, resulting in crashes that are difficult to recover from safely.

The same problem is applicable for log record body value.

Is it intentional or an oversight?
Should the limits be also applied for body value (even though that the whole naming is around attributes)?
Should we have different limits for log record value?

[pellared]

PTAL @open-telemetry/specs-logs-approvers

[djaglowski]

An important scaling factor for attributes is that they are commonly used by backends for indexing or other optimizations. Do we expect the body to be treated similarly by backends? If not, we probably don't need to apply the same strict limits, though other limits may be necessary.

Related, how would these limits apply to a non-structured body?

[jack-berg]

We might not want the limits by default, but I think its reasonable to have them available. Currently, its not possible to configure any limits for the body, and its cumbersome to do in the collector (see this slack convo - it previously wasn't possible).

Related, how would these limits apply to a non-structured body?

The key question is how do attribute limits work on an AnyValue type. Note this will be relevant for events as well.

The common attribute limits provide configuration for the max allowed attribute count, and the maximum attribute length limit.

AnyValue is a recursive data structure, and the common attribute limits were designed for a flat list of key value pairs. Here's how we might apply the rules to AnyValue:

• AnyValue is oneof: string_value, bool_value, int_value, double_value, array_value, kv_list_value, bytes_value
  • If bool_value, int_value, double_value, ignore attribute limits
  • If string_value, bytes_value, truncate the value to the AttributeValueLengthLimit
  • If kv_list_value, limit the entires to AttributeCountLimit, apply limits recursively to each AnyValue value
  • If array_value, apply limits recursively to each AnyValue value

Note that the current rules don't limit the number of entries in an array, which is odd. I've carried that rule forward into the proposal above, but perhaps there should be a third property which limits the number of array entries.

[pellared]

@jack-berg, I do not think that it would be good to use AttributeValueLengthLimit and AttributeCountLimit for Body. Reasons:

1. "we probably don't need to apply the same strict limits, though other limits may be necessary." by @djaglowski
2. While Body uses the same "structure" as Attributes they are not semantically equivalent therefore I do not think we should use a single configuration to control both of them.

For now, (as a baby-step) I simply propose to clarify in the specification that the attribute limits do not apply on the Body.

Side note: SeverityText has also no limits.

[svrnm]

@pellared @jack-berg please take a look, is this still needed? how can we move this forward?

[pellared]

Hard for me to evaluate as Logs are still Beta in OTel Go and personally, I have no feedback on how much this feature is needed. I think we can still leave a "community-feedback".