TimestampProvider not working or not being used

[MartijnSchoemaker]

Version Information
Version of Akka.NET?
1.4.19

Which Akka.NET Modules?
Akka.Persistence.Sql.Common.Journal

Describe the bug
I implemented a custom ITimestampProvider for SQL Persistence Journal.
I put a breakpoint in GenerateTimestamp() and let this method return 0.
But every time I persist an entity I keep seeing DateTime.UtcNow.Ticks used as TimeStamp.

To Reproduce

1. Added customer ITimestampProvider. Let method return 0.
2. Add config:
   persistence : { journal : { plugin : akka.persistence.journal.sql-server sql-server : { timestamp-provider : "xx.xx.xx, xx.xx" } }
3. Persist entity
4. Look in EventJournal table and see TimeStamp is a big long (not 0).

Expected behavior
Use long returned by custom ITimestampProvider.

Actual behavior
DateTime.UtcNow.Ticks is used as TimeStamp.

Additional context
I tried to debug this and found out that in SqlServerJournal line 62 is returning an instance of my own TimetampProvider. So that seems to work!
https://github.com/akkadotnet/Akka.Persistence.SqlServer/blob/6f9eae1f0d17ab76adf60809e2a631d8acfa2e82/src/Akka.Persistence.SqlServer/Journal/SqlServerJournal.cs#L62

But another thing I saw in the source is that in AbstractQueryExecutor the TimestampProvider is passed as parameter, but only the property is set. It seems that the property is never used.

akka.net/src/contrib/persistence/Akka.Persistence.Sql.Common/Journal/QueryExecutor.cs

Line 319 in 55c1175

TimestampProvider = timestampProvider;

In InsertBatchAsync as timestamp is used 'DateTime.UtcNow.Ticks'. I should expect the TimestampProvider being used here?

akka.net/src/contrib/persistence/Akka.Persistence.Sql.Common/Journal/QueryExecutor.cs

Line 687 in 55c1175

WriteEvent(command, evt.WithTimestamp(DateTime.UtcNow.Ticks), tags);

Screenshots

[Aaronontheweb]

This looks like a bug alright! We'll see if we can get this patched before we do v1.4.21 later this week.

[Aaronontheweb]

Should actually be a pretty trivial fix - just need to have the timestamp provider call there instead of DateTime.UtcNow.Ticks.

[MartijnSchoemaker]

Yes indeed. Thats what I thought as well. But I'm not really into this code, so I thought probably I missed something and I did something wrong... 😊

[Aaronontheweb]

@MartijnSchoemaker feel free to send us a PR if you want! 👍

[ismaelhamed]

Since the concept of TimestampProvider does not exist in the JVM, this was an oversight while porting the Timestamp support for envelopes.

[Aaronontheweb]

@ismaelhamed I figured as much - no big deal. This is the first time in ~7 years I've ever heard of someone actually customizing the ITimestampProvider....

[MartijnSchoemaker]

We get a list of mutations of an object from another party every 5 weeks.
In that data are mutations for example for 3 days ago. We need to be able to get the state of the object for a specific date. So its neccesary for us to add mutations (events) with a date in the past.
Thats why I was happy that I found the timestamp provider.😅

[Aaronontheweb]

@MartijnSchoemaker makes sense!