revise Datadog trace sampling configuration (#10151)

* datadog: sample_rate omitted by default

* config: use *float32 with nil instead of float32 with sentinel value

* change some names

* gofmt -s -w internal/ingress/controller/nginx.go

internal/ingress/controller/config/config.go

@@ -706,16 +706,9 @@ type Configuration struct {
 	// Default: nginx.handle
 	DatadogOperationNameOverride string `json:"datadog-operation-name-override"`
 
-	// DatadogPrioritySampling specifies to use client-side sampling
-	// If true disables client-side sampling (thus ignoring sample_rate) and enables distributed
-	// priority sampling, where traces are sampled based on a combination of user-assigned
-	// Default: true
-	DatadogPrioritySampling bool `json:"datadog-priority-sampling"`
-
 	// DatadogSampleRate specifies sample rate for any traces created.
-	// This is effective only when datadog-priority-sampling is false
-	// Default: 1.0
-	DatadogSampleRate float32 `json:"datadog-sample-rate"`
+	// Default: use a dynamic rate instead
+	DatadogSampleRate *float32 `json:"datadog-sample-rate",omitempty`
 
 	// MainSnippet adds custom configuration to the main section of the nginx configuration
 	MainSnippet string `json:"main-snippet"`
@@ -1001,8 +994,7 @@ func NewDefault() Configuration {
 		DatadogEnvironment:                     "prod",
 		DatadogCollectorPort:                   8126,
 		DatadogOperationNameOverride:           "nginx.handle",
-		DatadogSampleRate:                      1.0,
-		DatadogPrioritySampling:                true,
+		DatadogSampleRate:                      nil,
 		LimitReqStatusCode:                     503,
 		LimitConnStatusCode:                    503,
 		SyslogPort:                             514,

internal/ingress/controller/nginx.go

@@ -1011,16 +1011,6 @@ const jaegerTmpl = `{
   }
 }`
 
-const datadogTmpl = `{
-  "service": "{{ .DatadogServiceName }}",
-  "agent_host": "{{ .DatadogCollectorHost }}",
-  "agent_port": {{ .DatadogCollectorPort }},
-  "environment": "{{ .DatadogEnvironment }}",
-  "operation_name_override": "{{ .DatadogOperationNameOverride }}",
-  "sample_rate": {{ .DatadogSampleRate }},
-  "dd.priority.sampling": {{ .DatadogPrioritySampling }}
-}`
-
 const otelTmpl = `
 exporter = "otlp"
 processor = "batch"
@@ -1044,37 +1034,65 @@ ratio = {{ .OtelSamplerRatio }}
 parent_based = {{ .OtelSamplerParentBased }}
 `
 
-func createOpentracingCfg(cfg ngx_config.Configuration) error {
-	var tmpl *template.Template
-	var err error
+func datadogOpentracingCfg(cfg ngx_config.Configuration) (string, error) {
+	m := map[string]interface{}{
+		"service":                 cfg.DatadogServiceName,
+		"agent_host":              cfg.DatadogCollectorHost,
+		"agent_port":              cfg.DatadogCollectorPort,
+		"environment":             cfg.DatadogEnvironment,
+		"operation_name_override": cfg.DatadogOperationNameOverride,
+	}
 
-	if cfg.ZipkinCollectorHost != "" {
-		tmpl, err = template.New("zipkin").Parse(zipkinTmpl)
-		if err != nil {
-			return err
-		}
-	} else if cfg.JaegerCollectorHost != "" || cfg.JaegerEndpoint != "" {
-		tmpl, err = template.New("jaeger").Parse(jaegerTmpl)
-		if err != nil {
-			return err
-		}
-	} else if cfg.DatadogCollectorHost != "" {
-		tmpl, err = template.New("datadog").Parse(datadogTmpl)
-		if err != nil {
-			return err
-		}
-	} else {
-		tmpl, _ = template.New("empty").Parse("{}")
+	// Omit "sample_rate" if the configuration's sample rate is unset (nil).
+	// Omitting "sample_rate" from the plugin JSON indicates to the tracer that
+	// it should use dynamic rates instead of a configured rate.
+	if cfg.DatadogSampleRate != nil {
+		m["sample_rate"] = *cfg.DatadogSampleRate
+	}
+
+	buf, err := json.Marshal(m)
+	if err != nil {
+		return "", err
+	}
+
+	return string(buf), nil
+}
+
+func opentracingCfgFromTemplate(cfg ngx_config.Configuration, tmplName string, tmplText string) (string, error) {
+	tmpl, err := template.New(tmplName).Parse(tmplText)
+	if err != nil {
+		return "", err
 	}
 
 	tmplBuf := bytes.NewBuffer(make([]byte, 0))
 	err = tmpl.Execute(tmplBuf, cfg)
+	if err != nil {
+		return "", err
+	}
+
+	return tmplBuf.String(), nil
+}
+
+func createOpentracingCfg(cfg ngx_config.Configuration) error {
+	var configData string
+	var err error
+
+	if cfg.ZipkinCollectorHost != "" {
+		configData, err = opentracingCfgFromTemplate(cfg, "zipkin", zipkinTmpl)
+	} else if cfg.JaegerCollectorHost != "" || cfg.JaegerEndpoint != "" {
+		configData, err = opentracingCfgFromTemplate(cfg, "jaeger", jaegerTmpl)
+	} else if cfg.DatadogCollectorHost != "" {
+		configData, err = datadogOpentracingCfg(cfg)
+	} else {
+		configData = "{}"
+	}
+
 	if err != nil {
 		return err
 	}
 
 	// Expand possible environment variables before writing the configuration to file.
-	expanded := os.ExpandEnv(tmplBuf.String())
+	expanded := os.ExpandEnv(configData)
 
 	return os.WriteFile("/etc/nginx/opentracing.json", []byte(expanded), file.ReadWriteByUser)
 }