-
Notifications
You must be signed in to change notification settings - Fork 136
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
RUMM-303 Logging sampling rate #1045
Conversation
Datadog ReportBranch report: ✅ |
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.
Thanks, @maciejburda 👏
It's great to be able to have a global sampling rate in Datadog.Configuration
and be able to override that in any Logger
instance 👍 Also, the tests and objc interface looks great!
I see 2 things:
- We can align with other sampling rate and make it non-optional, it will simplify things.
- The sampling rate should be applied to
RemoteLogger
only, but all log entries to theConsoleLogger
should print.
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.
Really well done 🏅👍 - all changes make sense and tests coverage is nice 👌.
I left few minor feedbacks, but I don't see major blockers. Good to go after addressing @maxep 's feedback 🚀.
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.
Awesome 🚀 - looks good and reads easy 👍.
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.
Looks great, thank you 👍
What and why?
Adds capability of sampling logs. It was pointed out in the Github issue that this feature already exists in Android.
How?
Added implementation allows configuring sampling rate for logging both globally from configuration and locally for logger using builder.
Review checklist
Custom CI job configuration (optional)