Hi Matthias, Quick clarification: since KafkaProducer, KafkaAdminClient, etc. are in different packages than Metrics, wouldn't they lose access entirely once the constructor is package-private in 5.0, regardless of whether they pass Time.SYSTEM or a custom Time?
Looking at KafkaProducer specifically: One of its public constructors already hardcodes Time.SYSTEM, but the package-private "visible for testing" KafkaProducer constructor (used by tests to inject MockTime) still calls new Metrics(metricConfig, reporters, time, metricsContext): a cross-package call into org.apache.kafka.common.metrics. That call would break once Metrics' constructor becomes package-private, since KafkaProducer lives in org.apache.kafka.clients.producer. Would a sister class (like TestMetrics) be needed here too, or is there a different approach in mind for these "visible for testing" constructors/methods that sit in other packages? Thanks, Siddhartha On Fri, Jul 31, 2026 at 7:42 PM Matthias J. Sax <[email protected]> wrote: > I am not totally sure I understand the "plash radius" problem and claim > of a required huge code change? > > For `Metrics` for example, we currently pass in a `Time` object in unit > tests. So I think we can make the deprecated constructors > package-private in 5.0, effectively removing them from the public API. > We create a "sister class" `TestMetrics extends Metrics` (same package) > in a test module, and add a public constructor accepting the now > internal `Time` as parameter, allowing all tests to create `new > TestMetrics` object. > > This should not be a huge code change, and mostly only touching test > code? -- We can also POC this now to really judge how large such a PR > would get (happy to do it myself), and we could even merge it as part of > the KIP, do the 5.0 change is prepared and reduced to only removing the > `public` modifier on all deprecated constructors. > > > Thoughts? > > > -Matthias > > > On 7/29/26 10:02 AM, Siddhartha Devineni wrote: > > Hello all, > > > > I have updated the KIP. > > Please have a look: > > > https://cwiki.apache.org/confluence/spaces/KAFKA/pages/406623925/KIP-1311+Extract+minimal+public+Time+interface+from+internal+Time+API > > > > Thanks, > > Siddhartha > > > > On Tue, Jul 21, 2026 at 11:32 AM Siddhartha Devineni < > > [email protected]> wrote: > > > >> Sure and i will let you all know when it's done > >> > >> On Tue, 21 Jul 2026, 10:13 Chia-Ping Tsai, <[email protected]> wrote: > >> > >>> Yes, Sean’s approach LGTM > >>> > >>> Would you mind updating the KIP? > >>> > >>> Thanks! > >>> > >>>> Siddhartha Devineni <[email protected]> 於 2026年7月21日 > >>> 下午3:21 寫道： > >>>> > >>>> Hi Chia-Ping, > >>>> > >>>> You are right that @SuppressWarnings is just a temporary workaround. > >>>> The fundamental issue remains: in version 5.0, when constructors > become > >>>> package-private, internal Kafka code in other packages (KafkaProducer, > >>>> KafkaAdminClient, etc.) will lose access. > >>>> > >>>> Given these constraints, Sean's minimal public Time interface seems to > >>> be > >>>> the cleanest solution. > >>>> > >>>> Thanks. > >>>> > >>>>> On Tue, Jul 21, 2026 at 8:25 AM Chia-Ping Tsai <[email protected]> > >>> wrote: > >>>>> > >>>>> hi Siddhartha > >>>>> > >>>>> I might be misunderstanding the approach of this PR. The > >>>>> @SuppressWarnings("deprecation") annotation is just a temporary > >>> workaround > >>>>> for now, right? We will eventually face the same issue in version > 5.0: > >>> how > >>>>> to create a Metrics instance with a specific Time object from another > >>>>> package. > >>>>> > >>>>> Best, > >>>>> Chia-Ping > >>>>> > >>>>>> On 2026/07/20 21:34:15 Siddhartha Devineni wrote: > >>>>>> Hi Chia-Ping and Sean, > >>>>>> > >>>>>> To clarify the earlier discussion, after further investigation, > >>> "Metrics" > >>>>>> doesn't call any "Time" methods directly, rather it just stores and > >>>>> passes > >>>>>> it to internal components (this.time = time on line 174). > >>>>>> > >>>>>> This means no officially public Javadoc class actually needs to call > >>> any > >>>>>> "Time" methods. > >>>>>> > >>>>>> So, we could simply: > >>>>>> 1. Deprecate Time-accepting constructors in "Metrics" and > >>> "KafkaStreams" > >>>>>> 2. Keep "Time" as internal API > >>>>>> 3. No new public interface needed > >>>>>> > >>>>>> I have already created a PR implementing this approach after > >>>>>> withdrawing the KIP > >>>>>> > >>>>>> https://github.com/apache/kafka/pull/22689 > >>>>>> > >>>>>> WDYT? > >>>>>> > >>>>>> Thanks and Best regards, > >>>>>> Siddhartha > >>>>>> > >>>>>> On Mon, Jul 20, 2026 at 8:02 PM Chia-Ping Tsai <[email protected] > > > >>>>> wrote: > >>>>>> > >>>>>>> hi Alieh > >>>>>>> > >>>>>>> We could keep discussing on this mail thread. > >>>>>>> > >>>>>>> The solution provided by Sean is pretty good. Except for > >>> KafkaStreams, > >>>>> the > >>>>>>> others only use the `milliseconds` so we could have a new simple > >>>>> interface > >>>>>>> Time, which could be located at org.apache.kafka.common, to replace > >>>>> origin > >>>>>>> Time-accepting constructor > >>>>>>> > >>>>>>> Best, > >>>>>>> Chia-Ping > >>>>>>> > >>>>>>> On 2026/07/20 14:27:00 Alieh Saeedi via dev wrote: > >>>>>>>> Hi > >>>>>>>> > >>>>>>>> Why is the KIP marked as withdrawn if the discussion is still > >>>>> ongoing in > >>>>>>>> the same thread? > >>>>>>>> > >>>>>>>> -Alieh > >>>>>>>> > >>>>>>>> On Mon, Jul 20, 2026 at 3:31 PM Sean Quah via dev < > >>>>> [email protected]> > >>>>>>>> wrote: > >>>>>>>> > >>>>>>>>> Hi, > >>>>>>>>> > >>>>>>>>> I was hoping we could avoid making Time public. Failing that, is > it > >>>>>>>>> possible to reduce the public API surface further? I looked at > >>>>> Metrics > >>>>>>>>> before and it only wanted the wall clock (milliseconds()). > Perhaps > >>>>>>>>> the other constructors are the same? > >>>>>>>>> We could perhaps have a very simple public Time interface only > >>>>> exposing > >>>>>>>>> milliseconds (effectively a wall clock interface) and an internal > >>>>> Time > >>>>>>>>> interface which extends it with other methods. > >>>>>>>>> > >>>>>>>>> Thanks, > >>>>>>>>> Sean > >>>>>>>>> > >>>>>>>>> On Mon, Jul 20, 2026 at 8:51 AM Chia-Ping Tsai < > >>>>> [email protected]> > >>>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> hi all, > >>>>>>>>>> > >>>>>>>>>> I re-read the constructors, and I think deprecating the > >>>>>>> time-accepting > >>>>>>>>>> constructors will introduce huge changes to the codebase. > >>>>>>>>>> > >>>>>>>>>> Maybe we could just make Time public with a few methods, such as > >>>>>>>>>> milliseconds, nanoseconds, and sleep. Since Timer is not public, > >>>>> we > >>>>>>> could > >>>>>>>>>> add a helper method to Timer, like Timer.create(Time, ...), to > >>>>>>> replace > >>>>>>>>>> Time#timer(). > >>>>>>>>>> > >>>>>>>>>> WDYT? > >>>>>>>>>> > >>>>>>>>>> On 2026/04/23 20:57:19 Siddhartha Devineni wrote: > >>>>>>>>>>> Hi Chia-Ping, Kirk and Matthias, > >>>>>>>>>>> > >>>>>>>>>>> @Chia-Ping: You were right - after investigating, Time doesn't > >>>>>>> need to > >>>>>>>>> be > >>>>>>>>>>> public. > >>>>>>>>>>> > >>>>>>>>>>> @Kirk: You are right - the OAuth examples are instantiated via > >>>>>>>>>> reflection, > >>>>>>>>>>> not direct user code. > >>>>>>>>>>> > >>>>>>>>>>> @Matthias: Good points. I investigated whether Metrics can be > >>>>>>> changed > >>>>>>>>> to > >>>>>>>>>>> not expose Time. > >>>>>>>>>>> > >>>>>>>>>>> Findings: > >>>>>>>>>>> > >>>>>>>>>>> - new Metrics() - users call this (internally uses Time.SYSTEM) > >>>>>>>>>>> - new Metrics(Time time) and other variants - only called by > >>>>>>> internal > >>>>>>>>>> Kafka > >>>>>>>>>>> code (KafkaProducer, KafkaAdminClient, etc) and tests > >>>>>>>>>>> > >>>>>>>>>>> Proposed approach: > >>>>>>>>>>> > >>>>>>>>>>> - Withdraw KIP-1311 (Make Time public) > >>>>>>>>>>> - Create JIRA: "Deprecate Time-accepting constructors" > >>>>>>>>>>> - Deprecate Time constructors in both KafkaStreams and Metrics: > >>>>>>>>>>> - KafkaStreams(Topology, Properties, Time) > >>>>>>>>>>> - KafkaStreams(Topology, StreamsConfig, Time) > >>>>>>>>>>> - KafkaStreams(Topology, Properties, KafkaClientSupplier, > >>>>>>> Time) > >>>>>>>>>>> - Metrics(Time) > >>>>>>>>>>> - Metrics(MetricConfig, Time) > >>>>>>>>>>> - Metrics(MetricConfig, List<MetricsReporter>, Time) > >>>>>>>>>>> - (and other Metrics variants accepting Time) > >>>>>>>>>>> - In version 5.0, make these constructors package-private > >>>>>>>>>>> - Internal Kafka code continues using them > >>>>>>>>>>> > >>>>>>>>>>> Result: Time remains internal. > >>>>>>>>>>> > >>>>>>>>>>> Does this approach work? If so, I'll withdraw KIP-1311 and > >>>>> create > >>>>>>> the > >>>>>>>>>> JIRA. > >>>>>>>>>>> > >>>>>>>>>>> Thank you, > >>>>>>>>>>> Siddhartha > >>>>>>>>>>> > >>>>>>>>>>> On Tue, Apr 21, 2026 at 5:22 AM Matthias J. Sax < > >>>>> [email protected]> > >>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Thanks for the KIP. I am not sure if I understand why > >>>>>>>>>> `KafkaStreamsMock` > >>>>>>>>>>>> would be anything public? > >>>>>>>>>>>> > >>>>>>>>>>>> Also, why would we put it into some new `...test...` > >>>>> package? If > >>>>>>> we > >>>>>>>>>>>> change the package, we need to have `protected` access, > >>>>> which is > >>>>>>>>>> already > >>>>>>>>>>>> "semi-public"... > >>>>>>>>>>>> > >>>>>>>>>>>> If we want to keep `Time` internal, we would eventually make > >>>>> the > >>>>>>>>>>>> constructors that are marked deprecated, package-private, > >>>>> what > >>>>>>> allows > >>>>>>>>>> us > >>>>>>>>>>>> to add `org.apache.kafka.streams.KafkaStreamsMock` (same > >>>>> package > >>>>>>>>> name, > >>>>>>>>>>>> but int `test/` module) to still use these constructors, and > >>>>> the > >>>>>>>>>>>> corresponding unit test would use the new mock-factory > >>>>> instead of > >>>>>>>>>>>> calling `new`? > >>>>>>>>>>>> > >>>>>>>>>>>> For this case, the KIP does not need to mention anything > >>>>> about > >>>>>>>>>>>> `KafkaStreamsMock` as it's an helper in our `test/` module > >>>>> only, > >>>>>>> but > >>>>>>>>>> not > >>>>>>>>>>>> public API. -- If we want, we can still mention this plan on > >>>>> the > >>>>>>> KIP, > >>>>>>>>>>>> but atm the KIP is written in a way as if `KafkaStreamsMock` > >>>>>>> would > >>>>>>>>>>>> become public API, but to my understanding it should be an > >>>>>>>>> impl/testing > >>>>>>>>>>>> details only? > >>>>>>>>>>>> > >>>>>>>>>>>> Or did I misunderstand something? > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> Also wondering, if we could also change `Metrics` in a way, > >>>>> that > >>>>>>> we > >>>>>>>>>>>> would not need to make `Time` public to begin with? > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> -Matthias > >>>>>>>>>>>> > >>>>>>>>>>>> On 4/20/26 4:53 PM, Kirk True wrote: > >>>>>>>>>>>>> Hi Siddhartha, > >>>>>>>>>>>>> > >>>>>>>>>>>>> The OAuth examples use Time in their constructors for unit > >>>>>>> tests. > >>>>>>>>>>>> They're not intended to be instantiated by any user code > >>>>> since > >>>>>>>>> they're > >>>>>>>>>> in > >>>>>>>>>>>> an internals package. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>> Kirk > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Wed, Apr 15, 2026, at 8:52 AM, Siddhartha Devineni > >>>>> wrote: > >>>>>>>>>>>>>> Hi Chia-Ping, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Sorry that i didn't mention the following examples in the > >>>>> KIP > >>>>>>>>>> earlier. > >>>>>>>>>>>>>> Now, I have updated the KIP with the following public > >>>>> packages > >>>>>>>>>> examples, > >>>>>>>>>>>>>> where "Time" is exposed in the public constructors: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> // couple of examples from multiple > >>>>>>>>>>>>>> "org.apache.kafka.common.metrics.Metrics.java" > >>>>> constructors > >>>>>>>>>>>>>> public Metrics(Time time) {} > >>>>>>>>>>>>>> public Metrics(MetricConfig defaultConfig, Time time) {} > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> // in the public package > >>>>>>>>>> "org.apache.kafka.common.security.oauthbearer" > >>>>>>>>>>>>>> public JwtBearerJwtRetriever(Time time) {} > >>>>>>>>>>>>>> public ClientCredentialsJwtRetriever(Time time) {} > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Now, it should be clear. > >>>>>>>>>>>>>> Thanks for your time. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Best regards, > >>>>>>>>>>>>>> Siddhartha > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Tue, Apr 14, 2026 at 11:14 AM Chia-Ping Tsai < > >>>>>>>>> [email protected] > >>>>>>>>>>> > >>>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> hi Siddhartha > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thanks for this KIP. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> What is the exact benefit of exposing Time as a public > >>>>> API? > >>>>>>> Since > >>>>>>>>>> this > >>>>>>>>>>>> KIP > >>>>>>>>>>>>>>> proposes deprecating KafkaStreams(Topology, Properties, > >>>>>>> Time), it > >>>>>>>>>> seems > >>>>>>>>>>>>>>> there are no public interfaces relying on it anymore. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thus, it should be fine to just keep Time as an internal > >>>>> API, > >>>>>>>>>> right? > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>> Chia-Ping > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Siddhartha Devineni <[email protected]> 於 > >>>>>>>>> 2026年4月7日週二 > >>>>>>>>>>>>>>> 下午2:19寫道： > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Apologies, as I forgot to add the link to the KIP: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>> > >>>>> > >>> > https://urldefense.com/v3/__https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=406623925__;!!Ayb5sqE7!o-ar2zzpALvIzp5wF7s2E77bUw9C8CxXLU1JxqxsiiliVfUFzl_M8gIOcFoy3T1nFx5L78W0vA8d7XV_enMH$ > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Tue, Apr 7, 2026 at 9:13 AM Siddhartha Devineni < > >>>>>>>>>>>>>>>> [email protected]> wrote: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Hello everyone, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> I would like to start a discussion on [DISCUSS] > >>>>> KIP-1311: > >>>>>>> Make > >>>>>>>>>>>>>>> Time/Timer > >>>>>>>>>>>>>>>>> public API. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Following KIP-1247 (Make Bytes part of public API), the > >>>>>>> Time > >>>>>>>>>>>> interface > >>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>> Timer class are the next candidates from > >>>>>>>>>>>>>>> "org.apache.kafka.common.utils" > >>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>> be made officially public. Time is currently exposed > >>>>>>> through > >>>>>>>>>> public > >>>>>>>>>>>>>>> APIs > >>>>>>>>>>>>>>>>> (e.g., in clients, KafkaStreams constructors, etc) but > >>>>> not > >>>>>>>>>> officially > >>>>>>>>>>>>>>>>> designated as a public API. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> An earlier version of this KIP explored splitting Time > >>>>> into > >>>>>>>>>> focused > >>>>>>>>>>>>>>>>> interfaces (Clock, MonotonicClock, etc.), but this > >>>>> would > >>>>>>>>> require > >>>>>>>>>>>>>>>> rewriting > >>>>>>>>>>>>>>>>> thousands of method signatures across the Kafka > >>>>> codebase. > >>>>>>> The > >>>>>>>>>> simpler > >>>>>>>>>>>>>>>>> approach of making Time public as-is seems more > >>>>>>> appropriate to > >>>>>>>>>> avoid > >>>>>>>>>>>>>>>>> breaking changes. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Looking forward to your feedback. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Thank you, > >>>>>>>>>>>>>>>>> Siddhartha > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>>> > >>>>> > >>> > >> > > > >
