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

BufferOption configuration not working as expected #827

Closed
danigutierrezayuso opened this issue Sep 20, 2023 · 2 comments
Closed

BufferOption configuration not working as expected #827

danigutierrezayuso opened this issue Sep 20, 2023 · 2 comments
Labels
type:defect Bugs or weaknesses. The issue has to contain steps to reproduce.
Milestone

Comments

@danigutierrezayuso
Copy link
Contributor

We've seen that the bufferOption on the EmitterConfiguration is not being used to check if an event should be sent directly or not.

When sending a new event the function 'addPayload' from the Emitter class is called, and that function is going to save the event in the 'eventStore' (SQLite in our case) and then is going to call the flush() function directly, so every time a new event is added there will be a new request sending that single event to the server.

Maybe on that addPayload function we should only call the flush function when the number of events saved in the eventStore has reached the bufferOption value. Doing so I think we can have the behavior described in the documentation:

/// Sets whether the buffer should send events instantly or after the buffer
/// has reached it's limit. By default, this is set to BufferOption Default.
@objc
    var bufferOption: BufferOption { get set }

Current behavior
After setting the bufferOption configuration to defaultGroup (10 events) the events are being sent one by one.

Expected behavior
After setting the bufferOption configuration to defaultGroup (10 events) the events are being sent on requests with a payload containing an array of 10 events.

@danigutierrezayuso danigutierrezayuso added the type:defect Bugs or weaknesses. The issue has to contain steps to reproduce. label Sep 20, 2023
@danigutierrezayuso danigutierrezayuso changed the title BufferOption configuration not being used BufferOption configuration not working as expected Sep 20, 2023
@matus-tomlein
Copy link
Contributor

Hi @danigutierrezayuso, thanks for reporting this! I agree that the current behaviour is quite unexpected and doesn't fit with the buffer configuration options. It is on radar to revisit this, we'll discuss it in the team soon.

@danigutierrezayuso
Copy link
Contributor Author

I've created a PR with changes to fix this behavior (it doesn't include any tests though): #833

matus-tomlein pushed a commit that referenced this issue Nov 30, 2023
# Conflicts:
#	Sources/Core/Emitter/Emitter.swift
matus-tomlein pushed a commit that referenced this issue Dec 1, 2023
# Conflicts:
#	Sources/Core/Emitter/Emitter.swift
matus-tomlein pushed a commit that referenced this issue Dec 7, 2023
# Conflicts:
#	Sources/Core/Emitter/Emitter.swift
matus-tomlein pushed a commit that referenced this issue Dec 8, 2023
# Conflicts:
#	Sources/Core/Emitter/Emitter.swift
matus-tomlein pushed a commit that referenced this issue Jan 19, 2024
# Conflicts:
#	Sources/Core/Emitter/Emitter.swift
@matus-tomlein matus-tomlein added this to the 6.0.0 milestone Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:defect Bugs or weaknesses. The issue has to contain steps to reproduce.
Projects
None yet
Development

No branches or pull requests

2 participants