SNOW-1881874: CLIENT_TELEMETRY_ENABLED is always true and there is no possibility to override it

[semyonmelman]

What is the current behavior?

The issue is that CLIENT_TELEMETRY_ENABLED is always set to TRUE. As a result, the snowflake-jdbc driver will always attempt to send telemetry data, even when it's not possible. When the request fails, telemetry is disabled for the current session.

For example:

 } catch (SnowflakeSQLException e) {
        disableTelemetry(); // when got error like 404 or bad request, disable telemetry in this
        // telemetry instance
        logger.error(
            "Telemetry request failed, response: {}, exception: {}", response, e.getMessage());
        return false;
      } 

The root of the problem lies in the SFStatement class, specifically in the executeQueryInternal function:

 SFBaseResultSet executeQueryInternal(
      String sql,
      Map<String, ParameterBindingDTO> parameterBindings,
      boolean describeOnly,
      boolean internal,
      boolean asyncExec,
      CallingMethod caller,
      ExecTimeTelemetryData execTimeData)
      throws SQLException, SFException { ... }

This function sends a request to Snowflake to fetch parameters. One of these parameters is CLIENT_TELEMETRY_ENABLED, which is always set to TRUE on the server side. I would like the ability to override this behavior.

The result of the request is parsed into a JsonNode, and the resultSet variable already contains CLIENT_TELEMETRY_ENABLED == true.

What is the desired behavior?

It should be possible to disable telemetry, similar to how it works for CLIENT_OUT_OF_BAND_TELEMETRY_ENABLED.

How would this improve snowflake-jdbc?

1. The telemetry behavior would become more intuitive.
2. It would add the ability to disable telemetry using Session.builder().configs(..) .
3. It would align snowflake-jdbc with other frameworks, such as the Snowflake Python Connector.

References, Other Background

A similar issue was previously reported for the Snowflake Python Connector and has been resolved:

Issue: #1902
Fix: PR #2013

[semyonmelman]

I think the simplest way is to make isTelemetryServiceAvailable in TelemetryClient not hardcoded, and receive it from session parameters

[sfc-gh-dszmolka]

hi and thanks for raising this improvement request with us, the team will consider for further planning. If you perhaps feel like submitting a PR too, that's more than appreciated! Otherwise, will keep this thread posted.

[semyonmelman]

Sounds good, I will prepare PR

[sfc-gh-dszmolka]

Thank you for the PR, the team will review.
I don't think this is gonna work this way, as using a Snowflake Session parameter like your implementation, needs the Session parameter to , well, actually exist :) on the Snowflake backend. I don't think we're gonna add that.

But maybe we don't even need to add any new params; the existing ones should be good enough to be exposed to the user and the client driver modify the behaviour based on it. The team will review the PR as mentioned, and/or suggest an alternative approach.

[semyonmelman]

Yep I just realized that its not gonna come from server :) I will bring it through LoginInput

[semyonmelman]

PR ready for review #2034 , added TELEMETRY_SERVICE_AVAILABLE param for session params, and when TELEMETRY_SERVICE_AVAILABLE param is false, telemetry will be disabled

[semyonmelman]

@sfc-gh-dszmolka PR is ready for review, can someone review it?

[sfc-gh-dszmolka]

AFAIK the team goes over the community-submitted PRs periodically, so hopefully it gets reviewed in the coming days. Please note we have absolutely no SLA for PR reviews, to appreciate your patience here.