> A better solution is that we could allow the
> user to specify the suffix of the client version.

I don't think we need to make it configurable because the library
owner, not the user, has full control when creating the Connect
command.

We can update our protocol spec to recommend appropriate usage and of the field.

Thanks,
Michael

On Thu, Feb 23, 2023 at 2:51 AM Zike Yang <z...@apache.org> wrote:
>
> > But I'm wondering if it's possible to support configuring the client
> version string by users?
>
> I think it's possible. A better solution is that we could allow the
> user to specify the suffix of the client version. For example, by
> adding a new configuration like `clientVersionSuffix` to the
> ClientConffiguration, the client_version would be like
> `java-3.0.0-forked-v1.0.0`. In this way, we could always remain the
> part `java-3.0.0`. But of course, we have no control over the behavior
> of third-party clients.
>
>
> Thanks,
> Zike Yang
>
> On Wed, Feb 22, 2023 at 5:52 AM Michael Marshall <mmarsh...@apache.org> wrote:
> >
> > +1 - I think this is a helpful change. We already do this in the Java
> > Admin http client when we set the user agent [0].
> >
> > > But I'm wondering if it's possible to support configuring the client
> > > version string by users?
> >
> > This is always the case because we support third party clients. In my
> > view, this field is primarily helpful for troubleshooting and for
> > getting weakly trusted stats on usage in a given cluster. Operators
> > could even use this information to determine and alert if any clients
> > are connecting with known vulnerabilities.
> >
> > Thanks,
> > Michael
> >
> > [0] 
> > https://github.com/apache/pulsar/blob/d8569cd4ec6da14f8b2b9338db1ed2f6a3eacf0a/pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java#L105
> >
> > On Tue, Feb 21, 2023 at 9:50 AM Yunze Xu <y...@streamnative.io.invalid> 
> > wrote:
> > >
> > > +1
> > >
> > > But I'm wondering if it's possible to support configuring the client
> > > version string by users? For example, if users forked their own
> > > client, they might want to differ the client version with the official
> > > client. The disadvantage could be that users can configure any version
> > > string as they want.
> > >
> > > Thanks,
> > > Yunze
> > >
> > > On Tue, Feb 21, 2023 at 7:23 PM Enrico Olivelli <eolive...@gmail.com> 
> > > wrote:
> > > >
> > > > +1
> > > >
> > > > Enrico
> > > >
> > > > Il giorno mar 21 feb 2023 alle ore 10:23 Baodi Shi <ba...@apache.org>
> > > > ha scritto:
> > > > >
> > > > >  +1, Good idea.
> > > > >
> > > > > Thanks,
> > > > > Baodi Shi
> > > > >
> > > > >
> > > > > 在 2023年2月21日 15:24:45 上,avinash kala <avisu...@gmail.com> 写道:
> > > > >
> > > > > > +1, sounds good.
> > > > > >
> > > > > > On Tue, Feb 21, 2023, 12:39 PM Zixuan Liu <node...@gmail.com> wrote:
> > > > > >
> > > > > > +1, good idea!
> > > > > >
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Zixuan
> > > > > >
> > > > > >
> > > > > > Zike Yang <z...@apache.org> 于2023年2月21日周二 11:33写道:
> > > > > >
> > > > > >
> > > > > > > Hi, all
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > Currently, the Pulsar broker uses the field `client_version` to 
> > > > > > > get
> > > > > >
> > > > > > > the version of the client. The client will send the 
> > > > > > > client_version to
> > > > > >
> > > > > > > the broker through `CommandConnect` [0] during the connect or
> > > > > >
> > > > > > > `CommandAuthResponse ` [1] during the auth challenge. We could 
> > > > > > > get the
> > > > > >
> > > > > > > client_version from the admin using `pulsar-admin topics stats 
> > > > > > > xxx`
> > > > > >
> > > > > > > [2].
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > But the `client_version ` only shows the version information. And
> > > > > >
> > > > > > > clients in different languages use their own separate version 
> > > > > > > numbers.
> > > > > >
> > > > > > > This can lead to conflict. For example, the java client 2.10.2 
> > > > > > > uses
> > > > > >
> > > > > > > the same value `2.10.2` as the C++ client 2.10.2. And C++ client 
> > > > > > > 3.0.0
> > > > > >
> > > > > > > may conflict with the future java client 3.0.0. The information of
> > > > > >
> > > > > > > `client_version` is incomplete. We could not determine what
> > > > > >
> > > > > > > language(or specific library) the client uses. This raises
> > > > > >
> > > > > > > inconvenience for debugging.
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > I would like to propose adding the client language information to 
> > > > > > > the
> > > > > >
> > > > > > > `client_version`. Like:
> > > > > >
> > > > > > > * `java-2.11.1` for Java client 2.11
> > > > > >
> > > > > > > * `cpp-3.1.2` for C++ client 3.1.2
> > > > > >
> > > > > > > * `go-0.9.0` for go client 0.9.0
> > > > > >
> > > > > > > We can easily do that because the `client_version` is a string 
> > > > > > > type.
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > In addition, the Nodejs client and Python client all use the
> > > > > >
> > > > > > > client_version of C++ client they depend on. So it will use 
> > > > > > > `3.1.2` as
> > > > > >
> > > > > > > the client_verion in Nodejs client 1.8.0. This is incorrect, and I
> > > > > >
> > > > > > > think we need to fix them. We don't need to print the C++ client
> > > > > >
> > > > > > > version because we can get that information if we know the Nodejs 
> > > > > > > or
> > > > > >
> > > > > > > Python client version.
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > What do you think? Please share your thoughts if you have any 
> > > > > > > ideas.
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > [0]
> > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > https://github.com/apache/pulsar/blob/e0b50c9ec5f12d0fb8275f235d8ac00e87a9099e/pulsar-common/src/main/proto/PulsarApi.proto#L269
> > > > > >
> > > > > > > [1]
> > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > > https://github.com/apache/pulsar/blob/e0b50c9ec5f12d0fb8275f235d8ac00e87a9099e/pulsar-common/src/main/proto/PulsarApi.proto#L309
> > > > > >
> > > > > > > [2] 
> > > > > > > https://pulsar.apache.org/docs/next/admin-api-topics/#get-stats
> > > > > >
> > > > > > >
> > > > > >
> > > > > > > Thanks,
> > > > > >
> > > > > > > Zike Yang
> > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > >

Reply via email to