Skip to content

Send logs in batches #4378

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
wants to merge 1 commit into
base: feat/logs-options
Choose a base branch
from
Open

Send logs in batches #4378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import io.sentry.Sentry
import io.sentry.SentryEnvelope
import io.sentry.SentryEvent
import io.sentry.SentryLogEvent
import io.sentry.SentryLogEvents
import io.sentry.SentryOptions
import io.sentry.SentryReplayEvent
import io.sentry.Session
Expand Down Expand Up @@ -190,6 +191,10 @@ class SessionTrackingIntegrationTest {
TODO("Not yet implemented")
}

override fun captureBatchedLogEvents(logEvents: SentryLogEvents) {
TODO("Not yet implemented")
}

override fun getRateLimiter(): RateLimiter? {
TODO("Not yet implemented")
}
Expand Down
22 changes: 22 additions & 0 deletions sentry/api/sentry.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -987,6 +987,7 @@ public abstract interface class io/sentry/IScopesStorage {
}

public abstract interface class io/sentry/ISentryClient {
public abstract fun captureBatchedLogEvents (Lio/sentry/SentryLogEvents;)V
public abstract fun captureCheckIn (Lio/sentry/CheckIn;Lio/sentry/IScope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId;
public fun captureEnvelope (Lio/sentry/SentryEnvelope;)Lio/sentry/protocol/SentryId;
public abstract fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId;
Expand Down Expand Up @@ -2718,6 +2719,7 @@ public final class io/sentry/SentryBaseEvent$Serializer {

public final class io/sentry/SentryClient : io/sentry/ISentryClient {
public fun <init> (Lio/sentry/SentryOptions;)V
public fun captureBatchedLogEvents (Lio/sentry/SentryLogEvents;)V
public fun captureCheckIn (Lio/sentry/CheckIn;Lio/sentry/IScope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId;
public fun captureEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId;
public fun captureEvent (Lio/sentry/SentryEvent;Lio/sentry/IScope;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId;
Expand Down Expand Up @@ -3071,6 +3073,7 @@ public final class io/sentry/SentryLogEventAttributeValue$JsonKeys {

public final class io/sentry/SentryLogEvents : io/sentry/JsonSerializable, io/sentry/JsonUnknown {
public fun <init> (Ljava/util/List;)V
public fun getItems ()Ljava/util/List;
public fun getUnknown ()Ljava/util/Map;
public fun serialize (Lio/sentry/ObjectWriter;Lio/sentry/ILogger;)V
public fun setUnknown (Ljava/util/Map;)V
Expand Down Expand Up @@ -4666,6 +4669,11 @@ public abstract interface class io/sentry/logger/ILoggerApi {
public abstract fun warn (Ljava/lang/String;[Ljava/lang/Object;)V
}

public abstract interface class io/sentry/logger/ILoggerBatchProcessor {
public abstract fun add (Lio/sentry/SentryLogEvent;)V
public abstract fun close (Z)V
}

public final class io/sentry/logger/LoggerApi : io/sentry/logger/ILoggerApi {
public fun <init> (Lio/sentry/Scopes;)V
public fun debug (Ljava/lang/String;[Ljava/lang/Object;)V
Expand All @@ -4678,6 +4686,14 @@ public final class io/sentry/logger/LoggerApi : io/sentry/logger/ILoggerApi {
public fun warn (Ljava/lang/String;[Ljava/lang/Object;)V
}

public final class io/sentry/logger/LoggerBatchProcessor : io/sentry/logger/ILoggerBatchProcessor {
public static final field FLUSH_AFTER_MS I
public static final field MAX_BATCH_SIZE I
public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ISentryClient;)V
public fun add (Lio/sentry/SentryLogEvent;)V
public fun close (Z)V
}

public final class io/sentry/logger/NoOpLoggerApi : io/sentry/logger/ILoggerApi {
public fun debug (Ljava/lang/String;[Ljava/lang/Object;)V
public fun error (Ljava/lang/String;[Ljava/lang/Object;)V
Expand All @@ -4690,6 +4706,12 @@ public final class io/sentry/logger/NoOpLoggerApi : io/sentry/logger/ILoggerApi
public fun warn (Ljava/lang/String;[Ljava/lang/Object;)V
}

public final class io/sentry/logger/NoOpLoggerBatchProcessor : io/sentry/logger/ILoggerBatchProcessor {
public fun add (Lio/sentry/SentryLogEvent;)V
public fun close (Z)V
public static fun getInstance ()Lio/sentry/logger/NoOpLoggerBatchProcessor;
}

public final class io/sentry/opentelemetry/OpenTelemetryUtil {
public fun <init> ()V
public static fun applyIgnoredSpanOrigins (Lio/sentry/SentryOptions;)V
Expand Down
3 changes: 3 additions & 0 deletions sentry/src/main/java/io/sentry/ISentryClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,9 @@ SentryId captureProfileChunk(
@ApiStatus.Experimental
void captureLog(@NotNull SentryLogEvent logEvent, @Nullable IScope scope, @Nullable Hint hint);

@ApiStatus.Experimental
void captureBatchedLogEvents(@NotNull SentryLogEvents logEvents);

@ApiStatus.Internal
@Nullable
RateLimiter getRateLimiter();
Expand Down
6 changes: 6 additions & 0 deletions sentry/src/main/java/io/sentry/NoOpSentryClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ public void captureLog(
// do nothing
}

@ApiStatus.Experimental
@Override
public void captureBatchedLogEvents(@NotNull SentryLogEvents logEvents) {
// do nothing
}

@Override
public @Nullable RateLimiter getRateLimiter() {
return null;
Expand Down
53 changes: 32 additions & 21 deletions sentry/src/main/java/io/sentry/SentryClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
import io.sentry.hints.Backfillable;
import io.sentry.hints.DiskFlushNotification;
import io.sentry.hints.TransactionEnd;
import io.sentry.logger.ILoggerBatchProcessor;
import io.sentry.logger.LoggerBatchProcessor;
import io.sentry.logger.NoOpLoggerBatchProcessor;
import io.sentry.protocol.Contexts;
import io.sentry.protocol.DebugMeta;
import io.sentry.protocol.SentryId;
Expand All @@ -16,7 +19,6 @@
import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
Expand All @@ -36,6 +38,7 @@ public final class SentryClient implements ISentryClient {
private final @NotNull SentryOptions options;
private final @NotNull ITransport transport;
private final @NotNull SortBreadcrumbsByDate sortBreadcrumbsByDate = new SortBreadcrumbsByDate();
private final @NotNull ILoggerBatchProcessor loggerBatchProcessor;

@Override
public boolean isEnabled() {
Expand All @@ -55,6 +58,11 @@ public SentryClient(final @NotNull SentryOptions options) {

final RequestDetailsResolver requestDetailsResolver = new RequestDetailsResolver(options);
transport = transportFactory.create(options, requestDetailsResolver.resolve());
if (options.getExperimental().getLogs().isEnabled()) {
loggerBatchProcessor = new LoggerBatchProcessor(options, this);
} else {
loggerBatchProcessor = NoOpLoggerBatchProcessor.getInstance();
}
}

private boolean shouldApplyScopeData(
Expand Down Expand Up @@ -625,16 +633,15 @@ public void captureUserFeedback(final @NotNull UserFeedback userFeedback) {
return new SentryEnvelope(envelopeHeader, envelopeItems);
}

private @NotNull SentryEnvelope buildEnvelope(
final @NotNull SentryLogEvents logEvents, final @Nullable TraceContext traceContext) {
private @NotNull SentryEnvelope buildEnvelope(final @NotNull SentryLogEvents logEvents) {
final List<SentryEnvelopeItem> envelopeItems = new ArrayList<>();

final SentryEnvelopeItem logItem =
SentryEnvelopeItem.fromLogs(options.getSerializer(), logEvents);
envelopeItems.add(logItem);

final SentryEnvelopeHeader envelopeHeader =
new SentryEnvelopeHeader(null, options.getSdkVersion(), traceContext);
new SentryEnvelopeHeader(null, options.getSdkVersion(), null);

return new SentryEnvelope(envelopeHeader, envelopeItems);
}
Expand Down Expand Up @@ -1018,17 +1025,17 @@ public void captureLog(
hint = new Hint();
}

@Nullable TraceContext traceContext = null;
if (scope != null) {
final @Nullable ITransaction transaction = scope.getTransaction();
if (transaction != null) {
traceContext = transaction.traceContext();
} else {
final @NotNull PropagationContext propagationContext =
TracingUtils.maybeUpdateBaggage(scope, options);
traceContext = propagationContext.traceContext();
}
}
// @Nullable TraceContext traceContext = null;
Copy link
Member Author

Choose a reason for hiding this comment

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

(maybe) Updating baggage should be done in LoggerApi to freeze it on the first event going out.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, I don't fully understand when it should be done.

// if (scope != null) {
// final @Nullable ITransaction transaction = scope.getTransaction();
// if (transaction != null) {
// traceContext = transaction.traceContext();
// } else {
// final @NotNull PropagationContext propagationContext =
// TracingUtils.maybeUpdateBaggage(scope, options);
// traceContext = propagationContext.traceContext();
// }
// }

if (logEvent != null) {
logEvent = executeBeforeSendLog(logEvent, hint);
Expand All @@ -1040,15 +1047,18 @@ public void captureLog(
.recordLostEvent(DiscardReason.BEFORE_SEND, DataCategory.LogItem);
return;
}

loggerBatchProcessor.add(logEvent);
}

try {
final @NotNull SentryEnvelope envelope =
buildEnvelope(new SentryLogEvents(Arrays.asList(logEvent)), traceContext);
hint.clear();
Copy link
Member

Choose a reason for hiding this comment

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

Before we were passing the hint to sendEnvelope, now we always use null as the hint.
Does it make any difference?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it might have some effect especially for Android but I haven't found a good way to pass the hint through the batch processor unless we hold on to it until it's been sent and even then which Hint should we use if there's 100 log events being batched together?

}

hint.clear();
// TODO buffer
sendEnvelope(envelope, hint);
@Override
public void captureBatchedLogEvents(final @NotNull SentryLogEvents logEvents) {
try {
final @NotNull SentryEnvelope envelope = buildEnvelope(logEvents);
sendEnvelope(envelope, null);
} catch (IOException e) {
options.getLogger().log(SentryLevel.WARNING, e, "Capturing log failed.");
}
Expand Down Expand Up @@ -1307,6 +1317,7 @@ public void close(final boolean isRestarting) {
options.getLogger().log(SentryLevel.INFO, "Closing SentryClient.");
try {
flush(isRestarting ? 0 : options.getShutdownTimeoutMillis());
loggerBatchProcessor.close(isRestarting);
transport.close(isRestarting);
} catch (IOException e) {
options
Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/SentryEnvelopeItem.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ public static SentryEnvelopeItem fromLogs(
null,
null,
null,
1);
logEvents.getItems().size());

// avoid method refs on Android due to some issues with older AGP setups
// noinspection Convert2MethodRef
Expand Down
4 changes: 4 additions & 0 deletions sentry/src/main/java/io/sentry/SentryLogEvents.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ public SentryLogEvents(final @NotNull List<SentryLogEvent> items) {
this.items = items;
}

public @NotNull List<SentryLogEvent> getItems() {
return items;
}

// region json
public static final class JsonKeys {
public static final String ITEMS = "items";
Expand Down
10 changes: 10 additions & 0 deletions sentry/src/main/java/io/sentry/logger/ILoggerBatchProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package io.sentry.logger;

import io.sentry.SentryLogEvent;
import org.jetbrains.annotations.NotNull;

public interface ILoggerBatchProcessor {
void add(@NotNull SentryLogEvent event);

void close(boolean isRestarting);
}
4 changes: 2 additions & 2 deletions sentry/src/main/java/io/sentry/logger/LoggerApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public LoggerApi(final @NotNull Scopes scopes) {

@Override
public void trace(final @Nullable String message, final @Nullable Object... args) {
// TODO SentryLevel.TRACE does not exists yet
// log(SentryLevel.TRACE, message, args);
// TODO SentryLevel.TRACE does not exists yet so we just report it as DEBUG for now
log(SentryLevel.DEBUG, message, args);
Copy link
Member Author

Choose a reason for hiding this comment

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

I opted for this as a quick fix to get trace() working. We can have a proper fix later but we'll have to check what consequences this has for the rest of the SDK since the enum is used in multiple places already.

}

@Override
Expand Down
101 changes: 101 additions & 0 deletions sentry/src/main/java/io/sentry/logger/LoggerBatchProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
package io.sentry.logger;

import io.sentry.ISentryClient;
import io.sentry.ISentryLifecycleToken;
import io.sentry.SentryLogEvent;
import io.sentry.SentryLogEvents;
import io.sentry.SentryOptions;
import io.sentry.util.AutoClosableReentrantLock;
import java.util.ArrayList;
import java.util.List;
import java.util.Queue;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.Future;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

public final class LoggerBatchProcessor implements ILoggerBatchProcessor {

public static final int FLUSH_AFTER_MS = 5000;
public static final int MAX_BATCH_SIZE = 100;

private final @NotNull SentryOptions options;
private final @NotNull ISentryClient client;
private final @NotNull Queue<SentryLogEvent> queue;
private volatile @Nullable Future<?> scheduledFlush;
private static final @NotNull AutoClosableReentrantLock scheduleLock =
new AutoClosableReentrantLock();

public LoggerBatchProcessor(
final @NotNull SentryOptions options, final @NotNull ISentryClient client) {
this.options = options;
this.client = client;
this.queue = new ConcurrentLinkedQueue<>();
}

@Override
public void add(final @NotNull SentryLogEvent logEvent) {
queue.offer(logEvent);
maybeSchedule(false, false);
Copy link
Member Author

Choose a reason for hiding this comment

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

This means we're locking on every log event. I'll try to optimize this. Ideas:

  1. Have a bool hasScheduled or similar which we check first and avoid locking if already true
  2. Check the futures state first (not sure about performance of that)
  3. Just send a signal and have a background thread pick it up by periodically polling for it

}

@Override
public void close(final boolean isRestarting) {
if (isRestarting) {
maybeSchedule(true, true);
} else {
while (!queue.isEmpty()) {
flushBatch();
}
}
}

private void maybeSchedule(boolean forceSchedule, boolean immediately) {
try (final @NotNull ISentryLifecycleToken ignored = scheduleLock.acquire()) {
final @Nullable Future<?> latestScheduledFlush = scheduledFlush;
if (forceSchedule
|| latestScheduledFlush == null
|| latestScheduledFlush.isDone()
|| latestScheduledFlush.isCancelled()) {
final int flushAfterMs = immediately ? 0 : FLUSH_AFTER_MS;
scheduledFlush = options.getExecutorService().schedule(new BatchRunnable(), flushAfterMs);
}
}
}

private void flush() {
flushInternal();
try (final @NotNull ISentryLifecycleToken ignored = scheduleLock.acquire()) {
if (!queue.isEmpty()) {
maybeSchedule(true, false);
}
}
}

private void flushInternal() {
flushBatch();
if (queue.size() >= MAX_BATCH_SIZE) {
flushInternal();
}
Comment on lines +76 to +79
Copy link
Member

Choose a reason for hiding this comment

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

There is no need for recursion, personally I would prefer

Suggested change
flushBatch();
if (queue.size() >= MAX_BATCH_SIZE) {
flushInternal();
}
while (queue.size() >= MAX_BATCH_SIZE) {
flushBatch();
}

Copy link
Member Author

@adinauer adinauer May 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This would mean we send out at most two batches at a time. The way it's implemented atm sends out as many full batches as are available and then maybe reschedules (depending on queue being empty).

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I missed the while, sorry

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah we can change it.

Copy link
Member Author

Choose a reason for hiding this comment

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

But needs to be do ... while so we send out before having a full batch

}

private void flushBatch() {
final @NotNull List<SentryLogEvent> logEvents = new ArrayList<>(MAX_BATCH_SIZE);
do {
final @Nullable SentryLogEvent logEvent = queue.poll();
if (logEvent != null) {
logEvents.add(logEvent);
}
} while (!queue.isEmpty() && logEvents.size() < MAX_BATCH_SIZE);

client.captureBatchedLogEvents(new SentryLogEvents(logEvents));
}

private class BatchRunnable implements Runnable {

@Override
public void run() {
flush();
}
}
}
27 changes: 27 additions & 0 deletions sentry/src/main/java/io/sentry/logger/NoOpLoggerBatchProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.sentry.logger;

import io.sentry.SentryLogEvent;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;

@ApiStatus.Experimental
public final class NoOpLoggerBatchProcessor implements ILoggerBatchProcessor {

private static final NoOpLoggerBatchProcessor instance = new NoOpLoggerBatchProcessor();

private NoOpLoggerBatchProcessor() {}

public static NoOpLoggerBatchProcessor getInstance() {
return instance;
}

@Override
public void add(@NotNull SentryLogEvent event) {
// do nothing
}

@Override
public void close(final boolean isRestarting) {
Copy link
Member

Choose a reason for hiding this comment

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

just wondering, when is isRestarting intended to be used?

Copy link
Member Author

Choose a reason for hiding this comment

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

When the SDK is being re-initialized. LMK if you need more explanation.

// do nothing
}
}
Loading