-
Notifications
You must be signed in to change notification settings - Fork 237
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
base: main
Are you sure you want to change the base?
feat: Add log record attribute limits #1696
Conversation
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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
:
opentelemetry-ruby/sdk/lib/opentelemetry/sdk/internal.rb
Lines 51 to 63 in 98c629e
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 |
Similar to SpanLimits, add a LogRecordLimits class that handles configuration of attribute count and value length values.
Closes #1516