PR comments addressed

Author: onurkybsi

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LogRecordProcessor.java

@@ -9,7 +9,6 @@
 import io.opentelemetry.api.logs.Logger;
 import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.common.CompletableResultCode;
-import io.opentelemetry.sdk.logs.export.BatchLogRecordProcessor;
 import java.io.Closeable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -37,30 +36,18 @@ static LogRecordProcessor composite(LogRecordProcessor... processors) {
   /**
    * Returns a {@link LogRecordProcessor} which simply delegates to all processing to the {@code
    * processors} in order.
-   *
-   * <p>Note that if there are {@link BatchLogRecordProcessor} including, they are enforced to end
-   * of the processor pipeline.
    */
   static LogRecordProcessor composite(Iterable<LogRecordProcessor> processors) {
     List<LogRecordProcessor> processorList = new ArrayList<>();
-    List<BatchLogRecordProcessor> batchProcessors = new ArrayList<>();
     for (LogRecordProcessor processor : processors) {
-      if (!BatchLogRecordProcessor.class.equals(processor.getClass())) {
-        processorList.add(processor);
-      } else {
-        batchProcessors.add((BatchLogRecordProcessor) processor);
-      }
+      processorList.add(processor);
     }
-    processorList.addAll(batchProcessors);
-
     if (processorList.isEmpty()) {
       return NoopLogRecordProcessor.getInstance();
     }
-
     if (processorList.size() == 1) {
       return processorList.get(0);
     }
-
     return MultiLogRecordProcessor.create(processorList);
   }
 

sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProviderBuilder.java

@@ -96,6 +96,24 @@ public SdkLoggerProviderBuilder addLogRecordProcessor(LogRecordProcessor process
     return this;
   }
 
+  /**
+   * Add a log processor as first. {@link LogRecordProcessor#onEmit(Context, ReadWriteLogRecord)}
+   * will be called each time a log is emitted by {@link Logger} instances obtained from the {@link
+   * SdkLoggerProvider}.
+   *
+   * <p>Compared to {@link SdkLoggerProviderBuilder#addLogRecordProcessor(LogRecordProcessor)}, this
+   * method adds the processor to the beginning of the processor pipeline. So the {@code processor}
+   * given will be executed before than the other processors already added.
+   *
+   * @param processor the log processor
+   * @return this
+   */
+  public SdkLoggerProviderBuilder addLogRecordProcessorFirst(LogRecordProcessor processor) {
+    requireNonNull(processor, "processor");
+    logRecordProcessors.add(0, processor);
+    return this;
+  }
+
   /**
    * Assign a {@link Clock}.
    *

sdk/logs/src/test/java/io/opentelemetry/sdk/logs/SdkLoggerProviderTest.java

@@ -29,7 +29,6 @@
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.common.InstrumentationScopeInfo;
 import io.opentelemetry.sdk.logs.data.LogRecordData;
-import io.opentelemetry.sdk.logs.export.BatchLogRecordProcessor;
 import io.opentelemetry.sdk.resources.Resource;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
@@ -125,31 +124,12 @@ void builder_clockProvided() {
 
   @Test
   void builder_multipleProcessors() {
+    LogRecordProcessor firstProcessor = mock(LogRecordProcessor.class);
     assertThat(
             SdkLoggerProvider.builder()
                 .addLogRecordProcessor(logRecordProcessor)
                 .addLogRecordProcessor(logRecordProcessor)
-                .build())
-        .extracting("sharedState", as(InstanceOfAssertFactories.type(LoggerSharedState.class)))
-        .extracting(LoggerSharedState::getLogRecordProcessor)
-        .satisfies(
-            activeLogRecordProcessor -> {
-              assertThat(activeLogRecordProcessor).isInstanceOf(MultiLogRecordProcessor.class);
-              assertThat(activeLogRecordProcessor)
-                  .extracting(
-                      "logRecordProcessors",
-                      as(InstanceOfAssertFactories.list(LogRecordProcessor.class)))
-                  .hasSize(2);
-            });
-  }
-
-  @Test
-  void builder_multipleProcessorsWithBatchProcessor() {
-    assertThat(
-            SdkLoggerProvider.builder()
-                .addLogRecordProcessor(mock(BatchLogRecordProcessor.class))
-                .addLogRecordProcessor(logRecordProcessor)
-                .addLogRecordProcessor(logRecordProcessor)
+                .addLogRecordProcessorFirst(firstProcessor)
                 .build())
         .extracting("sharedState", as(InstanceOfAssertFactories.type(LoggerSharedState.class)))
         .extracting(LoggerSharedState::getLogRecordProcessor)
@@ -161,8 +141,8 @@ void builder_multipleProcessorsWithBatchProcessor() {
                       "logRecordProcessors",
                       as(InstanceOfAssertFactories.list(LogRecordProcessor.class)))
                   .hasSize(3)
-                  .last()
-                  .isInstanceOf(BatchLogRecordProcessor.class);
+                  .first()
+                  .isEqualTo(firstProcessor);
             });
   }