SpanExporter not able to process ArrayValue in Attributes

[EddeCCC]

Describe the bug

I've tried to use the export() method from the OtlpGrpcSpanExporter and OtlpHttpSpanExporter to export SpanData.
My SpanData contains Attributes with values of type ArrayValue. After running the method, I receive an exception and the export fails.
I thought that this behavior was fixed in this PR, but in version 1.35.0 it still occurs.

It seems like the reason for this error is the create(AttributeKey<?> attributeKey, Object value) method of the KeyValueMarshaler. If an array is detected, it tries to cast the value to a List<>, which does not work, since value does not implement any Collections.

However, an ArrayValue object contains a collection with the variable values_. I think this variable should somehow be used for casting.

Steps to reproduce
Call the export() method of OtlpGrpcSpanExporter or OtlpHttpSpanExprorter with SpanData that contains Attributes with a value of type ArrayValue. The array should not be empty.

What did you expect to see?
I expect the export() method to work sucessfully, since I use a valid SpanData collection.

What did you see instead?
It throws this exception:
java.lang.ClassCastException: class io.opentelemetry.proto.common.v1.ArrayValue cannot be cast to class java.util.List (io.opentelemetry.proto.common.v1.ArrayValue is in unnamed module of loader org.springframework.boot.loader.launch.LaunchedClassLoader @6f539caf; java.util.List is in module java.base of loader 'bootstrap') at io.opentelemetry.exporter.internal.otlp.KeyValueMarshaler.create(KeyValueMarshaler.java:78) at io.opentelemetry.exporter.internal.otlp.KeyValueMarshaler.access$000(KeyValueMarshaler.java:30) at io.opentelemetry.exporter.internal.otlp.KeyValueMarshaler$1.accept(KeyValueMarshaler.java:48) at io.opentelemetry.exporter.internal.otlp.KeyValueMarshaler$1.accept(KeyValueMarshaler.java:43) at io.opentelemetry.api.internal.ImmutableKeyValuePairs.forEach(ImmutableKeyValuePairs.java:94) at io.opentelemetry.exporter.internal.otlp.KeyValueMarshaler.createRepeated(KeyValueMarshaler.java:42) at io.opentelemetry.exporter.internal.otlp.ResourceMarshaler.create(ResourceMarshaler.java:39) at io.opentelemetry.exporter.internal.otlp.traces.ResourceSpansMarshaler.create(ResourceSpansMarshaler.java:57) at io.opentelemetry.exporter.internal.otlp.traces.TraceRequestMarshaler.create(TraceRequestMarshaler.java:32) at io.opentelemetry.exporter.otlp.http.trace.OtlpHttpSpanExporter.export(OtlpHttpSpanExporter.java:75)

What version and what artifacts are you using?

Artifacts: opentelemetry-bom, opentelemetry-bom-alpha, opentelemetry-sdk, opentelemetry-exporter-otlp, opentelemetry-semconv, opentelemetry-proto
Version: 1.35.0 (except opentelemetry-semconv, which uses 1.30.1-alpha and opentelemetry-proto, which uses 1.1.0-alpha)
How did you reference these artifacts? build.gradle

Environment
OS: Windows 10
Runtime: JDK 17

Additional context
We also use Spring Boot 3.2.2 in the project, if this matters. But we have not instrumented the application. It's just for processing data.

[breedx-splk]

Hi @EddeCCC .

This shouldn't be the case. Attributes are designed in a way that tightly couples the key type to the value type. You can even see in the create() method that you mentioned above, that the key types are a fixed set: https://github.com/open-telemetry/opentelemetry-java/blob/main/exporters/otlp/common/src/main/java/io/opentelemetry/exporter/internal/otlp/KeyValueMarshaler.java#L79. As far as I know, none of those should allow for a value of type ArrayValue.

So the question is, how are you creating the attributes that have an ArrayValue as the value? Can you provide a simple repro case that shows this?

[EddeCCC]

Hi @breedx-splk,

thanks for your answer. I created a showcase for this issue, which represents a cut-out of our actual project.
You can find it here: https://github.com/EddeCCC/span-exporter-showcase

Would be awesome, if you can find any hints, why the exporter is not working properly.

[breedx-splk]

Hey @EddeCCC . Thanks for the repro repo...that helped me to get a stronger sense of what's going on. I was able to reproduce this with an even more trivial example:

  public static void main(String[] args) {
    AttributeKey k = AttributeKey.stringArrayKey("mykey");
    ArrayValue av = ArrayValue.newBuilder()
        .addValues(AnyValue.newBuilder().setStringValue("boom").build())
        .build();
    Attributes attributes = Attributes.builder()
        .put(k, av)
        .build();
    KeyValueMarshaler[] marshaler = KeyValueMarshaler.createForAttributes(attributes);
    System.out.println("Will not reach this");
  }

This is an unfortunate artifact of using raw types instead of fully fleshed out generics. If you use the raw type AttributeKey (instead of the specific generic), you can unfortunately pass any type into the value when calling AttributesBuilder.put(). For example, I can change the above to:

Attributes attributes = Attributes.builder()
        .put(k, 42L)
        .build();

and it will explode with class java.lang.Long cannot be cast to class java.util.List. It's also true with any other key types.

In the otel java repos, we explicitly run with -Werror so that the warnings being flagged by the compiler are errors. If you refactor your code to use the real generics instead of raw types, I think you can probably avoid this problem entirely, but it will be some work. Another way to work around this is to be a little more diligent to ensure that you're matching the key types with the value types when you build the Attributes.

@open-telemetry/java-maintainers Curious what you think about this ability to break the Attributes type contract by ab/using raw types?

[jkwatson]

@open-telemetry/java-maintainers Curious what you think about this ability to break the Attributes type contract by ab/using raw types?

You can trivially do this with any of the generic JDK containers as well:

  public static void main(String... args) {
    List<String> strings = new ArrayList<>();
    ((List) strings).add(1L);

    strings.forEach(System.out::println); // boom
  }

I don't think this is something we can fix; it's baked in to how Java generic containers work.

[EddeCCC]

Hi @breedx-splk,

thanks for your feedback. I changed the implementation and now it's working again:

String key = attribute.getKey();
AnyValue value = attribute.getValue();
AnyValue.ValueCase valueCase = attribute.getValue().getValueCase();
switch (valueCase) {
    case STRING_VALUE ->  {
        AttributeKey<String> attributeKey = AttributeKey.stringKey(key);
        builder.put(attributeKey, value.getStringValue());
    }
    case BOOL_VALUE -> {
        AttributeKey<Boolean> attributeKey = AttributeKey.booleanKey(key);
        builder.put(attributeKey, value.getBoolValue());
    }
    case INT_VALUE -> {
        AttributeKey<Long> attributeKey = AttributeKey.longKey(key);
        builder.put(attributeKey, value.getIntValue());
    }
    case DOUBLE_VALUE -> {
        AttributeKey<Double> attributeKey = AttributeKey.doubleKey(key);
        builder.put(attributeKey, value.getDoubleValue());
    }
    case ARRAY_VALUE -> {
        AttributeKey<List<String>> attributeKey = AttributeKey.stringArrayKey(key);
        List<String> values = value.getArrayValue().getValuesList().stream()
                .map(AnyValue::getStringValue)
                .toList();
        builder.put(attributeKey, values);
    }
}

This fixed my problem.