Skip to content

Do not sending telemetry while on freeze or disabled#43

Open
pamellix wants to merge 3 commits intomainfrom
do-not-sending-telemetry-while-disabled
Open

Do not sending telemetry while on freeze or disabled#43
pamellix wants to merge 3 commits intomainfrom
do-not-sending-telemetry-while-disabled

Conversation

@pamellix
Copy link
Copy Markdown
Collaborator

@pamellix pamellix commented Apr 15, 2026

📦 Published PR as canary version: 2.0.18--canary.43.25312371161.0

✨ Test out this PR locally via:

npm install @salutejs/web-telemetry@2.0.18--canary.43.25312371161.0
# or 
yarn add @salutejs/web-telemetry@2.0.18--canary.43.25312371161.0

@pamellix pamellix requested a review from SeanSilke April 15, 2026 18:32
Copy link
Copy Markdown
Collaborator

@SeanSilke SeanSilke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нужно увести за флаг. Флаг пусть будет - pauseSendingWhenPageInactive .
И нужно решить проблему накопления событий когда отправка на паузе.
this.events.length >= this.config.buffSize. Сейчас scheduleSend работает так что переполнения не будет, при паузе будет переполнение.

В чем именно баг дизайна

  • Проверка this.events.length >= this.config.buffSize находится после canSendTelemetry().
  • Значит при pause-state ветка переполнения вообще не исполняется.
  • Итог: events.length может стать сколь угодно большим.

Можно делать принудительную отправку или ограничивать размер очереди.
Мне кажется предпочтительным вариант с maxQueueSize

Добавить maxQueueSize и политику дропа:

  • maxQueueSize (например 1000 по умолчанию)
  • queueOverflowStrategy: 'drop_oldest' | 'drop_newest' (обычно берут drop_oldest)
  • счетчик droppedEventsCount для диагностики

Логика:

  • В push() после events.push(...) сразу проверять maxQueueSize.
    Если превышен:
  • drop_oldest: events.splice(0, overflow)
  • drop_newest: events.length = maxQueueSize
  • scheduleSend() пусть дальше работает как сейчас (с учетом lifecycle).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants