Skip to content

Commit ddc415f

Browse files
committed
Refactor: replace ClientExplicit+bool with *bool tristate for EnableTelemetry
nil = unset (server flag decides), &true = client opt-in, &false = client opt-out. Eliminates the two-field smell; isTelemetryEnabled checks != nil to detect whether the client made an explicit choice, avoiding the need for a companion flag. Only two cases enable telemetry: client DSN=true (priority 1) or server flag enabled with no client override (priority 2). Co-authored-by: Isaac
1 parent c5c46ea commit ddc415f

4 files changed

Lines changed: 126 additions & 302 deletions

File tree

telemetry/config.go

Lines changed: 16 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,11 @@ type Config struct {
1212
// Enabled controls whether telemetry is active
1313
Enabled bool
1414

15-
// EnableTelemetry indicates user wants telemetry enabled.
16-
// Follows client > server > default priority.
17-
EnableTelemetry bool
18-
19-
// ClientExplicit is true when the client explicitly set EnableTelemetry via DSN.
20-
// When true, EnableTelemetry is used directly (overrides the server feature flag).
21-
// When false (default), the server feature flag controls enablement.
22-
ClientExplicit bool
15+
// EnableTelemetry is a tristate for the client DSN setting:
16+
// nil — not set by the client; server feature flag controls enablement
17+
// &true — client explicitly opted in (overrides server flag)
18+
// &false— client explicitly opted out (overrides server flag)
19+
EnableTelemetry *bool
2320

2421
// BatchSize is the number of metrics to batch before flushing
2522
BatchSize int
@@ -44,12 +41,11 @@ type Config struct {
4441
}
4542

4643
// DefaultConfig returns default telemetry configuration.
47-
// Note: Telemetry is disabled by default. The default will remain false until
48-
// server-side feature flags are wired in to control the rollout.
44+
// EnableTelemetry is nil (unset): the server feature flag controls enablement.
4945
func DefaultConfig() *Config {
5046
return &Config{
5147
Enabled: false,
52-
EnableTelemetry: false,
48+
EnableTelemetry: nil, // unset — server feature flag decides
5349
BatchSize: 100,
5450
FlushInterval: 5 * time.Second,
5551
MaxRetries: 3,
@@ -66,8 +62,7 @@ func ParseTelemetryConfig(params map[string]string) *Config {
6662

6763
if v, ok := params["enableTelemetry"]; ok {
6864
if b, err := strconv.ParseBool(v); err == nil {
69-
cfg.EnableTelemetry = b
70-
cfg.ClientExplicit = true // client explicitly set via DSN
65+
cfg.EnableTelemetry = &b // non-nil: client explicitly set via DSN
7166
}
7267
}
7368

@@ -98,38 +93,20 @@ func ParseTelemetryConfig(params map[string]string) *Config {
9893
return cfg
9994
}
10095

101-
// isTelemetryEnabled checks if telemetry should be enabled for this connection.
102-
// Implements the priority-based decision tree for telemetry enablement.
103-
//
104-
// Priority (highest to lowest):
105-
// 1. Client DSN setting (enableTelemetry=true|false) — overrides server when ClientExplicit=true
106-
// 2. Server feature flag (databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver)
107-
// 3. Default: disabled
96+
// isTelemetryEnabled returns true in exactly two cases:
97+
// 1. The client explicitly set enableTelemetry=true in the DSN.
98+
// 2. The client did not set enableTelemetry and the server feature flag is enabled
99+
// (databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver).
108100
//
109-
// Parameters:
110-
// - ctx: Context for the request
111-
// - cfg: Telemetry configuration
112-
// - host: Databricks host to check feature flags against
113-
// - driverVersion: Driver version string
114-
// - httpClient: HTTP client for making feature flag requests
115-
//
116-
// Returns:
117-
// - bool: true if telemetry should be enabled, false otherwise
101+
// In all other cases — explicit opt-out or server flag absent/unreachable — returns false.
118102
func isTelemetryEnabled(ctx context.Context, cfg *Config, host string, driverVersion string, httpClient *http.Client) bool {
119-
// Priority 1: Client DSN setting overrides server when explicitly set
120-
// enableTelemetry=true → enabled (no server check)
121-
// enableTelemetry=false → disabled (no server check)
122-
if cfg.ClientExplicit {
123-
return cfg.EnableTelemetry
103+
if cfg.EnableTelemetry != nil {
104+
return *cfg.EnableTelemetry
124105
}
125106

126-
// Priority 2: Check server-side feature flag (no explicit client setting)
127-
flagCache := getFeatureFlagCache()
128-
serverEnabled, err := flagCache.isTelemetryEnabled(ctx, host, driverVersion, httpClient)
107+
serverEnabled, err := getFeatureFlagCache().isTelemetryEnabled(ctx, host, driverVersion, httpClient)
129108
if err != nil {
130-
// Priority 3: Default disabled on server error
131109
return false
132110
}
133-
134111
return serverEnabled
135112
}

0 commit comments

Comments
 (0)