Fix OTLP marshaling for empty string attributes (#8014)

Author: trask

exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/Serializer.java

@@ -257,6 +257,16 @@ public void serializeStringWithContext(
     if (string == null || string.isEmpty()) {
       return;
     }
+    writeStringWithContext(field, string, context);
+  }
+
+  /**
+   * Writes a protobuf {@code string} field, even if it matches the default value. This method reads
+   * elements from context, use together with {@link StatelessMarshalerUtil#getUtf8Size(String,
+   * MarshalerContext)}.
+   */
+  public void writeStringWithContext(ProtoFieldInfo field, String string, MarshalerContext context)
+      throws IOException {
     if (context.marshalStringNoAllocation()) {
       writeString(field, string, context.getSize(), context);
     } else {

exporters/common/src/main/java/io/opentelemetry/exporter/internal/marshal/StatelessMarshalerUtil.java

@@ -93,14 +93,22 @@ public static int sizeStringWithContext(
     if (value == null || value.isEmpty()) {
       return sizeBytes(field, 0);
     }
+    return sizeBytes(field, getUtf8Size(value, context));
+  }
+
+  /**
+   * Returns the UTF-8 size of a string, adding data to the context for later serialization. Use
+   * together with {@link Serializer#writeString(ProtoFieldInfo, String, int, MarshalerContext)}.
+   */
+  public static int getUtf8Size(String value, MarshalerContext context) {
     if (context.marshalStringNoAllocation()) {
       int utf8Size = context.getStringEncoder().getUtf8Size(value);
       context.addSize(utf8Size);
-      return sizeBytes(field, utf8Size);
+      return utf8Size;
     } else {
       byte[] valueUtf8 = MarshalerUtil.toBytes(value);
       context.addData(valueUtf8);
-      return sizeBytes(field, valueUtf8.length);
+      return valueUtf8.length;
     }
   }
 

exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/StringAnyValueMarshaler.java

@@ -33,16 +33,10 @@ static MarshalerWithSize create(String value) {
 
   @Override
   public void writeTo(Serializer output) throws IOException {
-    if (valueUtf8.length == 0) {
-      return;
-    }
     output.writeString(AnyValue.STRING_VALUE, valueUtf8);
   }
 
   private static int calculateSize(byte[] valueUtf8) {
-    if (valueUtf8.length == 0) {
-      return 0;
-    }
     return AnyValue.STRING_VALUE.getTagSize()
         + CodedOutputStream.computeByteArraySizeNoTag(valueUtf8);
   }

exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/StringAnyValueStatelessMarshaler.java

@@ -5,6 +5,7 @@
 
 package io.opentelemetry.exporter.internal.otlp;
 
+import io.opentelemetry.exporter.internal.marshal.CodedOutputStream;
 import io.opentelemetry.exporter.internal.marshal.MarshalerContext;
 import io.opentelemetry.exporter.internal.marshal.Serializer;
 import io.opentelemetry.exporter.internal.marshal.StatelessMarshaler;
@@ -26,11 +27,14 @@ private StringAnyValueStatelessMarshaler() {}
   @Override
   public void writeTo(Serializer output, String value, MarshalerContext context)
       throws IOException {
-    output.serializeStringWithContext(AnyValue.STRING_VALUE, value, context);
+    output.writeStringWithContext(AnyValue.STRING_VALUE, value, context);
   }
 
   @Override
   public int getBinarySerializedSize(String value, MarshalerContext context) {
-    return StatelessMarshalerUtil.sizeStringWithContext(AnyValue.STRING_VALUE, value, context);
+    int utf8Size = StatelessMarshalerUtil.getUtf8Size(value, context);
+    return AnyValue.STRING_VALUE.getTagSize()
+        + CodedOutputStream.computeUInt32SizeNoTag(utf8Size)
+        + utf8Size;
   }
 }

exporters/otlp/common/src/test/java/io/opentelemetry/exporter/internal/otlp/ValueMarshalerTest.java

@@ -123,7 +123,23 @@ private static Stream<Arguments> serializeAnyValueArgs() {
             of("hello world".getBytes(StandardCharsets.UTF_8)),
             AnyValue.newBuilder()
                 .setBytesValue(ByteString.copyFrom("hello world".getBytes(StandardCharsets.UTF_8)))
-                .build()));
+                .build()),
+        // empty values
+        arguments(of(""), AnyValue.newBuilder().setStringValue("").build()),
+        arguments(of(false), AnyValue.newBuilder().setBoolValue(false).build()),
+        arguments(of(0), AnyValue.newBuilder().setIntValue(0).build()),
+        arguments(of(0.0), AnyValue.newBuilder().setDoubleValue(0.0).build()),
+        arguments(
+            Value.of(Collections.emptyList()),
+            AnyValue.newBuilder().setArrayValue(ArrayValue.newBuilder().build()).build()),
+        arguments(
+            of(Collections.emptyMap()),
+            AnyValue.newBuilder().setKvlistValue(KeyValueList.newBuilder().build()).build()),
+        arguments(of(new byte[0]), AnyValue.newBuilder().setBytesValue(ByteString.EMPTY).build())
+        // TODO add test for true empty value
+        // after https://github.com/open-telemetry/opentelemetry-java/pull/7973
+        // arguments(Value.empty(), AnyValue.newBuilder().build())
+        );
   }
 
   @SuppressWarnings("unchecked")

exporters/otlp/common/src/test/java/io/opentelemetry/exporter/internal/otlp/traces/TraceRequestMarshalerTest.java

@@ -157,8 +157,15 @@ void toProtoSpan(MarshalerSource marshalerSource) {
                                         "nested", Value.of("value"))))
                             .put("heterogeneousArray", Value.of(Value.of("string"), Value.of(123L)))
                             .put("empty", Value.empty())
+                            .put("empty_string", "")
+                            .put("false_value", false)
+                            .put("zero_int", 0L)
+                            .put("zero_double", 0.0)
+                            .put("empty_array", Value.of(Collections.emptyList()))
+                            .put("empty_map", Value.of(Collections.emptyMap()))
+                            .put("empty_bytes", Value.of(new byte[] {}))
                             .build())
-                    .setTotalAttributeCount(13)
+                    .setTotalAttributeCount(20)
                     .setEvents(
                         Collections.singletonList(
                             EventData.create(12347, "my_event", Attributes.empty())))
@@ -275,6 +282,37 @@ void toProtoSpan(MarshalerSource marshalerSource) {
                                         .build())
                                 .build())
                         .build())
+                .build(),
+            KeyValue.newBuilder()
+                .setKey("empty_string")
+                .setValue(AnyValue.newBuilder().setStringValue("").build())
+                .build(),
+            KeyValue.newBuilder()
+                .setKey("false_value")
+                .setValue(AnyValue.newBuilder().setBoolValue(false).build())
+                .build(),
+            KeyValue.newBuilder()
+                .setKey("zero_int")
+                .setValue(AnyValue.newBuilder().setIntValue(0).build())
+                .build(),
+            KeyValue.newBuilder()
+                .setKey("zero_double")
+                .setValue(AnyValue.newBuilder().setDoubleValue(0.0).build())
+                .build(),
+            KeyValue.newBuilder()
+                .setKey("empty_array")
+                .setValue(
+                    AnyValue.newBuilder().setArrayValue(ArrayValue.newBuilder().build()).build())
+                .build(),
+            KeyValue.newBuilder()
+                .setKey("empty_map")
+                .setValue(
+                    AnyValue.newBuilder().setKvlistValue(KeyValueList.newBuilder().build()).build())
+                .build(),
+            KeyValue.newBuilder()
+                .setKey("empty_bytes")
+                .setValue(
+                    AnyValue.newBuilder().setBytesValue(ByteString.copyFrom(new byte[] {})).build())
                 .build());
     assertThat(protoSpan.getDroppedAttributesCount()).isEqualTo(1);
     assertThat(protoSpan.getEventsList())