Support Protobuf descriptor files in addition to Base64 format

[xakassi]

This is an addition to First Protobuf PR.

In previous PR Protobuf descriptor files were specified in Base64 format.
Now you also can put descriptor files in a folder and specify folder and file names in configuration.

[xakassi]

Hi, @jorgheymans !
I will be glad if you review this PR as well as @tchiotludo :)

[tchiotludo]

Thanks @xakassi !
I would say, we need @jorgheymans to review 😅 !

[jorgheymans]

Thanks @xakassi . The current implementation does not support serializing types that are not known upfront using Any.

Considering this:

syntax = "proto3";
package org.akhq.utils;

import "album.proto";
import "film.proto";
import "google/protobuf/wrappers.proto";
import "google/protobuf/any.proto";

option java_package = "org.akhq.utils";
option java_outer_classname = "ComplexProto";

message Complex {
  Album album = 1;
  Film film = 2;
  google.protobuf.StringValue stringWrapper = 3;
  google.protobuf.Any anything = 4;
}

This will deserialize fine if the anything field is either an Album or Film type, but it won't work if it's any other type that is not directly referenced in the proto definition itself. Internally we have solved this by creating a dependency graph of all supplied proto definitions and supplementing it with the google wrapper types. That way the TypeRegistry has everything it needs to do serde. I am going to have a look if we can lift that code into this PR, it's not all that much.

EDIT:

So assuming book.proto :

syntax = "proto3";
package org.akhq.utils;

import "google/protobuf/wrappers.proto";

option java_package = "org.akhq.utils";
option java_outer_classname = "BookProto";

message Book {
  string title = 1;
  string author = 2;
  google.protobuf.DoubleValue price = 3;
}

This proto object will fail to deserialize:

    private void createComplexObject() {
        BookProto.Book bookProto =
            BookProto.Book.newBuilder().setAuthor("akhq").setPrice(DoubleValue.newBuilder().setValue(123d)).build();
        complexProto = ComplexProto.Complex.newBuilder()
            .setAlbum(albumProto)
            .setFilm(filmProto)
            .setAnything(Any.pack(bookProto))
            .setStringWrapper(StringValue.newBuilder().setValue("stringvalue").build()).build();
    }

2020-11-30 14:14:43,541 ERROR est worker ProtobufToJsonDeserializer Cannot deserialize message with Protobuf deserializer for topic [complex.topic.name] and message type [Complex]
com.google.protobuf.InvalidProtocolBufferException: Cannot find type for url: type.googleapis.com/org.akhq.utils.Book
	at com.google.protobuf.util.JsonFormat$PrinterImpl.printAny(JsonFormat.java:886)
	at com.google.protobuf.util.JsonFormat$PrinterImpl.access$1000(JsonFormat.java:714)
	at com.google.protobuf.util.JsonFormat$PrinterImpl$1.print(JsonFormat.java:787)
	at com.google.protobuf.util.JsonFormat$PrinterImpl.print(JsonFormat.java:766)
	at com.google.protobuf.util.JsonFormat$PrinterImpl.printSingleFieldValue(JsonFormat.java:1263)
	at com.google.protobuf.util.JsonFormat$PrinterImpl.printSingleFieldValue(JsonFormat.java:1128)
	at com.google.protobuf.util.JsonFormat$PrinterImpl.printField(JsonFormat.java:1052)

[xakassi]

Hi, @jorgheymans !
Thanks for the full example and steps to reproduce!
Ok, I understand the problem, I can investigate it a bit later.

Maybe we should do it in a different PR as bugfix? Since this PR is just to support descriptors as files. So we could merge it if you ok with it and then concentrate on the bug.

@tchiotludo , @jorgheymans what do you think about it?

[tchiotludo]

agree with @xakassi
It's another feature / fix.

@jorgheymans any things else is working ? we can merge ?

[jorgheymans]

I will test again the PR as is tomorrow. All our topics have Any attributes in them, so the serialization should have thrown an exception or at least logged an error but it did not. I'll have a closer look why and report back.

[jorgheymans]

I have configured the akhq yaml file with this:

    deserialization:
      protobuf:
        descriptors-folder: "/new/descriptors"
        topics-mapping:
          - topic-regex: "han.*"
            descriptor-file: "han-proto.protobin"
            value-message-type: "han.message.HanMessage"

When then browsing a topic han_test i do not get an exception and it's as if there is just a toString() happening on the message. Am I missing something ?

[xakassi]

Looks like your configuration is ok.

I suppose if descriptor file cannot be built the error should appear in the log:
https://github.com/xakassi/akhq/blob/d3770f0b885de619312d5a076f120b8825871671/src/main/java/org/akhq/utils/ProtobufToJsonDeserializer.java#L52

And for any deserialization error we should see in logs a message:
https://github.com/xakassi/akhq/blob/d3770f0b885de619312d5a076f120b8825871671/src/main/java/org/akhq/utils/ProtobufToJsonDeserializer.java#L105

I decided that an error message in the log is enough and we should try to see a topic content at least as a string if something went wrong. That's why I return Null if I cannot deserialize with Protobuf and try just to print String:
https://github.com/xakassi/akhq/blob/d3770f0b885de619312d5a076f120b8825871671/src/main/java/org/akhq/models/Record.java#L121

If there are no any errors from ProtobufToJsonDeserializer in the logs, I will try to reproduce your case and check.

[xakassi]

Hi, @jorgheymans !
Sorry for a big delay, it was a really busy week.
I have checked your case and created a new unit test for a complex object with Any field.
I think, we should not throw an exception but try to show topic content at least as a string. And there is an error message in logs.

Please, check error message (deserializeComplexObject() unit test).

And regarding throwing an exception if you and @tchiotludo suppose that it is preferred, I will change logic and will throw an exception.

[tchiotludo]

@xakassi, the better will be to show the exception on the ui like we are doing on the avro message since this commit : 7cdaacf

Possible ? 👍

[xakassi]

Sorry, I do not actually fully understand should I add something on UI side for error handling (I'm not very experienced with UI :c ), but I have changed my log.error() massages on throw new RuntimeException() and now error appear on UI when a topic cannot be deserialized with ProtobufDesrializer, or Protobuf descriptor files cannot be properly handled.
And actually error message appear even on the page with topics list, because Last Record cannot be calculated if some topics cannot be deserialized.

@tchiotludo Do you mean that? Or I did something wrong? Please, check.

[tchiotludo]

Sorry, I point you to the wrong commit !
I've added a property here : 92d4ee8 that will allow you to send the exception to the ui.

Can you fill this Record.exceptions with the exception please ? With that the exception will be available on the ui and not in the log.
It will remove the ugly popup for last record for example.

Thanks

[xakassi]

Now I fill Record.exceptions with the exception. It is indeed much more better than red pop-up and we able to see the list of topics even if there are some deserialization problems with some topics. That's great!

[xakassi]

@tchiotludo maybe we can merge this? And then I will try to solve the problem with "Any" field in a next PR. And @jorgheymans will be able to test it in a normal way, since all his topics records have "Any" field.

[tchiotludo]

agreed :) thanks for the hard work