docs(logs): document batching processor triggers - #5261
Conversation
|
|
arminru
added
editorial
Pull request dashboard statusWaiting on the author · refreshed 2026-08-24 18:58 UTC Respond to 1 review item (e.g. link a commit, explain why not, ask a follow-up):
Status above doesn't look right?
|
Adopt review feedback: invoke the LogRecordExporter's Export, drop the construction-vs-first-record OR in the first trigger, and remove the empty-batch OR skip flexibility.
| If the queue is empty when an export is triggered, the processor MAY export | ||
| an empty batch. |
There was a problem hiding this comment.
An alternative approach for us to consider:
| If the queue is empty when an export is triggered, the processor MAY export | |
| an empty batch. | |
| If the queue is empty when an export is triggered, the processor SHOULD invoke the `LogRecordExporter`'s `Export` and provide an empty batch. |
In this way the exporter can decide to simply do nothing when the input batch is empty, or the exporter can do some maintenance job (e.g. just sending some heartbeat, so the backend would be able to tell "I don't see data because there is no data, and my app is alive" versus "I don't see data, my app might be dead").
There was a problem hiding this comment.
I like the SHOULD export with empty batch behavior too. Since this was not specified before for Log SDK, I think it is okay. One thing to note is - the wording is copied as-is from Tracing sdk spec, which is marked stable - changing a MAY to SHOULD might not be allowed for existing stable spec.
There was a problem hiding this comment.
Adopted — the empty-queue clause now reads: "the processor SHOULD invoke the LogRecordExporter's Export and provide an empty batch, so the exporter can decide how to handle it." Pushed as 5acfb3f.
There was a problem hiding this comment.
Good point about the stable Trace wording — the Trace SDK text is untouched; this change is only in the Logs SDK block this PR adds (Logs SDK batching wording was previously unspecified), so the stable-spec constraint does not apply here. The clause now reads: "the processor SHOULD invoke the LogRecordExporter's Export and provide an empty batch". Pushed as 5acfb3f.
Adopt review feedback: when an export is triggered with an empty queue, the processor SHOULD invoke the exporter with an empty batch so the exporter can decide how to handle it.
| - `scheduledDelayMillis` after the previous export timer ends, OR the previous | ||
| export completes, OR the first `LogRecord` is added to the queue after the | ||
| previous export timer ends or previous batch completes. |
There was a problem hiding this comment.
I fail to understand this bullet
"OR the previous export completes" is this correct? Why "OR"? Is it not about "AND the previous export call has returned:" which was defined previously?
What is an export timer? When does it reset?
There was a problem hiding this comment.
Also note that the preceding MUST already ensures that Export calls are not concurrent, so “AND the previous export call has returned” is redundant if it only restates serialization.
Could we separate export triggers from timer-restart semantics? For example:
The processor SHOULD invoke the `LogRecordExporter`'s `Export` when:
- the scheduled-delay timer expires;
- the queue contains `maxExportBatchSize` or more `LogRecord`s;
- `ForceFlush` is called; or
- `Shutdown` is called.
If a trigger occurs while an `Export` call is in progress,
the processor SHOULD service the trigger after that call returns.
|
Hi @aryansk — just a friendly reminder that this pull request is waiting on you. The dashboard status comment has the open items and is kept current.
|
6 participants
Resolves #4434
Description
Document the Logs SDK batching processor's export triggers, empty-queue
behavior, and the relationship between
maxExportBatchSizeandscheduledDelayMillis. The wording follows the concise behavior described inthe linked discussion on #4409 and keeps the Logs SDK guidance explicit about
ForceFlushandShutdown.Testing
git diff --checkmake markdownlint