I found another data point in favor of moving in this direction. Our
protocol spec says the following about the client_version field:

message CommandConnect {
"client_version" : "Pulsar-Client-Java-v1.15.2",
"auth_method_name" : "my-authentication-plugin",
"auth_data" : "my-auth-data",
"protocol_version" : 6
}

* `client_version`: String-based identifier. Format is not enforced.

https://github.com/apache/pulsar-site/blame/main/docs/developing-binary-protocol.md#L133

I think it makes sense to make the format
"pulsar-<language>-<version>" for official clients. We could then
recommend third party clients replace "pulsar" with a relevant org
name. There isn't a way to enforce the version string though, so these
would only be recommendations.

For official clients, here are our version strings:

Go:
ClientVersionString = "Pulsar Go " + Version
https://github.com/apache/pulsar-client-go/blob/dedbdc45c63b06e6a12356785418cd906d6bab3c/pulsar/internal/version.go#L43

C++
connect->set_client_version(PULSAR_VERSION_STR);
https://github.com/apache/pulsar-client-cpp/blob/96507cecdc90f10eca5febc48a0623f72d338615/lib/Commands.cc#L269
std::string version = std::string("Pulsar-CPP-v") + PULSAR_VERSION_STR;
https://github.com/apache/pulsar-client-cpp/blob/05807bdaf3a4341b22efd7c71f7b00c47cc31413/lib/HTTPLookupService.cc#L195

I took a quick survey of some existing third party clients. Here are
the results for how the version string is written:

Rust:
client_version: String::from("2.0.1-incubating"),
https://github.com/streamnative/pulsar-rs/blob/eb87ef796bd8cb42fc72bb72ed14751fffa437e1/src/connection.rs#L1173

Haskell:
& F.clientVersion .~ "Pulsar-Client-Haskell-v" <> T.pack (showVersion version)
https://github.com/cr-org/supernova/blob/602409a18f47a38541ba24f5e885199efd383f48/lib/src/Pulsar/Protocol/Commands.hs#L22

PHP:
$cmd->setClientVersion(sprintf('ikilobyte/pulsar-client-php@v%s',
Client::VERSION_ID));
https://github.com/ikilobyte/pulsar-client-php/blob/e8847c62d3bf136886312a14a04f51c59ca99886/src/IO/StreamIO.php#L77

F#:
I'm not exactly sure how to interpret this input.
https://github.com/fsprojects/pulsar-client-dotnet/blob/5c6dcff0184f87005f55dcc847893a889aaee2ad/src/Pulsar.Client/Internal/ClientCnx.fs#L119

Scala:
All appear to wrap the Pulsar Java client (or don't have a
clientVersion string in their code base).

Thanks,
Michael

On Thu, Feb 23, 2023 at 11:39 AM Michael Marshall <mmarsh...@apache.org> wrote:
>
> > 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