fix: 404 for /topics connect endpoint logs a warn instead of showing up in CLI

[Gerrrr]

Description

The /topics connector endpoint is relatively new in CP. This endpoint is exercised by ksqlDB as part of DESCRIBE CONNECTOR. If the endpoint is not available (i.e., a 404 response is returned), ksql CLI shows a warning right after the command output. For example:

ksql> DESCRIBE CONNECTOR "jdbc-connector";

Name                : jdbc-connector
Class                : io.confluent.connect.jdbc.JdbcSourceConnector
Type                 : source
State                : RUNNING
WorkerId             : 192.168.1.118:8083
WARNING: Could not list related topics due to error: {"error_code":404,"message":"HTTP 404 Not Found"}

Instead of showing this warning in the CLI output, this PR forwards it to the server log. As a result, the server log contains 2 warnings for the not found endpoint - a generic one from the connect client as well as a specific entry from the DescribeConnectorExecutor:

[2021-12-07 14:57:14,646] INFO Received: KsqlRequest{ksql='DESCRIBE CONNECTOR "jdbc-connector";', configOverrides={}, requestProperties={}, sessionVariables={}, commandSequenceNumber=Optional[-1]} (io.confluent.ksql.rest.server.resources.KsqlResource:262)
[2021-12-07 14:57:14,654] INFO Query created (cf58a0d2-18ce-32ae-b58b-7660bd7cf2f8): DESCRIBE CONNECTOR connector; (io.confluent.ksql.logging.query.QueryLogger:68)
[2021-12-07 14:57:14,698] WARN Could not query topics of connector jdbc-connector: {"error_code":404,"message":"HTTP 404 Not Found"} (io.confluent.ksql.services.DefaultConnectClient:273)
[2021-12-07 14:57:14,699] WARN Could not list related topics due to error: {"error_code":404,"message":"HTTP 404 Not Found"} (io.confluent.ksql.rest.server.execution.DescribeConnectorExecutor:109)
[2021-12-07 14:57:14,699] INFO Processed successfully: KsqlRequest{ksql='DESCRIBE CONNECTOR "jdbc-connector";', configOverrides={}, requestProperties={}, sessionVariables={}, commandSequenceNumber=Optional[-1]} (io.confluent.ksql.rest.server.resources.KsqlResource:316)

In my opinion, having 2 similar log entries is OK in this case. We do want the users to have a way to find out why they don't see the topics information. Removing the additional log in DescribeConnectorExecutor would make an implicit dependency between this class and DefaultConnectClient. Given that we do not usually unit test for log entries, I'd say that little duplication is the lesser evil.

Testing done

I tested the change manually. I started CP, PostgreSQL, ksqlDB, then created a JDBC connector:

ksql> CREATE SOURCE CONNECTOR `jdbc-connector` WITH(
>  "connector.class"='io.confluent.connect.jdbc.JdbcSourceConnector',
>  "connection.url"='jdbc:postgresql://localhost:5432/postgres',
>  "mode"='bulk',
>  "topic.prefix"='jdbc-',
>  "key"='username');

 Message
----------------------------------
 Created connector jdbc-connector

Then ran

ksql> DESCRIBE CONNECTOR "jdbc-connector";

Name               : jdbc-connector
Class                : io.confluent.connect.jdbc.JdbcSourceConnector
Type                 : source
State                : RUNNING
WorkerId          : 192.168.1.118:8083
WARNING: Could not list related topics due to error: {"error_code":404,"message":"HTTP 404 Not Found"}

Applied the change and re-ran the last command:

ksql> DESCRIBE CONNECTOR "jdbc-connector";

Name               : jdbc-connector
Class                : io.confluent.connect.jdbc.JdbcSourceConnector
Type                 : source
State                : RUNNING
WorkerId          : 192.168.1.118:8083

Then checked that there is a new entry in the server logs:

[2021-12-07 14:57:14,699] WARN Could not list related topics due to error: {"error_code":404,"message":"HTTP 404 Not Found"} (io.confluent.ksql.rest.server.execution.DescribeConnectorExecutor:109)

Reviewer checklist

• Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
• Ensure relevant issues are linked (description should include text like "Fixes #")

[vcrfxia]

Thanks for the quick fix, @Gerrrr ! Any chance we can add a quick unit test to prevent regressions on this new behavior?

[vcrfxia]

Can we add a quick comment in the code about why we handle the 404 case differently? I can imagine future readers seeing this and being confused.

[Gerrrr]

Done in 9eda284.

[Gerrrr]

@vcrfxia I added a test case in 64ff933.

[vcrfxia]

Yup, just wanted to see the unit test. Thanks a bunch, @Gerrrr !