Logging Structs should prefer map over iterable when serializing #2432
Description
The current serialization code looks like this at time of writing:
private static Value objectToValue(final Object obj) {
Value.Builder builder = Value.newBuilder();
if (obj == null) {
builder.setNullValue(NullValue.NULL_VALUE);
return builder.build();
}
Class<?> objClass = obj.getClass();
if (obj instanceof String) {
builder.setStringValue((String) obj);
} else if (obj instanceof Number) {
builder.setNumberValue(((Number) obj).doubleValue());
} else if (obj instanceof Boolean) {
builder.setBoolValue((Boolean) obj);
} else if (obj instanceof Iterable<?> || objClass.isArray()) {
builder.setListValue(ListValue.newBuilder()
.addAllValues(Iterables.transform(Types.iterableOf(obj), OBJECT_TO_VALUE)));
} else if (objClass.isEnum()) {
builder.setStringValue(((Enum<?>) obj).name());
} else if (obj instanceof Map) {
Map<String, Object> map = (Map<String, Object>) obj;
builder.setStructValue(newStruct(map));
} else {
throw new IllegalArgumentException(String.format("Unsupported protobuf value %s", obj));
}
return builder.build();
}I think it would be better if the check for instanceof Map happened before instanceof Iterable<?>. It's true that Maps in the stdlib are generally not iterable, but third party ones can be. For example, Clojure's PersistentHashMap and PersistentArrayMap are both Iterable. If something is both a Map and Iterable, Map seems the most desirable behavior.