Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add log record attribute limits #1696

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kaylareopelle
Copy link
Contributor

Similar to SpanLimits, add a LogRecordLimits class that handles configuration of attribute count and value length values.

Closes #1516

Similar to SpanLimits, add a LogRecordLimits class that handles
configuration of attribute count and value length values.
end

def validate_attributes(attrs)
# Similar to Internal.valid_attributes?, but with different messages
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update Internal.valid_attributes? to accept log record as a kind?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could! I wanted to minimize the impact on the tracing SDK and other gems, so I didn't reach toward refactoring it initially. Would you prefer a refactor over this duplication?

One way to update it could be to add a new signal attribute, refactor the message to remove span, and perhaps use log_record.body for owner:

def valid_attributes?(owner, kind, attrs)
attrs.nil? || attrs.each do |k, v|
if !valid_key?(k)
OpenTelemetry.handle_error(message: "invalid #{kind} attribute key type #{k.class} on span '#{owner}'")
return false
elsif !valid_value?(v)
OpenTelemetry.handle_error(message: "invalid #{kind} attribute value type #{v.class} for key '#{k}' on span '#{owner}'")
return false
end
end
true
end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Logs SDK - Log Record Limits
2 participants