Skip to content

Commit

Permalink
Address the comments from the reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
@ansman
ansman committed Dec 11, 2024
1 parent 52018e3 commit 6bcdec0
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 105 deletions.
64 changes: 36 additions & 28 deletions core/src/main/java/io/opentelemetry/android/OpenTelemetryRumBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,11 @@ public final class OpenTelemetryRumBuilder {

private Resource resource;

private final Object lock = new Object();
private boolean isBuilt = false;

// Writes guarded by "lock"
@Nullable private volatile ServiceManager serviceManager;
@Nullable private ServiceManager serviceManager;

// Writes guarded by "lock"
@Nullable private volatile ExportScheduleHandler exportScheduleHandler;
@Nullable private ExportScheduleHandler exportScheduleHandler;

private static TextMapPropagator buildDefaultPropagator() {
return TextMapPropagator.composite(
Expand All @@ -129,6 +127,7 @@ public static OpenTelemetryRumBuilder create(Application application, OtelRumCon
* @return {@code this}
*/
public OpenTelemetryRumBuilder setResource(Resource resource) {
checkNotBuilt();
this.resource = resource;
return this;
}
Expand All @@ -141,6 +140,7 @@ public OpenTelemetryRumBuilder setResource(Resource resource) {
* @return {@code this}
*/
public OpenTelemetryRumBuilder mergeResource(Resource resource) {
checkNotBuilt();
this.resource = this.resource.merge(resource);
return this;
}
Expand All @@ -161,6 +161,7 @@ public OpenTelemetryRumBuilder mergeResource(Resource resource) {
public OpenTelemetryRumBuilder addTracerProviderCustomizer(
BiFunction<SdkTracerProviderBuilder, Application, SdkTracerProviderBuilder>
customizer) {
checkNotBuilt();
tracerProviderCustomizers.add(customizer);
return this;
}
Expand All @@ -180,6 +181,7 @@ public OpenTelemetryRumBuilder addTracerProviderCustomizer(
*/
public OpenTelemetryRumBuilder addMeterProviderCustomizer(
BiFunction<SdkMeterProviderBuilder, Application, SdkMeterProviderBuilder> customizer) {
checkNotBuilt();
meterProviderCustomizers.add(customizer);
return this;
}
Expand All @@ -200,6 +202,7 @@ public OpenTelemetryRumBuilder addMeterProviderCustomizer(
public OpenTelemetryRumBuilder addLoggerProviderCustomizer(
BiFunction<SdkLoggerProviderBuilder, Application, SdkLoggerProviderBuilder>
customizer) {
checkNotBuilt();
loggerProviderCustomizers.add(customizer);
return this;
}
Expand All @@ -211,6 +214,7 @@ public OpenTelemetryRumBuilder addLoggerProviderCustomizer(
*/
public OpenTelemetryRumBuilder addInstrumentation(AndroidInstrumentation instrumentation) {
instrumentations.add(instrumentation);
checkNotBuilt();
return this;
}

Expand All @@ -225,6 +229,7 @@ public OpenTelemetryRumBuilder addInstrumentation(AndroidInstrumentation instrum
public OpenTelemetryRumBuilder addPropagatorCustomizer(
Function<? super TextMapPropagator, ? extends TextMapPropagator> propagatorCustomizer) {
requireNonNull(propagatorCustomizer, "propagatorCustomizer");
checkNotBuilt();
Function<? super TextMapPropagator, ? extends TextMapPropagator> existing =
this.propagatorCustomizer;
this.propagatorCustomizer =
Expand All @@ -244,6 +249,7 @@ public OpenTelemetryRumBuilder addPropagatorCustomizer(
public OpenTelemetryRumBuilder addSpanExporterCustomizer(
Function<? super SpanExporter, ? extends SpanExporter> spanExporterCustomizer) {
requireNonNull(spanExporterCustomizer, "spanExporterCustomizer");
checkNotBuilt();
Function<? super SpanExporter, ? extends SpanExporter> existing =
this.spanExporterCustomizer;
this.spanExporterCustomizer =
Expand All @@ -263,6 +269,7 @@ public OpenTelemetryRumBuilder addSpanExporterCustomizer(
public OpenTelemetryRumBuilder addLogRecordExporterCustomizer(
Function<? super LogRecordExporter, ? extends LogRecordExporter>
logRecordExporterCustomizer) {
checkNotBuilt();
Function<? super LogRecordExporter, ? extends LogRecordExporter> existing =
this.logRecordExporterCustomizer;
this.logRecordExporterCustomizer =
Expand All @@ -283,6 +290,10 @@ public OpenTelemetryRumBuilder addLogRecordExporterCustomizer(
* @return A new {@link OpenTelemetryRum} instance.
*/
public OpenTelemetryRum build() {
if (isBuilt) {
throw new IllegalStateException("You cannot call build multiple times");
}
isBuilt = true;
InitializationEvents initializationEvents = InitializationEvents.get();
applyConfiguration(initializationEvents);

Expand Down Expand Up @@ -373,21 +384,16 @@ private void initializeExporters(
@NonNull
private ServiceManager getServiceManager() {
if (serviceManager == null) {
synchronized (lock) {
if (serviceManager == null) {
serviceManager = ServiceManagerImpl.Companion.create(application);
}
}
serviceManager = ServiceManagerImpl.Companion.create(application);
}
// This can never be null since we never write `null` to it
return requireNonNull(serviceManager);
}

public OpenTelemetryRumBuilder setServiceManager(@NonNull ServiceManager serviceManager) {
requireNonNull(serviceManager, "serviceManager cannot be null");
synchronized (lock) {
this.serviceManager = serviceManager;
}
checkNotBuilt();
this.serviceManager = serviceManager;
return this;
}

Expand All @@ -398,9 +404,8 @@ public OpenTelemetryRumBuilder setServiceManager(@NonNull ServiceManager service
public OpenTelemetryRumBuilder setExportScheduleHandler(
@NonNull ExportScheduleHandler exportScheduleHandler) {
requireNonNull(exportScheduleHandler, "exportScheduleHandler cannot be null");
synchronized (lock) {
this.exportScheduleHandler = exportScheduleHandler;
}
checkNotBuilt();
this.exportScheduleHandler = exportScheduleHandler;
return this;
}

Expand All @@ -423,18 +428,14 @@ private StorageConfiguration createStorageConfiguration() throws IOException {

private void scheduleDiskTelemetryReader(@Nullable SignalFromDiskExporter signalExporter) {
if (exportScheduleHandler == null) {
synchronized (lock) {
if (exportScheduleHandler == null) {
ServiceManager serviceManager = getServiceManager();
// TODO: Is it safe to get the work service yet here? If so, we can
// avoid all this lazy supplier stuff....
Function0<PeriodicWorkService> getWorkService =
serviceManager::getPeriodicWorkService;
exportScheduleHandler =
new DefaultExportScheduleHandler(
new DefaultExportScheduler(getWorkService), getWorkService);
}
}
ServiceManager serviceManager = getServiceManager();
// TODO: Is it safe to get the work service yet here? If so, we can
// avoid all this lazy supplier stuff....
Function0<PeriodicWorkService> getWorkService =
serviceManager::getPeriodicWorkService;
exportScheduleHandler =
new DefaultExportScheduleHandler(
new DefaultExportScheduler(getWorkService), getWorkService);
}

final ExportScheduleHandler exportScheduleHandler =
Expand All @@ -461,6 +462,7 @@ private void scheduleDiskTelemetryReader(@Nullable SignalFromDiskExporter signal
* @return this
*/
public OpenTelemetryRumBuilder addOtelSdkReadyListener(Consumer<OpenTelemetrySdk> callback) {
checkNotBuilt();
otelSdkReadyListeners.add(callback);
return this;
}
Expand Down Expand Up @@ -574,4 +576,10 @@ private ContextPropagators buildFinalPropagators() {
TextMapPropagator defaultPropagator = buildDefaultPropagator();
return ContextPropagators.create(propagatorCustomizer.apply(defaultPropagator));
}

private void checkNotBuilt() {
if (isBuilt) {
throw new IllegalStateException("This method cannot be called after calling build");
}
}
}
27 changes: 15 additions & 12 deletions core/src/main/java/io/opentelemetry/android/export/BufferDelegatingLogExporter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package io.opentelemetry.android.export
import io.opentelemetry.sdk.common.CompletableResultCode
import io.opentelemetry.sdk.logs.data.LogRecordData
import io.opentelemetry.sdk.logs.export.LogRecordExporter
import io.opentelemetry.sdk.trace.export.SpanExporter

/**
* An in-memory buffer delegating log exporter that buffers log records in memory until a delegate is set.
Expand All @@ -17,19 +18,21 @@ import io.opentelemetry.sdk.logs.export.LogRecordExporter
*/
internal class BufferDelegatingLogExporter(
maxBufferedLogs: Int = 5_000,
) : BufferedDelegatingExporter<LogRecordData, LogRecordExporter>(bufferedSignals = maxBufferedLogs),
LogRecordExporter {
override fun exportToDelegate(
delegate: LogRecordExporter,
data: Collection<LogRecordData>,
): CompletableResultCode = delegate.export(data)
) : LogRecordExporter {
private val delegatingExporter = BufferedDelegatingExporter<LogRecordExporter, LogRecordData>(
export = LogRecordExporter::export,
shutdown = LogRecordExporter::shutdown,
flush = LogRecordExporter::flush,
maxBufferedData = maxBufferedLogs
)

override fun shutdownDelegate(delegate: LogRecordExporter): CompletableResultCode = delegate.shutdown()
fun setDelegate(delegate: LogRecordExporter) {
delegatingExporter.setDelegate(delegate)
}

override fun export(logs: Collection<LogRecordData>): CompletableResultCode = bufferOrDelegate(logs)
override fun export(logs: Collection<LogRecordData>): CompletableResultCode =
delegatingExporter.export(logs)

override fun flush(): CompletableResultCode =
withDelegateOrNull { delegate ->
delegate?.flush() ?: CompletableResultCode.ofSuccess()
}
override fun flush(): CompletableResultCode = delegatingExporter.flush()
override fun shutdown(): CompletableResultCode = delegatingExporter.shutdown()
}
27 changes: 15 additions & 12 deletions core/src/main/java/io/opentelemetry/android/export/BufferDelegatingSpanExporter.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package io.opentelemetry.android.export

import io.opentelemetry.sdk.common.CompletableResultCode
import io.opentelemetry.sdk.logs.data.LogRecordData
import io.opentelemetry.sdk.trace.data.SpanData
import io.opentelemetry.sdk.trace.export.SpanExporter

Expand All @@ -17,19 +18,21 @@ import io.opentelemetry.sdk.trace.export.SpanExporter
*/
internal class BufferDelegatingSpanExporter(
maxBufferedSpans: Int = 5_000,
) : BufferedDelegatingExporter<SpanData, SpanExporter>(bufferedSignals = maxBufferedSpans),
SpanExporter {
override fun exportToDelegate(
delegate: SpanExporter,
data: Collection<SpanData>,
): CompletableResultCode = delegate.export(data)
) : SpanExporter {
private val delegatingExporter = BufferedDelegatingExporter<SpanExporter, SpanData>(
export = SpanExporter::export,
shutdown = SpanExporter::shutdown,
flush = SpanExporter::flush,
maxBufferedData = maxBufferedSpans
)

override fun shutdownDelegate(delegate: SpanExporter): CompletableResultCode = delegate.shutdown()
fun setDelegate(delegate: SpanExporter) {
delegatingExporter.setDelegate(delegate)
}

override fun export(spans: Collection<SpanData>): CompletableResultCode = bufferOrDelegate(spans)
override fun export(spans: MutableCollection<SpanData>): CompletableResultCode =
delegatingExporter.export(spans)

override fun flush(): CompletableResultCode =
withDelegateOrNull { delegate ->
delegate?.flush() ?: CompletableResultCode.ofSuccess()
}
override fun flush(): CompletableResultCode = delegatingExporter.flush()
override fun shutdown(): CompletableResultCode = delegatingExporter.shutdown()
}
Loading

0 comments on commit 6bcdec0

Please sign in to comment.