hi all, If we want to avoid both "large changes" and "unnecessary public APIs", another option is to introduce a nested @InterfaceAudience.Private class inside the @InterfaceAudience.Public class. I previously shared an example of this approach here: https://github.com/apache/kafka/pull/21337#discussion_r3574009798
The advantages are straightforward, and the only downside is that users must understand what the word "Internal" means. Fortunately, I have high confidence in most developers' vocabulary :) Best, Chia-Ping On 2026/08/01 23:04:02 "Matthias J. Sax" wrote: > Can't we rewrite the package-private constructor to create `TestMetrics` > object instead of `Metrics`? Or instead of passing in `Time`, we pass in > the `Metrics` object itself, and we let the test-code create `new > TestMetrics`? > > > -Matthias > > On 7/31/26 2:48 PM, Siddhartha Devineni wrote: > > 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 > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>> > >>>> > >>> > >> > >> > > > >
