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.

Sign up for GitHub

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

Update LogRecord batching processor behavior description #4409

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
+4 −0
Open

Update LogRecord batching processor behavior description #4409

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
bb2add8
Add more description to OTEL_BSP_SCHEDULE_DELAY and OTEL_BLRP_SCHEDUL…
JDUNNIN Feb 10, 2025
0b10e27
Correct name of property
JDUNNIN Feb 10, 2025
fe114f8
Add reference to Batching processor for Trace and Logs + make Logs de…
JDUNNIN Feb 11, 2025
dea0d32
Merge branch 'main' into issue-4402
JDUNNIN Feb 11, 2025
3fc4784
Change to relative hyperlink + remove full stops added by mistake (ad…
JDUNNIN Feb 14, 2025
86aaf01
Merge branch 'issue-4402' of github.com:JDUNNIN/opentelemetry-specifi…
JDUNNIN Feb 14, 2025
eb7868f
Add equivallent description from Trace to Logs batch
JDUNNIN Feb 14, 2025
73500cb
Merge branch 'main' into issue-4402
pellared Feb 14, 2025
8c22ecc
Add shutdown detail
JDUNNIN Feb 14, 2025
63df65c
Merge branch 'issue-4402' of github.com:JDUNNIN/opentelemetry-specifi…
JDUNNIN Feb 14, 2025
3173b33
Fix relative links by adding .md
JDUNNIN Feb 14, 2025
e929a07
Merge branch 'main' into issue-4402
JDUNNIN Feb 24, 2025
85e53ef
Unpick all changes in logs+trace sdk.md files (raise new issue for th…
JDUNNIN Feb 26, 2025
b59eb0f
Merge branch 'main' into issue-4402
JDUNNIN Feb 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions specification/configuration/sdk-environment-variables.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ Depending on the value of `OTEL_TRACES_SAMPLER`, `OTEL_TRACES_SAMPLER_ARG` may b

## Batch Span Processor

Refer to the Trace SDK [Batching processor specification](../trace/sdk/#batching-processor) for more information on these configuration parameters.

| Name | Description | Default | Type | Notes |
|--------------------------------|------------------------------------------------------------------|---------|--------------|-----------------------------------------------------------------------------------|
| OTEL_BSP_SCHEDULE_DELAY | Delay interval (in milliseconds) between two consecutive exports | 5000 | [Duration][] | |
Expand All @@ -217,6 +219,8 @@ Depending on the value of `OTEL_TRACES_SAMPLER`, `OTEL_TRACES_SAMPLER_ARG` may b

## Batch LogRecord Processor

Refer to the Logs SDK [Batching processor specification](../logs/sdk/#batching-processor) for more information on these configuration parameters.

| Name | Description | Default | Type | Notes |
|---------------------------------|------------------------------------------------------------------|---------|--------------|------------------------------------------------------------------------------------|
| OTEL_BLRP_SCHEDULE_DELAY | Delay interval (in milliseconds) between two consecutive exports | 1000 | [Duration][] | |
Expand Down
18 changes: 17 additions & 1 deletion specification/logs/sdk.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,20 @@ representations to the configured `LogRecordExporter`.
The processor MUST synchronize calls to `LogRecordExporter`'s `Export`
to make sure that they are not invoked concurrently.

The processor SHOULD export a batch when any of the following happens AND the
Copy link
Member

Choose a reason for hiding this comment

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

Batching processor

This is an implementation of the LogRecordProcessor which create batches
of LogRecords and passes the export-friendly ReadableLogRecord
representations to the configured LogRecordExporter.
The processor MUST synchronize calls to LogRecordExporter's Export
to make sure that they are not invoked concurrently.

A batch SHOULD be exported when any of the following conditions are met:

  • The scheduled delay (scheduledDelayMillis) elapses.
  • The queue contains at least maxExportBatchSize LogRecords.
  • ForceFlush is called.
  • Shutdown is called.

If the queue is empty when an export is triggered, the processor MAY either export an empty batch or skip the export and consider it complete immediately.

^Wording suggestion.

Additional notes from implementing this:
I don't think the spec has explicitly clarified the following, so I went with my own interpretations!

  1. Does time taken for export be considered? For example, should export gets triggered at consistent times T,T+d,T+2d,T+2d
    Or
    T, T+e1+d, T+e2+2d... where e1,e2 represents the time take for the export itself
  2. What happens if export() takes more time than the interval itself? Should the next export() be skipped? or attempted?

previous export call has returned:

- `scheduledDelayMillis` after the processor is constructed OR the first `LogRecord`
Copy link
Member

Choose a reason for hiding this comment

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

the first LogRecord
is received by the log record processor.

Why export when first record is received 🤔 ?

Copy link
Member

@pellared pellared Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think that the original author meant "delay after first record'. Probably this could be clarified, but I do not feel it has to be done in this PR.

is received by the log record processor.
Copy link
Member

Choose a reason for hiding this comment

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

Why would we want to have the 2nd part? Asking this question more coming from "why do we want to have complex rules instead of simple rules".

  • the first LogRecord is received by the log record processor.

Copy link
Author

Choose a reason for hiding this comment

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

I was bringing this documentation in line with how the trace batching processor describes behaviour. I'm happy to make changes (with direction) in this PR or could there be another issue to address this part of the specification for both logs and trace batching?

My original issue was with environment variables not referring back to related parts of the SDKs, and us finding this "missing" content for logs is extras that have been suggested as improvements. Typical case of the more you look, the more you find :)

Copy link
Member

Choose a reason for hiding this comment

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

I see that you were trying to get the spec from Trace and copy it to Log. But IMHO, the trace section itself is not very clear. (I am sure I have discussed this before, but not able to find that discussion anymore!!)

https://github.com/open-telemetry/opentelemetry-specification/pull/4409/files#r1968141138 - This comment has my suggested wording for both trace/log. See if it is simpler and yet conveys the full intent.

- `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.
- The queue contains `maxExportBatchSize` or more `LogRecord`s.
- `ForceFlush` is called.

If the queue is empty when an export is triggered, the processor MAY export
an empty batch OR skip the export and consider it to be completed immediately.

**Configurable parameters:**

* `exporter` - the exporter where the `LogRecord`s are pushed.
Expand All @@ -431,7 +445,9 @@ to make sure that they are not invoked concurrently.
* `exportTimeoutMillis` - how long the export can run before it is cancelled.
The default value is `30000`.
* `maxExportBatchSize` - the maximum batch size of every export. It must be
smaller or equal to `maxQueueSize`. The default value is `512`.
smaller or equal to `maxQueueSize`. If the queue reaches `maxExportBatchSize`
a batch will be exported even if `scheduledDelayMillis` milliseconds have not
elapsed. The default value is `512`.

## LogRecordExporter

Expand Down