Rework log fields (#3837)

* Rework log fields * PR feedback
open-telemetry · Nov 10, 2021 · cb057dd · cb057dd
1 parent fc68a88
commit cb057dd
Show file tree

Hide file tree

Showing 22 changed files with 186 additions and 192 deletions.
diff --git a/.../src/test/java/io/opentelemetry/exporter/logging/otlp/OtlpJsonLoggingLogExporterTest.java b/.../src/test/java/io/opentelemetry/exporter/logging/otlp/OtlpJsonLoggingLogExporterTest.java
@@ -12,6 +12,11 @@
 
 import io.github.netmikey.logunit.api.LogCapturer;
 import io.opentelemetry.api.common.Attributes;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanContext;
+import io.opentelemetry.api.trace.TraceFlags;
+import io.opentelemetry.api.trace.TraceState;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
 import io.opentelemetry.sdk.logs.data.LogData;
 import io.opentelemetry.sdk.logs.data.LogDataBuilder;
@@ -35,26 +40,38 @@ class OtlpJsonLoggingLogExporterTest {
       LogDataBuilder.create(RESOURCE, InstrumentationLibraryInfo.create("instrumentation", "1"))
           .setName("testLog1")
           .setBody("body1")
-          .setFlags(0)
           .setSeverity(Severity.INFO)
           .setSeverityText("INFO")
-          .setSpanId("8765432112345876")
-          .setTraceId("12345678876543211234567887654322")
           .setEpoch(1631533710L, TimeUnit.MILLISECONDS)
           .setAttributes(Attributes.of(stringKey("animal"), "cat", longKey("lives"), 9L))
+          .setContext(
+              Context.root()
+                  .with(
+                      Span.wrap(
+                          SpanContext.create(
+                              "12345678876543211234567887654322",
+                              "8765432112345876",
+                              TraceFlags.getDefault(),
+                              TraceState.getDefault()))))
           .build();
 
   private static final LogData LOG2 =
       LogDataBuilder.create(RESOURCE, InstrumentationLibraryInfo.create("instrumentation2", "2"))
           .setName("testLog2")
           .setBody("body2")
-          .setFlags(0)
           .setSeverity(Severity.INFO)
           .setSeverityText("INFO")
-          .setSpanId("8765432112345875")
-          .setTraceId("12345678876543211234567887654322")
           .setEpoch(1631533710L, TimeUnit.MILLISECONDS)
           .setAttributes(Attributes.of(booleanKey("important"), true))
+          .setContext(
+              Context.root()
+                  .with(
+                      Span.wrap(
+                          SpanContext.create(
+                              "12345678876543211234567887654322",
+                              "8765432112345875",
+                              TraceFlags.getDefault(),
+                              TraceState.getDefault()))))
           .build();
 
   @RegisterExtension

diff --git a/exporters/logging/src/main/java/io/opentelemetry/exporter/logging/SystemOutLogExporter.java b/exporters/logging/src/main/java/io/opentelemetry/exporter/logging/SystemOutLogExporter.java
@@ -52,9 +52,9 @@ static void formatLog(StringBuilder stringBuilder, LogData log) {
         .append(" '")
         .append(log.getBody().asString())
         .append("' : ")
-        .append(log.getTraceId())
+        .append(log.getSpanContext().getTraceId())
         .append(" ")
-        .append(log.getSpanId())
+        .append(log.getSpanContext().getSpanId())
         .append(" [libraryInfo: ")
         .append(instrumentationLibraryInfo.getName())
         .append(":")

diff --git a/...ers/logging/src/test/java/io/opentelemetry/exporter/logging/SystemOutLogExporterTest.java b/...ers/logging/src/test/java/io/opentelemetry/exporter/logging/SystemOutLogExporterTest.java
@@ -11,11 +11,13 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.trace.SpanId;
-import io.opentelemetry.api.trace.TraceId;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanContext;
+import io.opentelemetry.api.trace.TraceFlags;
+import io.opentelemetry.api.trace.TraceState;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
-import io.opentelemetry.sdk.logs.data.Body;
 import io.opentelemetry.sdk.logs.data.LogData;
 import io.opentelemetry.sdk.logs.data.LogDataBuilder;
 import io.opentelemetry.sdk.logs.data.Severity;
@@ -54,11 +56,18 @@ private static LogData sampleLog(long timestamp) {
     return LogDataBuilder.create(
             Resource.empty(), InstrumentationLibraryInfo.create("logTest", "1.0"))
         .setAttributes(Attributes.of(stringKey("cheese"), "cheddar", longKey("amount"), 1L))
-        .setBody(Body.stringBody("message"))
+        .setBody("message")
         .setSeverity(Severity.ERROR3)
         .setEpoch(timestamp, TimeUnit.MILLISECONDS)
-        .setTraceId(TraceId.fromLongs(1, 2))
-        .setSpanId(SpanId.fromLong(3))
+        .setContext(
+            Context.root()
+                .with(
+                    Span.wrap(
+                        SpanContext.create(
+                            "00000000000000010000000000000002",
+                            "0000000000000003",
+                            TraceFlags.getDefault(),
+                            TraceState.getDefault()))))
         .build();
   }
 }
diff --git a/.../logs/src/test/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogExporterTest.java b/.../logs/src/test/java/io/opentelemetry/exporter/otlp/http/logs/OtlpHttpLogExporterTest.java
@@ -29,12 +29,10 @@
 import io.opentelemetry.proto.logs.v1.ResourceLogs;
 import io.opentelemetry.sdk.common.CompletableResultCode;
 import io.opentelemetry.sdk.common.InstrumentationLibraryInfo;
-import io.opentelemetry.sdk.logs.data.Body;
 import io.opentelemetry.sdk.logs.data.LogData;
 import io.opentelemetry.sdk.logs.data.LogDataBuilder;
 import io.opentelemetry.sdk.logs.data.Severity;
 import io.opentelemetry.sdk.resources.Resource;
-import io.opentelemetry.sdk.trace.IdGenerator;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.UncheckedIOException;
@@ -323,14 +321,11 @@ private static LogData generateFakeLog() {
             Resource.getDefault(),
             InstrumentationLibraryInfo.create("testLib", "1.0", "http://url"))
         .setName("log-name")
-        .setBody(Body.stringBody("log body"))
+        .setBody("log body")
         .setAttributes(Attributes.builder().put("key", "value").build())
         .setSeverity(Severity.INFO)
         .setSeverityText(Severity.INFO.name())
-        .setTraceId(IdGenerator.random().generateTraceId())
-        .setSpanId(IdGenerator.random().generateSpanId())
         .setEpoch(Instant.now())
-        .setFlags(0)
         .build();
   }
 }
diff --git a/.../otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/logs/LogMarshaler.java b/.../otlp/common/src/main/java/io/opentelemetry/exporter/otlp/internal/logs/LogMarshaler.java
@@ -5,6 +5,10 @@
 
 package io.opentelemetry.exporter.otlp.internal.logs;
 
+import io.opentelemetry.api.trace.SpanContext;
+import io.opentelemetry.api.trace.SpanId;
+import io.opentelemetry.api.trace.TraceFlags;
+import io.opentelemetry.api.trace.TraceId;
 import io.opentelemetry.exporter.otlp.internal.KeyValueMarshaler;
 import io.opentelemetry.exporter.otlp.internal.MarshalerUtil;
 import io.opentelemetry.exporter.otlp.internal.MarshalerWithSize;
@@ -18,14 +22,17 @@
 import javax.annotation.Nullable;
 
 final class LogMarshaler extends MarshalerWithSize {
+  private static final String INVALID_TRACE_ID = TraceId.getInvalid();
+  private static final String INVALID_SPAN_ID = SpanId.getInvalid();
+
   private final long timeUnixNano;
   private final ProtoEnumInfo severityNumber;
   private final byte[] severityText;
   private final byte[] nameUtf8;
   private final MarshalerWithSize anyValueMarshaler;
   private final KeyValueMarshaler[] attributeMarshalers;
   private final int droppedAttributesCount;
-  private final int flags;
+  private final TraceFlags traceFlags;
   @Nullable private final String traceId;
   @Nullable private final String spanId;
 
@@ -37,6 +44,7 @@ static LogMarshaler create(io.opentelemetry.sdk.logs.data.LogData logData) {
     StringAnyValueMarshaler anyValueMarshaler =
         new StringAnyValueMarshaler(MarshalerUtil.toBytes(logData.getBody().asString()));
 
+    SpanContext spanContext = logData.getSpanContext();
     return new LogMarshaler(
         logData.getEpochNanos(),
         toProtoSeverityNumber(logData.getSeverity()),
@@ -46,9 +54,9 @@ static LogMarshaler create(io.opentelemetry.sdk.logs.data.LogData logData) {
         attributeMarshalers,
         // TODO (trask) implement droppedAttributesCount in LogRecord
         0,
-        logData.getFlags(),
-        logData.getTraceId(),
-        logData.getSpanId());
+        spanContext.getTraceFlags(),
+        spanContext.getTraceId().equals(INVALID_TRACE_ID) ? null : spanContext.getTraceId(),
+        spanContext.getSpanId().equals(INVALID_SPAN_ID) ? null : spanContext.getSpanId());
   }
 
   private LogMarshaler(
@@ -59,7 +67,7 @@ private LogMarshaler(
       MarshalerWithSize anyValueMarshaler,
       KeyValueMarshaler[] attributeMarshalers,
       int droppedAttributesCount,
-      int flags,
+      TraceFlags traceFlags,
       @Nullable String traceId,
       @Nullable String spanId) {
     super(
@@ -71,13 +79,13 @@ private LogMarshaler(
             anyValueMarshaler,
             attributeMarshalers,
             droppedAttributesCount,
-            flags,
+            traceFlags,
             traceId,
             spanId));
     this.timeUnixNano = timeUnixNano;
     this.traceId = traceId;
     this.spanId = spanId;
-    this.flags = flags;
+    this.traceFlags = traceFlags;
     this.severityNumber = severityNumber;
     this.severityText = severityText;
     this.nameUtf8 = nameUtf8;
@@ -101,7 +109,7 @@ protected void writeTo(Serializer output) throws IOException {
     output.serializeRepeatedMessage(LogRecord.ATTRIBUTES, attributeMarshalers);
     output.serializeUInt32(LogRecord.DROPPED_ATTRIBUTES_COUNT, droppedAttributesCount);
 
-    output.serializeFixed32(LogRecord.FLAGS, flags);
+    output.serializeFixed32(LogRecord.FLAGS, toUnsignedInt(traceFlags.asByte()));
     output.serializeTraceId(LogRecord.TRACE_ID, traceId);
     output.serializeSpanId(LogRecord.SPAN_ID, spanId);
   }
@@ -114,7 +122,7 @@ private static int calculateSize(
       MarshalerWithSize anyValueMarshaler,
       KeyValueMarshaler[] attributeMarshalers,
       int droppedAttributesCount,
-      int flags,
+      TraceFlags traceFlags,
       @Nullable String traceId,
       @Nullable String spanId) {
     int size = 0;
@@ -131,7 +139,7 @@ private static int calculateSize(
     size += MarshalerUtil.sizeRepeatedMessage(LogRecord.ATTRIBUTES, attributeMarshalers);
     size += MarshalerUtil.sizeUInt32(LogRecord.DROPPED_ATTRIBUTES_COUNT, droppedAttributesCount);
 
-    size += MarshalerUtil.sizeFixed32(LogRecord.FLAGS, flags);
+    size += MarshalerUtil.sizeFixed32(LogRecord.FLAGS, toUnsignedInt(traceFlags.asByte()));
     size += MarshalerUtil.sizeTraceId(LogRecord.TRACE_ID, traceId);
     size += MarshalerUtil.sizeSpanId(LogRecord.SPAN_ID, spanId);
     return size;
@@ -194,4 +202,9 @@ static ProtoEnumInfo toProtoSeverityNumber(Severity severity) {
     // NB: Should not be possible with aligned versions.
     return SeverityNumber.SEVERITY_NUMBER_UNSPECIFIED;
   }
+
+  /** Vendored {@link Byte#toUnsignedInt(byte)} to support Android. */
+  private static int toUnsignedInt(byte x) {
+    return ((int) x) & 0xff;
+  }
 }
diff --git a/.../src/test/java/io/opentelemetry/exporter/otlp/internal/logs/LogsRequestMarshalerTest.java b/.../src/test/java/io/opentelemetry/exporter/otlp/internal/logs/LogsRequestMarshalerTest.java
@@ -14,8 +14,13 @@
 import io.opentelemetry.api.common.AttributeKey;
 import io.opentelemetry.api.common.Attributes;
 import io.opentelemetry.api.internal.OtelEncodingUtils;
+import io.opentelemetry.api.trace.Span;
+import io.opentelemetry.api.trace.SpanContext;
 import io.opentelemetry.api.trace.SpanId;
+import io.opentelemetry.api.trace.TraceFlags;
 import io.opentelemetry.api.trace.TraceId;
+import io.opentelemetry.api.trace.TraceState;
+import io.opentelemetry.context.Context;
 import io.opentelemetry.exporter.otlp.internal.Marshaler;
 import io.opentelemetry.proto.common.v1.AnyValue;
 import io.opentelemetry.proto.common.v1.InstrumentationLibrary;
@@ -58,8 +63,15 @@ void toProtoResourceLogs() {
                     .setBody(BODY)
                     .setSeverity(Severity.INFO)
                     .setSeverityText("INFO")
-                    .setTraceId(TRACE_ID)
-                    .setSpanId(SPAN_ID)
+                    .setContext(
+                        Context.root()
+                            .with(
+                                Span.wrap(
+                                    SpanContext.create(
+                                        TRACE_ID,
+                                        SPAN_ID,
+                                        TraceFlags.getDefault(),
+                                        TraceState.getDefault()))))
                     .setAttributes(Attributes.of(AttributeKey.booleanKey("key"), true))
                     .setEpoch(12345, TimeUnit.NANOSECONDS)
                     .build()));
@@ -91,8 +103,15 @@ void toProtoLogRecord() {
                     .setBody(BODY)
                     .setSeverity(Severity.INFO)
                     .setSeverityText("INFO")
-                    .setTraceId(TRACE_ID)
-                    .setSpanId(SPAN_ID)
+                    .setContext(
+                        Context.root()
+                            .with(
+                                Span.wrap(
+                                    SpanContext.create(
+                                        TRACE_ID,
+                                        SPAN_ID,
+                                        TraceFlags.getDefault(),
+                                        TraceState.getDefault()))))
                     .setAttributes(Attributes.of(AttributeKey.booleanKey("key"), true))
                     .setEpoch(12345, TimeUnit.NANOSECONDS)
                     .build()));
@@ -120,23 +139,17 @@ void toProtoLogRecord_MinimalFields() {
                 LogDataBuilder.create(
                         Resource.create(Attributes.builder().put("testKey", "testValue").build()),
                         InstrumentationLibraryInfo.create("instrumentation", "1"))
-                    .setBody(BODY)
-                    .setSeverity(Severity.INFO)
-                    .setAttributes(Attributes.of(AttributeKey.booleanKey("key"), true))
                     .setEpoch(12345, TimeUnit.NANOSECONDS)
                     .build()));
 
-    assertThat(logRecord.getTraceId().toByteArray()).isEmpty();
-    assertThat(logRecord.getSpanId().toByteArray()).isEmpty();
+    assertThat(logRecord.getTraceId()).isEmpty();
+    assertThat(logRecord.getSpanId()).isEmpty();
     assertThat(logRecord.getName()).isBlank();
     assertThat(logRecord.getSeverityText()).isBlank();
-    assertThat(logRecord.getBody()).isEqualTo(AnyValue.newBuilder().setStringValue(BODY).build());
-    assertThat(logRecord.getAttributesList())
-        .containsExactly(
-            KeyValue.newBuilder()
-                .setKey("key")
-                .setValue(AnyValue.newBuilder().setBoolValue(true).build())
-                .build());
+    assertThat(logRecord.getSeverityNumber().getNumber())
+        .isEqualTo(Severity.UNDEFINED_SEVERITY_NUMBER.getSeverityNumber());
+    assertThat(logRecord.getBody()).isEqualTo(AnyValue.newBuilder().setStringValue("").build());
+    assertThat(logRecord.getAttributesList()).isEmpty();
     assertThat(logRecord.getTimeUnixNano()).isEqualTo(12345);
   }
 

diff --git a/...otlp/logs/src/test/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogsExporterTest.java b/...otlp/logs/src/test/java/io/opentelemetry/exporter/otlp/logs/OtlpGrpcLogsExporterTest.java
@@ -19,9 +19,6 @@
 import io.grpc.inprocess.InProcessServerBuilder;
 import io.grpc.stub.StreamObserver;
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.trace.SpanId;
-import io.opentelemetry.api.trace.TraceFlags;
-import io.opentelemetry.api.trace.TraceId;
 import io.opentelemetry.exporter.otlp.internal.RetryPolicy;
 import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporter;
 import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
@@ -373,9 +370,6 @@ private static LogData generateFakeLog() {
             Resource.create(Attributes.builder().put("testKey", "testValue").build()),
             InstrumentationLibraryInfo.create("instrumentation", "1"))
         .setEpoch(Instant.now())
-        .setTraceId(TraceId.getInvalid())
-        .setSpanId(SpanId.getInvalid())
-        .setFlags(TraceFlags.getDefault().asByte())
         .setSeverity(Severity.ERROR)
         .setSeverityText("really severe")
         .setName("log1")

diff --git a/...ters/otlp/logs/src/testGrpcNetty/java/io/opentelemetry/exporter/otlp/logs/ExportTest.java b/...ters/otlp/logs/src/testGrpcNetty/java/io/opentelemetry/exporter/otlp/logs/ExportTest.java
@@ -14,9 +14,6 @@
 import com.linecorp.armeria.testing.junit5.server.ServerExtension;
 import io.grpc.stub.StreamObserver;
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.trace.SpanId;
-import io.opentelemetry.api.trace.TraceFlags;
-import io.opentelemetry.api.trace.TraceId;
 import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
 import io.opentelemetry.proto.collector.logs.v1.ExportLogsServiceRequest;
 import io.opentelemetry.proto.collector.logs.v1.ExportLogsServiceResponse;
@@ -44,9 +41,6 @@ class ExportTest {
                   Resource.create(Attributes.builder().put("testKey", "testValue").build()),
                   InstrumentationLibraryInfo.create("instrumentation", "1"))
               .setEpoch(Instant.now())
-              .setTraceId(TraceId.getInvalid())
-              .setSpanId(SpanId.getInvalid())
-              .setFlags(TraceFlags.getDefault().asByte())
               .setSeverity(Severity.ERROR)
               .setSeverityText("really severe")
               .setName("log1")

diff --git a/...tlp/logs/src/testGrpcNettyShaded/java/io/opentelemetry/exporter/otlp/logs/ExportTest.java b/...tlp/logs/src/testGrpcNettyShaded/java/io/opentelemetry/exporter/otlp/logs/ExportTest.java
@@ -14,9 +14,6 @@
 import com.linecorp.armeria.testing.junit5.server.ServerExtension;
 import io.grpc.stub.StreamObserver;
 import io.opentelemetry.api.common.Attributes;
-import io.opentelemetry.api.trace.SpanId;
-import io.opentelemetry.api.trace.TraceFlags;
-import io.opentelemetry.api.trace.TraceId;
 import io.opentelemetry.exporter.otlp.internal.grpc.DefaultGrpcExporterBuilder;
 import io.opentelemetry.proto.collector.logs.v1.ExportLogsServiceRequest;
 import io.opentelemetry.proto.collector.logs.v1.ExportLogsServiceResponse;
@@ -44,9 +41,6 @@ class ExportTest {
                   Resource.create(Attributes.builder().put("testKey", "testValue").build()),
                   InstrumentationLibraryInfo.create("instrumentation", "1"))
               .setEpoch(Instant.now())
-              .setTraceId(TraceId.getInvalid())
-              .setSpanId(SpanId.getInvalid())
-              .setFlags(TraceFlags.getDefault().asByte())
               .setSeverity(Severity.ERROR)
               .setSeverityText("really severe")
               .setName("log1")