Skip to content

feat: implement BatchLogRecordProcessor environment variables and val… #66

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

Open
harshitt13 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: implement BatchLogRecordProcessor environment variables and val… #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
79ef10e
feat: implement BatchLogRecordProcessor environment variables and val…
harshitt13 Jul 11, 2026
015d6bf
format code
harshitt13 Jul 11, 2026
19a9975
Merge upstream/main and resolve conflicts
harshitt13 Jul 19, 2026
d97aa78
applied suggested changes
harshitt13 Jul 20, 2026
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
19 changes: 19 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
two hand-rolled copies of the same assembly; wire output is unchanged
(same instrumentation-scope constant, same `OTel.resource(null)`
fallback, resolved by the caller so the transformer stays a pure leaf).
- **`BatchLogRecordProcessorConfig.fromEnvironment()`** — new factory
constructor symmetric with `BatchSpanProcessorConfig.fromEnvironment()`.
Spec defaults and validation now live on the config class (Single
Responsibility), eliminating duplication across `OTelEnv`,
`LogsConfiguration`, and the constructor.

### Changed
- **OTEL_BLRP_* env var validation now warns on invalid values** — previously,
invalid `OTEL_BLRP_SCHEDULE_DELAY` and `OTEL_BLRP_EXPORT_TIMEOUT` values
were silently ignored; they now emit `OTelLog.warn` diagnostics consistent
with BSP behavior. `OTEL_BLRP_SCHEDULE_DELAY=0` is now accepted as valid
(meaning "export as fast as possible"), and `OTEL_BLRP_EXPORT_TIMEOUT=0`
means "no limit", mirroring BSP semantics.
- **`OTelEnv._getPositiveIntEnv` now warns on unusable values** — non-numeric,
below-minimum, and above-maximum values all emit `OTelLog.warn`, giving
consistent diagnostics to every caller without per-site bookkeeping.
- **`OTelEnv.getBlrpConfig()` simplified to raw env reading** — domain-level
defaults, validation, and batch-to-queue clamping moved to
`BatchLogRecordProcessorConfig.fromEnvironment()`.

## [1.1.0-beta.9] - 2026-07-18

Expand Down
109 changes: 73 additions & 36 deletions lib/src/environment/otel_env.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,14 +474,6 @@ class OTelEnv {
return config;
}

/// Get Batch LogRecord Processor (BLRP) configuration from environment variables.
///
/// Returns a map containing the BLRP configuration read from environment variables.
/// Keys returned:
/// - 'scheduleDelay': Duration for the schedule delay
/// - 'exportTimeout': Duration for the export timeout
/// - 'maxQueueSize': int for maximum queue size
/// - 'maxExportBatchSize': int for maximum export batch size
/// Reads `OTEL_PROPAGATORS` (sdk-environment-variables.md, "General SDK
/// Configuration"): a comma-separated list of propagator names. Returns
/// the normalized (trimmed, lowercased) names, defaulting to the spec
Expand All @@ -498,43 +490,45 @@ class OTelEnv {
.toList();
}

/// Get Batch LogRecord Processor (BLRP) configuration from environment variables.
///
/// Returns a map containing the raw parsed BLRP values from environment
/// variables. Domain-level defaults, validation, and clamping belong in
/// [BatchLogRecordProcessorConfig.fromEnvironment].
///
/// Keys returned (only present when the env var is set and valid):
/// - 'scheduleDelay': Duration for the schedule delay
/// - 'exportTimeout': Duration for the export timeout
/// - 'maxQueueSize': int for maximum queue size
/// - 'maxExportBatchSize': int for maximum export batch size
static Map<String, dynamic> getBlrpConfig() {

@robert-northmind robert-northmind Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One thought here. I know there is an existing pattern here to return Map<String, dynamic> objects. But what if we instead returned like a named dart record. Something like:

typedef BlrpEnvironmentValues = ({
  Duration? scheduleDelay,
  Duration? exportTimeout,
  int? maxQueueSize,
  int? maxExportBatchSize,
});

Then we get the benefit that we know the types of each entry. and we don't need to use string-keys to find them. I think it would in general make the code a bit safer.
It reduces the need for like as int? or is Duration?
Not sure if we wanna go down this path though.
What are your thoughts on this @michaelbushe ?

final config = <String, dynamic>{};

// Get schedule delay
final scheduleDelay = _getEnv(otelBlrpScheduleDelay);
if (scheduleDelay != null) {
final delayMs = int.tryParse(scheduleDelay);
if (delayMs != null) {
config['scheduleDelay'] = Duration(milliseconds: delayMs);
}
// Get schedule delay — 0 is valid ("export as fast as possible")
final scheduleDelayMs =
_getPositiveIntEnv(otelBlrpScheduleDelay, minInclusive: 0);
if (scheduleDelayMs != null) {
config['scheduleDelay'] = Duration(milliseconds: scheduleDelayMs);
}

// Get export timeout
final exportTimeout = _getEnv(otelBlrpExportTimeout);
if (exportTimeout != null) {
final timeoutMs = int.tryParse(exportTimeout);
if (timeoutMs != null) {
config['exportTimeout'] = Duration(milliseconds: timeoutMs);
}
// Get export timeout — 0 is valid ("no limit")
final exportTimeoutMs =
_getPositiveIntEnv(otelBlrpExportTimeout, minInclusive: 0);
if (exportTimeoutMs != null) {
config['exportTimeout'] = Duration(milliseconds: exportTimeoutMs);
}

// Get max queue size
final maxQueueSize = _getEnv(otelBlrpMaxQueueSize);
if (maxQueueSize != null) {
final size = int.tryParse(maxQueueSize);
if (size != null) {
config['maxQueueSize'] = size;
}
// Get queue and batch sizes — must be positive (>0)
final parsedMaxQueueSize =
_getPositiveIntEnv(otelBlrpMaxQueueSize, minInclusive: 1);
Comment on lines +522 to +523

@robert-northmind robert-northmind Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One thought here. Does this pub a bit of Blrp domain knowledge in the OTelEnv parsing? Like that a queue needs to be bigger than 0?
What if the OTelEnv parsing only checks if the env variable exisits and has right type? Like it could warn if an int is a string.

But then the logic to check if the acutal int is bigger than 0, that could happen in the BatchLogRecordProcessorConfig.fromEnvironment()?

if (parsedMaxQueueSize != null) {
config['maxQueueSize'] = parsedMaxQueueSize;
}

// Get max export batch size
final maxExportBatchSize = _getEnv(otelBlrpMaxExportBatchSize);
if (maxExportBatchSize != null) {
final size = int.tryParse(maxExportBatchSize);
if (size != null) {
config['maxExportBatchSize'] = size;
}
final parsedMaxExportBatchSize =
_getPositiveIntEnv(otelBlrpMaxExportBatchSize, minInclusive: 1);
if (parsedMaxExportBatchSize != null) {
config['maxExportBatchSize'] = parsedMaxExportBatchSize;
}

return config;
Expand Down Expand Up @@ -638,4 +632,47 @@ class OTelEnv {

return null;
}

/// Get a non-negative integer environment variable value.
///
/// Returns null when not set, non-numeric, or outside the accepted range.
/// Warns via [OTelLog.warn] when the raw value is present but unusable
/// (non-numeric, below [minInclusive], or above [maxInclusive]).
static int? _getPositiveIntEnv(
Comment thread
harshitt13 marked this conversation as resolved.

@robert-northmind robert-northmind Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does this need to be private? if it would be public, we could easily directly test the logic in here. Just a thought

String name, {
required int minInclusive,
int? maxInclusive,
}) {
final rawValue = _getEnv(name);
if (rawValue == null) {
return null;
}

final value = int.tryParse(rawValue);
if (value == null) {
if (OTelLog.isWarn()) {
OTelLog.warn('OTelEnv: Illegal non-numeric value for $name: '
'"$rawValue", ignoring.');
}
return null;
}

if (value < minInclusive) {
if (OTelLog.isWarn()) {
OTelLog.warn('OTelEnv: Illegal value for $name: $value is below '
'minimum $minInclusive, ignoring.');
}
return null;
}

if (maxInclusive != null && value > maxInclusive) {
if (OTelLog.isWarn()) {
OTelLog.warn('OTelEnv: Illegal value for $name: $value exceeds '
'maximum $maxInclusive, ignoring.');
}
return null;
}

return value;
}
}
98 changes: 98 additions & 0 deletions lib/src/logs/export/batch_log_record_processor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'dart:collection';
import 'package:dartastic_opentelemetry_api/dartastic_opentelemetry_api.dart';
import 'package:synchronized/synchronized.dart';

import '../../environment/otel_env.dart';
import '../log_record_processor.dart';
import '../readable_log_record.dart';
import 'log_record_exporter.dart';
Expand All @@ -16,6 +17,12 @@ import 'log_record_exporter.dart';
/// This class configures how the batch log record processor behaves, including
/// queue size limits, export scheduling, and batch size parameters.
class BatchLogRecordProcessorConfig {
/// Stand-in for "no limit": `OTEL_BLRP_EXPORT_TIMEOUT=0` means no timeout
/// per spec, represented as the max milliseconds in a 32-bit integer
/// (~24.8 days). Web-safe and within Dart timer limits for
/// `Future.timeout()`.
static const Duration noLimit = Duration(milliseconds: 0x7FFFFFFF);

/// The maximum queue size for log records. After this is reached,
/// log records will be dropped.
final int maxQueueSize;
Expand All @@ -41,6 +48,97 @@ class BatchLogRecordProcessorConfig {
this.maxExportBatchSize = 512,
this.exportTimeout = const Duration(seconds: 30),
});

/// Creates a configuration by reading `OTEL_BLRP_*` environment variables

@robert-northmind robert-northmind Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What if we make the BatchLogRecordProcessorConfig constructor private, and make all its params required. So that we dont have to provide the defaults there.
And instead, we can only create BatchLogRecordProcessorConfig either via the factory BatchLogRecordProcessorConfig.fromEnvironment() or a factory BatchLogRecordProcessorConfig({...});.

Then in those factories we could do the validation. something like:

factory BatchLogRecordProcessorConfig({
    int maxQueueSize = defaultQueueSize,
    // ...
  }) {
    if (!_validQueueSize(maxQueueSize)) {
      throw ArgumentError.value(maxQueueSize, 'maxQueueSize');
    }
    return BatchLogRecordProcessorConfig._(
      maxQueueSize: maxQueueSize,
    );
  }

factory BatchLogRecordProcessorConfig.fromEnvironment() {
    final env = OTelEnv.getBlrpConfig();
    final queueSize = _validQueueSize(env.maxQueueSize)
        ? env.maxQueueSize!
        : defaultQueueSize;
    if (env.maxQueueSize != null && !_validQueueSize(env.maxQueueSize)) {
      OTelLog.warn(/* invalid environment value */);
    }
    return BatchLogRecordProcessorConfig(
      maxQueueSize: queueSize,
    );
  }

/// via [OTelEnv]. Falls back to standard OTel defaults if variables are
/// missing or invalid.
///
/// | Environment Variable | Type | Default | Notes |
/// |-----------------------------------|----------|----------|----------------------------------------------------------|
/// | `OTEL_BLRP_SCHEDULE_DELAY` | Duration | `1000` | Delay between exports (ms). 0 is valid (export ASAP). |
/// | `OTEL_BLRP_EXPORT_TIMEOUT` | Timeout | `30000` | Export timeout (ms). 0 means no limit. |
/// | `OTEL_BLRP_MAX_QUEUE_SIZE` | Integer | `2048` | Maximum log record queue size. |
/// | `OTEL_BLRP_MAX_EXPORT_BATCH_SIZE` | Integer | `512` | Maximum batch size. Must be ≤ `MAX_QUEUE_SIZE`. |
///
/// Invalid or out-of-range values emit an [OTelLog.warn] diagnostic and
/// fall back to the spec default.
factory BatchLogRecordProcessorConfig.fromEnvironment() {
final env = OTelEnv.getBlrpConfig();

var queueSize = (env['maxQueueSize'] as int?) ?? 2048;

@robert-northmind robert-northmind Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We have now this 2048 defined here and in the constructor in line 46 also, and in a few other places in this file.
should we break that our to a private static default variable?
Like: static const _defaultQueueSize = 2048;

var batchSize = (env['maxExportBatchSize'] as int?) ?? 512;

// --- scheduleDelay ---
// Spec type: Duration. Zero is valid ("export as fast as possible").
// Negative values MUST warn and fall back to default.
Duration scheduleDelay;
if (env['scheduleDelay'] is Duration) {
final delay = env['scheduleDelay'] as Duration;
if (delay.inMilliseconds >= 0) {
scheduleDelay = delay;
} else {
if (OTelLog.isWarn()) {
OTelLog.warn('BatchLogRecordProcessorConfig: Negative '
'OTEL_BLRP_SCHEDULE_DELAY (${delay.inMilliseconds} ms) is '
'invalid per spec, using default 1000 ms.');
}
scheduleDelay = const Duration(milliseconds: 1000);
}
} else {
scheduleDelay = const Duration(milliseconds: 1000);
}

// --- exportTimeout ---
// Spec type: Timeout. Zero means "no limit" — substitute a very large
// duration. Negative values MUST warn and fall back to default.
Duration exportTimeout;
if (env['exportTimeout'] is Duration) {
final timeout = env['exportTimeout'] as Duration;
if (timeout.inMilliseconds == 0) {
exportTimeout = noLimit;
} else if (timeout.inMilliseconds > 0) {
exportTimeout = timeout;
} else {
if (OTelLog.isWarn()) {
OTelLog.warn('BatchLogRecordProcessorConfig: Negative '
'OTEL_BLRP_EXPORT_TIMEOUT (${timeout.inMilliseconds} ms) is '
'invalid per spec, using default 30000 ms.');
}
exportTimeout = const Duration(milliseconds: 30000);
}
} else {
exportTimeout = const Duration(milliseconds: 30000);
}

// --- Validation Logic ---
if (queueSize <= 0) {
if (OTelLog.isWarn()) {
OTelLog.warn('BatchLogRecordProcessorConfig: Non-positive '
'OTEL_BLRP_MAX_QUEUE_SIZE ($queueSize) is invalid per spec, '
'using default 2048.');
}
queueSize = 2048;
}
if (batchSize <= 0) {
if (OTelLog.isWarn()) {
OTelLog.warn('BatchLogRecordProcessorConfig: Non-positive '
'OTEL_BLRP_MAX_EXPORT_BATCH_SIZE ($batchSize) is invalid per '
'spec, using default 512.');
}
batchSize = 512;
}
// Spec rule: maxExportBatchSize must be less than or equal to maxQueueSize
if (batchSize > queueSize) {
batchSize = queueSize;
}

return BatchLogRecordProcessorConfig(
maxQueueSize: queueSize,
maxExportBatchSize: batchSize,
scheduleDelay: scheduleDelay,
exportTimeout: exportTimeout,
);
}
}

/// A LogRecordProcessor that batches log records before export.
Expand Down
46 changes: 29 additions & 17 deletions lib/src/logs/export/logs_config.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,30 +189,42 @@ class LogsConfiguration {

/// Creates a log record processor with BLRP configuration from environment.
static LogRecordProcessor _createProcessor(LogRecordExporter exporter) {
final blrpConfig = OTelEnv.getBlrpConfig();
final processorConfig = BatchLogRecordProcessorConfig.fromEnvironment();
return BatchLogRecordProcessor(exporter, processorConfig);
}

/// Builds [BatchLogRecordProcessorConfig] from a BLRP environment config map.
///
/// Exposed for testing to validate normalization and spec-rule handling.
/// Prefer [BatchLogRecordProcessorConfig.fromEnvironment] for production use.
static BatchLogRecordProcessorConfig buildBatchLogRecordProcessorConfig(
Map<String, dynamic> blrpConfig,
) {
Comment on lines +200 to +202

@robert-northmind robert-northmind Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I didn't quite follow the need for this one. this is only used for testing right?
is there some other way to structure this. so that we dont need public api for testing only needs?

if (blrpConfig.isEmpty) {
// Use defaults
return BatchLogRecordProcessor(
exporter,
const BatchLogRecordProcessorConfig(),
);
return const BatchLogRecordProcessorConfig();
}

// Build config from environment
final scheduleDelay = blrpConfig['scheduleDelay'] as Duration?;
final exportTimeout = blrpConfig['exportTimeout'] as Duration?;
final maxQueueSize = blrpConfig['maxQueueSize'] as int?;
final maxExportBatchSize = blrpConfig['maxExportBatchSize'] as int?;

return BatchLogRecordProcessor(
exporter,
BatchLogRecordProcessorConfig(
scheduleDelay: scheduleDelay ?? const Duration(milliseconds: 1000),
exportTimeout: exportTimeout ?? const Duration(seconds: 30),
maxQueueSize: maxQueueSize ?? 2048,
maxExportBatchSize: maxExportBatchSize ?? 512,
),
final maxQueueSize = blrpConfig['maxQueueSize'] as int? ?? 2048;
var maxExportBatchSize = blrpConfig['maxExportBatchSize'] as int? ?? 512;

if (maxExportBatchSize > maxQueueSize) {
if (OTelLog.isWarn()) {
OTelLog.warn(
'LogsConfiguration: maxExportBatchSize ($maxExportBatchSize) exceeds '
'maxQueueSize ($maxQueueSize). Clamping batch size to queue size.',
);
}
maxExportBatchSize = maxQueueSize;
}

return BatchLogRecordProcessorConfig(
scheduleDelay: scheduleDelay ?? const Duration(milliseconds: 1000),
exportTimeout: exportTimeout ?? const Duration(seconds: 30),
maxQueueSize: maxQueueSize,
maxExportBatchSize: maxExportBatchSize,
);
}

Expand Down
Loading
Loading