Allow LogRotatorSink be a Sink[T] instead of Sink[ByteString]

[shagoon]

The current implementation of LogRotatorSink works well for time-based rotation, if the time is the real time. It does not fit well, if the time to trigger on is contained in the stream itself, since the stream is already of type ByteString. You had to parse that ByteString to gain access to the time contained in the data (or anything else you would like to trigger on).

This PR slightly modifies LogRotatorSink to accept an additional type parameter T. The triggerGeneratorCreator function becomes T => Option[C], the sinkFactory becomes C => Sink[T, Future[R]]. This allows to trigger on data contained in T, the sink also works on T and probably uses a transformer T => ByteString. For details, have a look at the new test in LogRotatorSinkSpec.

[ennru]

Hi @shagoon,

Thank you for your suggestion to make the log rotator more versatile!

Can you re-work your suggestion a bit so that the current withSinkFactory API in the object LogRotatorSink stays as before and just add new methods to support extracting the timestamp from the data?

[shagoon]

Hi @ennru,

thanks for you feedback. Yes, I can try to rework my changes as you suggested, but, could you please explain, why you suggested those changes? The way I implemented withSinkFactory requires no further changes in existing code (i.e. the existing test in LogRotatorSinkSpec utilising withSinkFactory had not to be modified). I guess binary compatibility is an issue here?

If I introduce a new method to the object LogRotatorSink, this method would have the same signature as the existing one (after type erasure), so, a different name would be required. Any suggestions?

[ennru]

The goal is to stay binary-compatible in the public API.
See https://travis-ci.com/github/akka/alpakka/jobs/338190546#L669

You will need to add Mima exclusions to make it accept the changes to the LogRotatorSink stage class.

Maybe genericSinkFactory or withTypedSinkFactory?

[shagoon]

I see. I just pushed an update with withSinkFactory reverted to the original code and an additional method withTypedSinkFactory (which I liked more than genericSinkFactory).

[ennru]

LGTM.