ClickHouse client v0.8 instrumentation

[mznet]

• Since many variables in Client class are private, it wasn’t possible to extract the exact insert query from the insert method to db.statement, so I did not implement input method tracing.

• Since ClickHouse Java Client 0.8, the V2 interface seems to have stabilized, so I assumed support beginning with 0.8.

• For the V1, tracing can still be achieved through the existing ClickHouse instrumentation.

resolve #14364

[laurit]

usually we keep advice classes as nested classes of the instrumentation class

[laurit]

is this class used outside of the package it is in?

[mznet]

@laurit no, ClickHouseNetworkAttributesGetter is only used inside of the package, public accessor is not needed.

[laurit]

Generally the semantic conventions require that attributes correspond to the server that was connected. If the api allows specifying multiple servers you will need to determine the exact server that was used or not fill the attribute at all. Please verify that what you are doing here is correct. Also note that usually we use https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation-api-incubator/src/main/java/io/opentelemetry/instrumentation/api/incubator/semconv/net/internal/UrlParser.java to parse urls.

[mznet]

@laurit The Client class is using the first endpoint of the connected endpoints to send its request. If it would be nice, it's possible to use getNextAliveNode method of the Client class to get the valid endpoint, but their methods are mostly private, there are no public methods to do that. Instead I emulated its private method to get the endpoint.

[laurit]

https://clickhouse.com/docs/integrations/language-clients/java/client says Currently only one endpoint is supported You could add a comment about that.

[laurit]

since these are always the same you could have an override for attributeAssertions that takes the 2 parameters that change and calls the attributeAssertions that takes 5 parameters

[mznet]

@laurit If we follow your suggestion, the code would need to be changed as below:

public final class ClickHouseAttributeAssertions {

  private static String dbName;
  private static String host;
  private static int port;

  public static void init(String dbName, String host, int port) {
    ClickHouseAttributeAssertions.dbName = dbName;
    ClickHouseAttributeAssertions.host = host;
    ClickHouseAttributeAssertions.port = port;
  }

  public static List<AttributeAssertion> attributeAssertions(String statement, String operation) {
    return attributeAssertions(dbName, host, port, statement, operation);
  }

  public static List<AttributeAssertion> attributeAssertions(
      String dbName, String host, int port, String statement, String operation) {
      return ...
  }

  private ClickHouseAttributeAssertions() {}
}

However, if we write the code like this, we have to call the init method before the test code starts to set dbName, host, and port into the ClickHouseAttributeAssertions static variables.
This adds an extra step that must be performed before using attributeAssertions method, which makes it a bit cumbersome.