-
Notifications
You must be signed in to change notification settings - Fork 51
Add option to configure batch size #476
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
Conversation
0xnm
left a comment
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.
lgtm, I've added few suggestions.
| } | ||
|
|
||
| @Test | ||
| fun `𝕄 initialize native SDK 𝕎 initialize() {small batch size}`( |
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.
minor: since the size of DdSdkTest file is growing, maybe it is worth to convert some similar tests to ParametrizedTest? So that there is once test with just different inputs.
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.
I was looking for something like this! Thanks a lot I will update some of the tests that will benefit from it :)
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 are using them in the Android SDK codebase, you can check there.
| public uploadFrequency: UploadFrequency = DEFAULTS.uploadFrequency; | ||
|
|
||
| /** | ||
| * Sets the preferred size for uploaded batches of data. |
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.
I would suggest to have a bit more explanation on this parameter, because currently it is not obvious what does it change (yes, size is different, but what is the real impact?).
Android SDK says the following: Defines the batch size (impacts the size and number of requests performed by Datadog)., so at least we can hint the user that number of requests will be different, resulting in the different number of radio invocations (and smaller size has better chances to squeeze for the cost of more radio invocations).
iOS documentation says the following:
Defines the Datadog SDK policy when batching data together before uploading it to Datadog servers.
Smaller batches mean smaller but more network requests, whereas larger batches mean fewer but larger network requests.
What does this PR do?
Add an option to configure the batch size of data uploads
Review checklist (to be filled by reviewers)