Skip to content

Commit 27e2a0f

Browse files
committed
Remove ForceEnableTelemetry; unify under client > server > default priority
ForceEnableTelemetry was redundant with the opt-in priority logic introduced in PR #319: setting EnableTelemetry=true (client) already takes precedence over server feature flags, so a separate bypass flag was unnecessary. - Remove ForceEnableTelemetry from UserConfig, DeepCopy, and DSN parsing - Simplify connector.go telemetry init to check only EnableTelemetry - Update DESIGN.md to reflect the unified priority model
1 parent 1f0608c commit 27e2a0f

File tree

3 files changed

+14
-38
lines changed

3 files changed

+14
-38
lines changed

connector.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,14 +76,12 @@ func (c *connector) Connect(ctx context.Context) (driver.Conn, error) {
7676
}
7777
log := logger.WithContext(conn.id, driverctx.CorrelationIdFromContext(ctx), "")
7878

79-
// Initialize telemetry (always attempt, let feature flags decide)
79+
// Initialize telemetry: pass user opt-in flag; if unset, feature flags decide
8080
var enableTelemetry *bool
81-
if c.cfg.ForceEnableTelemetry || c.cfg.EnableTelemetry {
82-
// User explicitly enabled telemetry
81+
if c.cfg.EnableTelemetry {
8382
trueVal := true
8483
enableTelemetry = &trueVal
8584
}
86-
// else: leave nil to check server feature flag
8785

8886
conn.telemetry = telemetry.InitializeForConnection(
8987
ctx,

internal/config/config.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,7 @@ type UserConfig struct {
9999
RetryWaitMax time.Duration
100100
RetryMax int
101101
// Telemetry configuration
102-
EnableTelemetry bool // Opt-in for telemetry (respects server feature flags)
103-
ForceEnableTelemetry bool // Force enable telemetry (bypasses server checks)
102+
EnableTelemetry bool // Opt-in for telemetry; follows client > server > default priority
104103
Transport http.RoundTripper
105104
UseLz4Compression bool
106105
EnableMetricViewMetadata bool
@@ -148,7 +147,6 @@ func (ucfg UserConfig) DeepCopy() UserConfig {
148147
EnableMetricViewMetadata: ucfg.EnableMetricViewMetadata,
149148
CloudFetchConfig: ucfg.CloudFetchConfig,
150149
EnableTelemetry: ucfg.EnableTelemetry,
151-
ForceEnableTelemetry: ucfg.ForceEnableTelemetry,
152150
}
153151
}
154152

@@ -295,13 +293,6 @@ func ParseDSN(dsn string) (UserConfig, error) {
295293
ucfg.EnableTelemetry = enableTelemetry
296294
}
297295

298-
if forceEnableTelemetry, ok, err := params.extractAsBool("forceEnableTelemetry"); ok {
299-
if err != nil {
300-
return UserConfig{}, err
301-
}
302-
ucfg.ForceEnableTelemetry = forceEnableTelemetry
303-
}
304-
305296
// for timezone we do a case insensitive key match.
306297
// We use getNoCase because we want to leave timezone in the params so that it will also
307298
// be used as a session param.

telemetry/DESIGN.md

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1440,12 +1440,9 @@ type Config struct {
14401440
// Enabled controls whether telemetry is active
14411441
Enabled bool
14421442

1443-
// ForceEnableTelemetry bypasses server-side feature flag checks
1444-
// When true, telemetry is always enabled regardless of server flags
1445-
ForceEnableTelemetry bool
1446-
1447-
// EnableTelemetry indicates user wants telemetry enabled if server allows
1448-
// Respects server-side feature flags and rollout percentage
1443+
// EnableTelemetry indicates user wants telemetry enabled.
1444+
// Follows client > server > default priority: if set by the client it takes
1445+
// precedence; otherwise the server feature flag and defaults are consulted.
14491446
EnableTelemetry bool
14501447

14511448
// BatchSize is the number of metrics to batch before flushing
@@ -1474,9 +1471,8 @@ type Config struct {
14741471
// Note: Telemetry is disabled by default and requires explicit opt-in.
14751472
func DefaultConfig() *Config {
14761473
return &Config{
1477-
Enabled: false, // Disabled by default, requires explicit opt-in
1478-
ForceEnableTelemetry: false,
1479-
EnableTelemetry: false,
1474+
Enabled: false, // Disabled by default, requires explicit opt-in
1475+
EnableTelemetry: false,
14801476
BatchSize: 100,
14811477
FlushInterval: 5 * time.Second,
14821478
MaxRetries: 3,
@@ -1495,14 +1491,7 @@ func DefaultConfig() *Config {
14951491
func ParseTelemetryConfig(params map[string]string) *Config {
14961492
cfg := DefaultConfig()
14971493

1498-
// Check for forceEnableTelemetry flag (bypasses server feature flags)
1499-
if v, ok := params["forceEnableTelemetry"]; ok {
1500-
if v == "true" || v == "1" {
1501-
cfg.ForceEnableTelemetry = true
1502-
}
1503-
}
1504-
1505-
// Check for enableTelemetry flag (respects server feature flags)
1494+
// Check for enableTelemetry flag (follows client > server > default priority)
15061495
if v, ok := params["enableTelemetry"]; ok {
15071496
if v == "true" || v == "1" {
15081497
cfg.EnableTelemetry = true
@@ -2108,17 +2097,15 @@ func BenchmarkInterceptor_Disabled(b *testing.B) {
21082097

21092098
### Phase 5: Opt-In Configuration Integration ✅ COMPLETED
21102099
- [x] Implement `isTelemetryEnabled()` with priority-based logic in config.go
2111-
- [x] Priority 1: ForceEnableTelemetry=true bypasses all checks → return true
2112-
- [x] Priority 2: EnableTelemetry=false explicit opt-out → return false
2113-
- [x] Priority 3: EnableTelemetry=true + check server feature flag
2114-
- [x] Priority 4: Server-side feature flag only (default behavior)
2115-
- [x] Priority 5: Default disabled if no flags set and server check fails
2100+
- [x] Priority 1 (client): EnableTelemetry=true → enable regardless of server flag
2101+
- [x] Priority 2 (client): EnableTelemetry=false → disable regardless of server flag
2102+
- [x] Priority 3 (server): Server feature flag controls when client preference unset
2103+
- [x] Priority 4 (default): Disabled if no flags set and server check fails
21162104
- [x] Integrate feature flag cache with opt-in logic
21172105
- [x] Wire up isTelemetryEnabled() to call featureFlagCache.isTelemetryEnabled()
21182106
- [x] Implement fallback behavior on errors (return cached value or false)
21192107
- [x] Add proper error handling
21202108
- [x] Add unit tests for opt-in priority logic
2121-
- [x] Test forceEnableTelemetry=true (always enabled, bypasses server)
21222109
- [x] Test enableTelemetry=false (always disabled, explicit opt-out)
21232110
- [x] Test enableTelemetry=true with server flag enabled
21242111
- [x] Test enableTelemetry=true with server flag disabled
@@ -2165,7 +2152,7 @@ func BenchmarkInterceptor_Disabled(b *testing.B) {
21652152
- [x] Increment feature flag cache reference count
21662153
- [x] Store telemetry interceptor in connection
21672154
- [x] Add telemetry configuration to UserConfig
2168-
- [x] EnableTelemetry and ForceEnableTelemetry fields
2155+
- [x] EnableTelemetry field (client > server > default priority)
21692156
- [x] DSN parameter parsing
21702157
- [x] DeepCopy support
21712158
- [x] Add cleanup in `Close()` methods

0 commit comments

Comments
 (0)