ProtobufJsonPayloadConverter does not work correctly with shaded artifact

[rocketraman]

Expected Behavior

The ProtobufJsonPayloadConverter does not work correctly when using the shaded Temporal artifact, which is sometimes necessary to avoid gRPC version conflicts.

The problem is:

sdk-java/temporal-sdk/src/main/java/io/temporal/common/converter/ProtobufJsonPayloadConverter.java

Lines 51 to 53 in 81f3cc3

	if (!(value instanceof MessageOrBuilder)) {
	return Optional.empty();
	}

Here, MessageOrBuilder is rewritten in the byte-code to be io.temporal.shaded.com.google.protobuf.MessageOrBuilder, and so passing in a non-shaded value (as certainly client code will be using) fails, and the converter fails to convert the value.

The converter, whether it is shaded or not, should correctly recognize protobuf values coming from client code.

Actual Behavior

Protobuf values coming from client code are ignored, and serialization fails.

Steps to Reproduce the Problem

Using the shaded temporal artifact, run this code:

import com.google.protobuf.StringValue;
import io.temporal.common.converter.ProtobufJsonPayloadConverter;

class Test {
    public static void main(String[] args) {
        var converter = new ProtobufJsonPayloadConverter();
        var msg = StringValue.of("hello");
        var payloadOpt = converter.toData(msg);
        System.out.println(payloadOpt.isPresent());
    }
}

We can see that the payload is not present.

Specifications

• Version: 1.32.1
• Platform: N/A

[mjameswh]

Thanks for the report and the repro.

What you describe is unfortunate, but expected when using the shaded artifact. As you pointed out, once protobuf is shaded, io.temporal.shaded.com.google.protobuf.* is a completely different type from com.google.protobuf.*, so not only will the instanceof MessageOrBuilder check will fail for non-shaded messages, but casting a message to what the SDK knows as the MessageOrBuilder type would also fail. That's by definition.

Can you clarify what you have in mind?

[rocketraman]

@mjameswh Thanks for your reply.

What you describe is unfortunate, but expected when using the shaded artifact. As you pointed out, once protobuf is shaded, io.temporal.shaded.com.google.protobuf.* is a completely different type from com.google.protobuf.*, so not only will the instanceof MessageOrBuilder check will fail for non-shaded messages, but casting a message to what the SDK knows as the MessageOrBuilder type would also fail. That's by definition.

Can you clarify what you have in mind?

One option might be to release the ProtobufJsonPayloadConverter (and any other affected converters) as a separate artifact, and remove them from the shaded artifact. This way, anyone using the shaded jar will be forced to pull and depend on the artifact containing the non-shaded (and thus functional) payload converters.

Another option would be to configure the shader to filter the converters from the relocation operation. I haven't checked which tool/library is being used to do the shading, but if using the common shadow plugin, you can use these relocation filtering options to exclude the ProtobufJsonPayloadConverter (and any other affected converters) from the relocation.