Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class LogRecordToDiskExporter implements LogRecordExporter {
* @return A new LogRecordToDiskExporter instance.
* @throws IOException if the delegate ToDiskExporter could not be created.
*/
@SuppressWarnings("deprecation")
public static LogRecordToDiskExporter create(
LogRecordExporter delegate, StorageConfiguration config) throws IOException {
ToDiskExporter<LogRecordData> toDisk =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ public class MetricToDiskExporter implements MetricExporter {
* @return A new MetricToDiskExporter instance.
* @throws IOException if the delegate ToDiskExporter could not be created.
*/
@SuppressWarnings("deprecation")
public static MetricToDiskExporter create(MetricExporter delegate, StorageConfiguration config)
throws IOException {
ToDiskExporter<MetricData> toDisk =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.opentelemetry.contrib.disk.buffering.config.StorageConfiguration;
import io.opentelemetry.contrib.disk.buffering.internal.exporter.ToDiskExporter;
import io.opentelemetry.contrib.disk.buffering.internal.serialization.serializers.SignalSerializer;
import io.opentelemetry.contrib.disk.buffering.internal.storage.Storage;
import io.opentelemetry.contrib.disk.buffering.internal.utils.SignalTypes;
import io.opentelemetry.sdk.common.CompletableResultCode;
import io.opentelemetry.sdk.trace.data.SpanData;
Expand All @@ -31,6 +32,7 @@ public class SpanToDiskExporter implements SpanExporter {
* @return A new SpanToDiskExporter instance.
* @throws IOException if the delegate ToDiskExporter could not be created.
*/
@Deprecated
public static SpanToDiskExporter create(SpanExporter delegate, StorageConfiguration config)
Copy link
Contributor

Choose a reason for hiding this comment

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

We've previously added breaking changes without adding deprecations, because this tool isn't stable yet, if you think it's worth doing so, I'm fine with it. However, I think it should be fine to just change the signature here to replace StorageConfiguration by Storage for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense 👍 I think the getBody() one was causing issues before because it's defined upstream (maybe those are sorted now), but apart from that, I don't see why we should keep deprecated methods for now.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

throws IOException {
ToDiskExporter<SpanData> toDisk =
Expand All @@ -43,6 +45,23 @@ public static SpanToDiskExporter create(SpanExporter delegate, StorageConfigurat
return new SpanToDiskExporter(toDisk);
}

/**
* Creates a new SpanToDiskExporter that will buffer Span telemetry on disk storage.
*
* @param delegate - The SpanExporter to delegate to if disk writing fails.
* @return A new SpanToDiskExporter instance.
* @throws IOException if the delegate ToDiskExporter could not be created.
*/
public static SpanToDiskExporter create(SpanExporter delegate, Storage storage)
throws IOException {
ToDiskExporter<SpanData> toDisk =
ToDiskExporter.<SpanData>builder(storage)
.setSerializer(SignalSerializer.ofSpans())
.setExportFunction(delegate::export)
.build();
return new SpanToDiskExporter(toDisk);
}

// Visible for testing
SpanToDiskExporter(ToDiskExporter<SpanData> delegate) {
this.delegate = delegate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,15 @@ public class ToDiskExporter<EXPORT_DATA> {
this.logger = DebugLogger.wrap(Logger.getLogger(ToDiskExporter.class.getName()), debugEnabled);
}

@Deprecated
public static <T> ToDiskExporterBuilder<T> builder() {
return new ToDiskExporterBuilder<>();
}

public static <T> ToDiskExporterBuilder<T> builder(Storage storage) {
return new ToDiskExporterBuilder<>(storage);
}

public CompletableResultCode export(Collection<EXPORT_DATA> data) {
logger.log("Intercepting exporter batch.", Level.FINER);
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,30 @@
import java.io.IOException;
import java.util.Collection;
import java.util.function.Function;
import javax.annotation.Nullable;

public final class ToDiskExporterBuilder<T> {

private SignalSerializer<T> serializer = ts -> new byte[0];

private final StorageBuilder storageBuilder = Storage.builder();
@Deprecated private final StorageBuilder storageBuilder = Storage.builder();

@Nullable private Storage storage = null;

private Function<Collection<T>, CompletableResultCode> exportFunction =
x -> CompletableResultCode.ofFailure();
private boolean debugEnabled = false;

@Deprecated
ToDiskExporterBuilder() {}

ToDiskExporterBuilder(Storage storage) {
if (storage == null) {
throw new NullPointerException("Storage cannot be null");
}
this.storage = storage;
}

@CanIgnoreReturnValue
public ToDiskExporterBuilder<T> enableDebug() {
return setDebugEnabled(true);
Expand All @@ -39,25 +50,34 @@ public ToDiskExporterBuilder<T> setDebugEnabled(boolean debugEnabled) {
return this;
}

@Deprecated
@CanIgnoreReturnValue
public ToDiskExporterBuilder<T> setFolderName(String folderName) {
storageBuilder.setFolderName(folderName);
return this;
}

@Deprecated
@CanIgnoreReturnValue
public ToDiskExporterBuilder<T> setStorageConfiguration(StorageConfiguration configuration) {
validateConfiguration(configuration);
storageBuilder.setStorageConfiguration(configuration);
return this;
}

@Deprecated
@CanIgnoreReturnValue
public ToDiskExporterBuilder<T> setStorageClock(Clock clock) {
storageBuilder.setStorageClock(clock);
return this;
}

@CanIgnoreReturnValue
public ToDiskExporterBuilder<T> setStorage(Storage storage) {
this.storage = storage;
return this;
}

@CanIgnoreReturnValue
public ToDiskExporterBuilder<T> setSerializer(SignalSerializer<T> serializer) {
this.serializer = serializer;
Expand All @@ -72,7 +92,7 @@ public ToDiskExporterBuilder<T> setExportFunction(
}

public ToDiskExporter<T> build() throws IOException {
Storage storage = storageBuilder.build();
Storage storage = this.storage != null ? this.storage : storageBuilder.build();
return new ToDiskExporter<>(serializer, exportFunction, storage, debugEnabled);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import io.opentelemetry.contrib.disk.buffering.internal.exporter.ToDiskExporter;
import io.opentelemetry.contrib.disk.buffering.internal.serialization.deserializers.SignalDeserializer;
import io.opentelemetry.contrib.disk.buffering.internal.serialization.serializers.SignalSerializer;
import io.opentelemetry.contrib.disk.buffering.internal.storage.Storage;
import io.opentelemetry.contrib.disk.buffering.internal.utils.SignalTypes;
import io.opentelemetry.sdk.common.Clock;
import io.opentelemetry.sdk.common.CompletableResultCode;
Expand Down Expand Up @@ -67,11 +68,18 @@ public class IntegrationTest {
private static final long INITIAL_TIME_IN_MILLIS = 1000;
private static final long NOW_NANOS = MILLISECONDS.toNanos(INITIAL_TIME_IN_MILLIS);
private StorageConfiguration storageConfig;
private Storage storage;

@BeforeEach
void setUp() throws IOException {
storageConfig = StorageConfiguration.getDefault(rootDir);
clock = mock();
storageConfig = StorageConfiguration.getDefault(rootDir);
storage =
Storage.builder()
.setFolderName(SignalTypes.spans.name())
.setStorageConfiguration(storageConfig)
.setStorageClock(clock)
.build();

when(clock.now()).thenReturn(NOW_NANOS);

Expand Down Expand Up @@ -102,12 +110,9 @@ void setUp() throws IOException {
private <T> ToDiskExporter<T> buildToDiskExporter(
SignalSerializer<T> serializer, Function<Collection<T>, CompletableResultCode> exporter)
throws IOException {
return ToDiskExporter.<T>builder()
.setFolderName(SignalTypes.spans.name())
.setStorageConfiguration(storageConfig)
return ToDiskExporter.<T>builder(storage)
.setSerializer(serializer)
.setExportFunction(exporter)
.setStorageClock(clock)
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

class ToDiskExporterBuilderTest {

@SuppressWarnings("deprecation") // todo
@Test
void whenMinFileReadIsNotGraterThanMaxFileWrite_throwException() {
StorageConfiguration invalidConfig =
Expand Down