segmentio/analytics-node

Flush on first message should be explicit configurable

ioann opened this issue · 1 comments

ioann commented

In enqueue method present logic with calling asynchronous flush without await. No any legal or documented way to avoid it.

if (!this.flushed) {
    this.flushed = true
    this.flush(callback)
    return
  }

Yes, it's still JS and I can force set private field flushed to true. But it's so implicit and hard to understand without investigation source code.

It becomes more confusing if you set flushAt option to 10 but anyway first message fire flush.
This is looks as surprise compare to documented behaviour
flushAt:Number - The number of messages to enqueue before flushing.

In addition in AWS Lambda (or any similar environment) this unexpected flush just skipped because no await and lambda finished faster then flush

I saw section Batching in docs with remark about default batch behaviour

  • The very first time it gets a message.
  • Every 20 messages (controlled by options.flushAt).
  • If 10 seconds has passed since the last flush (controlled by options.flushInterval)

I suggest pop up field flushed in options with some explicit naming to make possibility configure it (flushAtFirst or some similar)

Anyway this logic (hardcoded flush on first message without await) looks like very implicit and confused. Maybe need think about rework it or even remove.

felipe-najson-ntf commented

Hi @ioann, thanks for your contribution. I understand your confusion but the idea of ​​flushing the first message is mainly to make sure that everything is correctly configured and that all the corresponding messages will be sent in the next flush.

We will also be evaluating this request with the team and we will give you an answer in the next few days.

Greetings, Segment team.

  • 👍2