Repository navigation
Extract common class for WithTimestamps.AddTimestampsDoFn and Create.TimestampedValues.ConvertTimestamps #18350
Description
It seems like these APIs are predominantly duplicative of each other and, that it's hard to find one of them if you knew about the other.
What would make the most sense to me is if TimestampedValues were implemented in terms of both Values and WithTimestamps. I'm still learning about Beam though -- would this approach cause some kind of performance problem?
Imported from Jira BEAM-1811. Original Jira may contain additional context.
Reported by: wtanaka.
Activity
Gaurav598 commentedon Sep 1, 2026
I investigated the current implementation of this issue and wanted to leave a few findings here that may be useful to whoever picks it up next.
The duplication between
WithTimestamps.AddTimestampsDoFnandCreate.TimestampedValues.ConvertTimestampsis still present: both ultimately useoutputWithTimestamp(...).However, a direct
Create.Values + WithTimestampsimplementation does not appear to be a straightforward replacement. The timestamp is part of eachTimestampedValue<T>and is not necessarily derivable fromT, so duplicate/equal values can have different timestamps.I also found that
Create.TimestampedValueswas previously related toCreate.Valuesthrough inheritance, but that was intentionally removed because it caused runner matching issues. Reintroducing that relationship would therefore be risky.The current
TimestampedValuesimplementation also deliberately:Tvalues,TimestampedValueCoderfor the intermediate collection,ConvertTimestamps,Coder<T>.Because of this, a conservative internal abstraction for the common timestamp-output behavior seems safer than restructuring
TimestampedValuesaroundValues + WithTimestamps. This would allow the existing execution path, coder behavior, and transform structure to remain unchanged.The relevant tests also include renderer and Dataflow runner expectations around the current
Create.TimestampedValues/ConvertTimestampsstructure, so those are worth considering before changing the pipeline graph.Hopefully these findings save some investigation time for the next contributor.
xujiantop-crypto commentedon Sep 9, 2026
.take-issue
kennknowles commentedon Sep 9, 2026
TBH I don't think this is a good idea. This creates an inheritance-based coupling when just having this trivial code in two places is more flexible and keeps control flow simpler.