KAFKA-13511: Add support for different unix precisions in TimestampConverter SMT

[jchanaud]

Currently, the SMT TimestampConverter can convert Timestamp from either source String, Long or Date into target String, Long or Date.

The problem is that Long source or target is required to be epoch in milliseconds.

In many cases, epoch is represented with different precisions. This leads to several Jira tickets :

KAFKA-12364: add support for date from int32 to timestampconverter
KAFKA-10561: Support microseconds precision for Timestamps
I propose to add a new config to TimestampConverter called "epoch.precision" which defaults to "millis" so as to not impact existing code, and allows for more precisions : seconds, millis, micros.

"transforms": "TimestampConverter",
"transforms.TimestampConverter.type": "org.apache.kafka.connect.transforms.TimestampConverter$Value",
"transforms.TimestampConverter.field": "event_date",
"transforms.TimestampConverter.epoch.precision": "micros",
"transforms.TimestampConverter.target.type": "Timestamp"

Exactly like "format" field which is used as input when the source in String and output when the target.type is string, this new field would be used as input when the field is Long, and as output when the target.type is "unix"

Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[jchanaud]

@mimaison @rhauch before considering going further with this PR, any chance I could to get your opinion on this subject ?

The main idea behind this PR is that Avro logical types are not fully supported in Kafka Connect but even if they were, it would not help schema-less messages that need to convert epoch with a precision different than ms.

For this reason, I consider this would be an interesting addition.
Thanks for your help.

[mimaison]

Hi @twobeeb, thanks for the PR, this looks like a useful addition. However as this is adding a new configuration, this change requires a KIP before we can accept it.
Take a look at https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals. This page details the KIP process. Let me know if you have any questions.

[jchanaud]

Hi @mimaison,
I haven't received any update on the KIP-808 which I created as per your suggestion.

Could you be of any help ?

[mimaison]

Sorry @twobeeb, I'll try to take a look this afternoon. Thanks

[jchanaud]

Hi everyone,
Since KIP-808 is now adopted, could you review and comment this PR ?
Thanks

[tombentley]

Thanks for the PR!

[jchanaud]

@tombentley
Thanks for your inputs.
Regarding your last comment which I fixed, the same case could be made for target.type.
https://github.com/apache/kafka/blob/trunk/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/TimestampConverter.java#L254-L257
Should we address this ? If so, this PR or new one ?

[mimaison]

Thanks @twobeeb for the PR. It looks good, I just left a minor comment.

[mimaison]

@tombentley Thanks for your inputs. Regarding your last comment which I fixed, the same case could be made for target.type. https://github.com/apache/kafka/blob/trunk/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/TimestampConverter.java#L254-L257 Should we address this ? If so, this PR or new one ?

Yes feel free to address it in this PR if you want.

[mimaison]

LGTM, thanks for the PR!