Skip to content

Commit 3525ca5

Browse files
committed
Align Go driver with JDBC connector-service feature flag endpoint
Updated feature flag fetching to use the same connector-service endpoint as the JDBC driver: - Old: /api/2.0/feature-flags?flags=... - New: /api/2.0/connector-service/feature-flags/OSS_GO_SQL/{version} Changes: - Updated endpoint format to include driver name and version in URL path - Changed response parsing from map format to array of flag entries - Thread driverVersion parameter through the call chain - Updated all tests to match new connector-service response format - Response format: {"flags": [{"name": "...", "value": "true"}]} This aligns the Go driver with the JDBC driver's feature flag API, ensuring consistent behavior across Databricks SQL drivers. Signed-off-by: Samikshya Chand <samikshya.chand@databricks.com> Signed-off-by: samikshya-chand_data <samikshya.chand@databricks.com>
1 parent 9e2ada9 commit 3525ca5

File tree

6 files changed

+83
-99
lines changed

6 files changed

+83
-99
lines changed

telemetry/config.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,11 @@ func ParseTelemetryConfig(params map[string]string) *Config {
9999
// - cfg: Telemetry configuration
100100
// - host: Databricks host to check feature flags against
101101
// - httpClient: HTTP client for making feature flag requests
102+
// - driverVersion: Driver version string for feature flag endpoint
102103
//
103104
// Returns:
104105
// - bool: true if telemetry should be enabled, false otherwise
105-
func isTelemetryEnabled(ctx context.Context, cfg *Config, host string, httpClient *http.Client) bool {
106+
func isTelemetryEnabled(ctx context.Context, cfg *Config, host string, httpClient *http.Client, driverVersion string) bool {
106107
// Priority 1: Client explicitly set (overrides server)
107108
if cfg.EnableTelemetry.IsSet() {
108109
val, _ := cfg.EnableTelemetry.Get()
@@ -111,7 +112,7 @@ func isTelemetryEnabled(ctx context.Context, cfg *Config, host string, httpClien
111112

112113
// Priority 2: Check server-side feature flag
113114
flagCache := getFeatureFlagCache()
114-
serverEnabled, err := flagCache.isTelemetryEnabled(ctx, host, httpClient)
115+
serverEnabled, err := flagCache.isTelemetryEnabled(ctx, host, httpClient, driverVersion)
115116
if err != nil {
116117
// Priority 3: Fail-safe default (disabled)
117118
return false

telemetry/config_test.go

Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package telemetry
22

33
import (
44
"context"
5-
"encoding/json"
65
"net/http"
76
"net/http/httptest"
87
"testing"
@@ -206,12 +205,9 @@ func TestIsTelemetryEnabled_ClientOverrideEnabled(t *testing.T) {
206205
// Setup: Create a server that returns disabled
207206
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
208207
// Server says disabled, but client override should win
209-
resp := map[string]interface{}{
210-
"flags": map[string]bool{
211-
"databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver": false,
212-
},
213-
}
214-
_ = json.NewEncoder(w).Encode(resp)
208+
w.Header().Set("Content-Type", "application/json")
209+
w.WriteHeader(http.StatusOK)
210+
_, _ = w.Write([]byte(`{"flags": [{"name": "databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver", "value": "false"}]}`))
215211
}))
216212
defer server.Close()
217213

@@ -228,7 +224,7 @@ func TestIsTelemetryEnabled_ClientOverrideEnabled(t *testing.T) {
228224
defer flagCache.releaseContext(server.URL)
229225

230226
// Client override should bypass server check
231-
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient)
227+
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient, "1.0.0")
232228

233229
if !result {
234230
t.Error("Expected telemetry to be enabled when client explicitly sets enableTelemetry=true, got disabled")
@@ -240,12 +236,9 @@ func TestIsTelemetryEnabled_ClientOverrideDisabled(t *testing.T) {
240236
// Setup: Create a server that returns enabled
241237
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
242238
// Server says enabled, but client override should win
243-
resp := map[string]interface{}{
244-
"flags": map[string]bool{
245-
"databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver": true,
246-
},
247-
}
248-
_ = json.NewEncoder(w).Encode(resp)
239+
w.Header().Set("Content-Type", "application/json")
240+
w.WriteHeader(http.StatusOK)
241+
_, _ = w.Write([]byte(`{"flags": [{"name": "databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver", "value": "true"}]}`))
249242
}))
250243
defer server.Close()
251244

@@ -261,7 +254,7 @@ func TestIsTelemetryEnabled_ClientOverrideDisabled(t *testing.T) {
261254
flagCache.getOrCreateContext(server.URL)
262255
defer flagCache.releaseContext(server.URL)
263256

264-
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient)
257+
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient, "1.0.0")
265258

266259
if result {
267260
t.Error("Expected telemetry to be disabled when client explicitly sets enableTelemetry=false, got enabled")
@@ -272,12 +265,9 @@ func TestIsTelemetryEnabled_ClientOverrideDisabled(t *testing.T) {
272265
func TestIsTelemetryEnabled_ServerEnabled(t *testing.T) {
273266
// Setup: Create a server that returns enabled
274267
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
275-
resp := map[string]interface{}{
276-
"flags": map[string]bool{
277-
"databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver": true,
278-
},
279-
}
280-
_ = json.NewEncoder(w).Encode(resp)
268+
w.Header().Set("Content-Type", "application/json")
269+
w.WriteHeader(http.StatusOK)
270+
_, _ = w.Write([]byte(`{"flags": [{"name": "databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver", "value": "true"}]}`))
281271
}))
282272
defer server.Close()
283273

@@ -293,7 +283,7 @@ func TestIsTelemetryEnabled_ServerEnabled(t *testing.T) {
293283
flagCache.getOrCreateContext(server.URL)
294284
defer flagCache.releaseContext(server.URL)
295285

296-
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient)
286+
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient, "1.0.0")
297287

298288
if !result {
299289
t.Error("Expected telemetry to be enabled when server flag is true, got disabled")
@@ -304,12 +294,9 @@ func TestIsTelemetryEnabled_ServerEnabled(t *testing.T) {
304294
func TestIsTelemetryEnabled_ServerDisabled(t *testing.T) {
305295
// Setup: Create a server that returns disabled
306296
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
307-
resp := map[string]interface{}{
308-
"flags": map[string]bool{
309-
"databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver": false,
310-
},
311-
}
312-
_ = json.NewEncoder(w).Encode(resp)
297+
w.Header().Set("Content-Type", "application/json")
298+
w.WriteHeader(http.StatusOK)
299+
_, _ = w.Write([]byte(`{"flags": [{"name": "databricks.partnerplatform.clientConfigsFeatureFlags.enableTelemetryForGoDriver", "value": "false"}]}`))
313300
}))
314301
defer server.Close()
315302

@@ -325,7 +312,7 @@ func TestIsTelemetryEnabled_ServerDisabled(t *testing.T) {
325312
flagCache.getOrCreateContext(server.URL)
326313
defer flagCache.releaseContext(server.URL)
327314

328-
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient)
315+
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient, "1.0.0")
329316

330317
if result {
331318
t.Error("Expected telemetry to be disabled when server flag is false, got enabled")
@@ -340,7 +327,7 @@ func TestIsTelemetryEnabled_FailSafeDefault(t *testing.T) {
340327
httpClient := &http.Client{Timeout: 5 * time.Second}
341328

342329
// No server available, should default to disabled (fail-safe)
343-
result := isTelemetryEnabled(ctx, cfg, "nonexistent-host", httpClient)
330+
result := isTelemetryEnabled(ctx, cfg, "nonexistent-host", httpClient, "1.0.0")
344331

345332
if result {
346333
t.Error("Expected telemetry to be disabled when server unavailable (fail-safe), got enabled")
@@ -367,7 +354,7 @@ func TestIsTelemetryEnabled_ServerError(t *testing.T) {
367354
flagCache.getOrCreateContext(server.URL)
368355
defer flagCache.releaseContext(server.URL)
369356

370-
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient)
357+
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient, "1.0.0")
371358

372359
// On error, should default to disabled (fail-safe)
373360
if result {
@@ -390,7 +377,7 @@ func TestIsTelemetryEnabled_ServerUnreachable(t *testing.T) {
390377
flagCache.getOrCreateContext(unreachableHost)
391378
defer flagCache.releaseContext(unreachableHost)
392379

393-
result := isTelemetryEnabled(ctx, cfg, unreachableHost, httpClient)
380+
result := isTelemetryEnabled(ctx, cfg, unreachableHost, httpClient, "1.0.0")
394381

395382
// On error, should default to disabled (fail-safe)
396383
if result {
@@ -418,7 +405,7 @@ func TestIsTelemetryEnabled_ClientOverridesServerError(t *testing.T) {
418405
flagCache.getOrCreateContext(server.URL)
419406
defer flagCache.releaseContext(server.URL)
420407

421-
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient)
408+
result := isTelemetryEnabled(ctx, cfg, server.URL, httpClient, "1.0.0")
422409

423410
// Client override should work even when server errors
424411
if !result {

telemetry/driver_integration.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func InitializeForConnection(
3737
// else: leave unset (will check server feature flag)
3838

3939
// Check if telemetry should be enabled
40-
if !isTelemetryEnabled(ctx, cfg, host, httpClient) {
40+
if !isTelemetryEnabled(ctx, cfg, host, httpClient, driverVersion) {
4141
return nil
4242
}
4343

telemetry/featureflag.go

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func (c *featureFlagCache) releaseContext(host string) {
9090
// getFeatureFlag retrieves a specific feature flag value for the host.
9191
// This is the generic method that handles caching and fetching for any flag.
9292
// Uses cached value if available and not expired.
93-
func (c *featureFlagCache) getFeatureFlag(ctx context.Context, host string, httpClient *http.Client, flagName string) (bool, error) {
93+
func (c *featureFlagCache) getFeatureFlag(ctx context.Context, host string, httpClient *http.Client, flagName string, driverVersion string) (bool, error) {
9494
c.mu.RLock()
9595
flagCtx, exists := c.contexts[host]
9696
c.mu.RUnlock()
@@ -111,7 +111,7 @@ func (c *featureFlagCache) getFeatureFlag(ctx context.Context, host string, http
111111

112112
// If we just created the context, make the initial blocking fetch
113113
if !exists {
114-
flags, err := fetchFeatureFlags(ctx, host, httpClient)
114+
flags, err := fetchFeatureFlags(ctx, host, httpClient, driverVersion)
115115

116116
flagCtx.mu.Lock()
117117
flagCtx.fetching = false
@@ -155,7 +155,7 @@ func (c *featureFlagCache) getFeatureFlag(ctx context.Context, host string, http
155155
flagCtx.mu.RUnlock()
156156

157157
// Fetch fresh values for all flags
158-
flags, err := fetchFeatureFlags(ctx, host, httpClient)
158+
flags, err := fetchFeatureFlags(ctx, host, httpClient, driverVersion)
159159

160160
// Update cache (with proper locking)
161161
flagCtx.mu.Lock()
@@ -184,8 +184,8 @@ func (c *featureFlagCache) getFeatureFlag(ctx context.Context, host string, http
184184

185185
// isTelemetryEnabled checks if telemetry is enabled for the host.
186186
// Uses cached value if available and not expired.
187-
func (c *featureFlagCache) isTelemetryEnabled(ctx context.Context, host string, httpClient *http.Client) (bool, error) {
188-
return c.getFeatureFlag(ctx, host, httpClient, flagEnableTelemetry)
187+
func (c *featureFlagCache) isTelemetryEnabled(ctx context.Context, host string, httpClient *http.Client, driverVersion string) (bool, error) {
188+
return c.getFeatureFlag(ctx, host, httpClient, flagEnableTelemetry, driverVersion)
189189
}
190190

191191
// isExpired returns true if the cache has expired.
@@ -203,36 +203,42 @@ func getAllFeatureFlags() []string {
203203
}
204204
}
205205

206-
// fetchFeatureFlags fetches multiple feature flag values from Databricks in a single request.
206+
// featureFlagEntry represents a single feature flag from the server response.
207+
type featureFlagEntry struct {
208+
Name string `json:"name"`
209+
Value string `json:"value"`
210+
}
211+
212+
// featureFlagsResponse represents the response from the connector-service feature flags endpoint.
213+
type featureFlagsResponse struct {
214+
Flags []featureFlagEntry `json:"flags"`
215+
TTLSeconds *int `json:"ttl_seconds,omitempty"`
216+
}
217+
218+
// fetchFeatureFlags fetches multiple feature flag values from Databricks connector-service endpoint.
207219
// Returns a map of flag names to their boolean values.
208-
func fetchFeatureFlags(ctx context.Context, host string, httpClient *http.Client) (map[string]bool, error) {
220+
func fetchFeatureFlags(ctx context.Context, host string, httpClient *http.Client, driverVersion string) (map[string]bool, error) {
209221
// Add timeout to context if it doesn't have a deadline
210222
if _, hasDeadline := ctx.Deadline(); !hasDeadline {
211223
var cancel context.CancelFunc
212224
ctx, cancel = context.WithTimeout(ctx, featureFlagHTTPTimeout)
213225
defer cancel()
214226
}
215227

216-
// Construct endpoint URL, adding https:// if not already present
228+
// Construct connector-service endpoint URL with driver name and version
229+
// Format: /api/2.0/connector-service/feature-flags/OSS_GO_SQL/{version}
217230
var endpoint string
218231
if strings.HasPrefix(host, "http://") || strings.HasPrefix(host, "https://") {
219-
endpoint = fmt.Sprintf("%s/api/2.0/feature-flags", host)
232+
endpoint = fmt.Sprintf("%s/api/2.0/connector-service/feature-flags/OSS_GO_SQL/%s", host, driverVersion)
220233
} else {
221-
endpoint = fmt.Sprintf("https://%s/api/2.0/feature-flags", host)
234+
endpoint = fmt.Sprintf("https://%s/api/2.0/connector-service/feature-flags/OSS_GO_SQL/%s", host, driverVersion)
222235
}
223236

224237
req, err := http.NewRequestWithContext(ctx, "GET", endpoint, nil)
225238
if err != nil {
226239
return nil, fmt.Errorf("failed to create feature flag request: %w", err)
227240
}
228241

229-
// Add query parameter with comma-separated list of feature flags
230-
// This fetches all flags in a single request for efficiency
231-
allFlags := getAllFeatureFlags()
232-
q := req.URL.Query()
233-
q.Add("flags", strings.Join(allFlags, ","))
234-
req.URL.RawQuery = q.Encode()
235-
236242
resp, err := httpClient.Do(req)
237243
if err != nil {
238244
return nil, fmt.Errorf("failed to fetch feature flags: %w", err)
@@ -245,18 +251,19 @@ func fetchFeatureFlags(ctx context.Context, host string, httpClient *http.Client
245251
return nil, fmt.Errorf("feature flag check failed: %d", resp.StatusCode)
246252
}
247253

248-
var result struct {
249-
Flags map[string]bool `json:"flags"`
250-
}
251-
if err := json.NewDecoder(resp.Body).Decode(&result); err != nil {
254+
var response featureFlagsResponse
255+
if err := json.NewDecoder(resp.Body).Decode(&response); err != nil {
252256
return nil, fmt.Errorf("failed to decode feature flag response: %w", err)
253257
}
254258

255-
// Return the full map of flags
256-
// Flags not present in the response will have false value when accessed
257-
if result.Flags == nil {
258-
return make(map[string]bool), nil
259+
// Convert array of flag entries to map
260+
flags := make(map[string]bool)
261+
if response.Flags != nil {
262+
for _, flag := range response.Flags {
263+
// Parse string value as boolean
264+
flags[flag.Name] = flag.Value == "true"
265+
}
259266
}
260267

261-
return result.Flags, nil
268+
return flags, nil
262269
}

0 commit comments

Comments
 (0)