Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate distributed tracing flags off experimental prefix #19096

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

migrate distributed tracing flags off experimental prefix #19096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 38 additions & 15 deletions server/embed/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ var (
// This is the mapping from the non boolean `experimental-` to the new flags.
// TODO: delete in v3.7
experimentalNonBoolFlagMigrationMap = map[string]string{
"experimental-compact-hash-check-time": "compact-hash-check-time",
"experimental-compact-hash-check-time": "compact-hash-check-time",
"experimental-distributed-tracing-address": "distributed-tracing-address",
"experimental-distributed-tracing-service-name": "distributed-tracing-service-name",
"experimental-distributed-tracing-instance-id": "distributed-tracing-instance-id",
"experimental-distributed-tracing-sampling-rate": "distributed-tracing-sampling-rate",
}
)

Expand Down Expand Up @@ -408,23 +412,33 @@ type Config struct {
ListenMetricsUrls []url.URL
ListenMetricsUrlsJSON string `json:"listen-metrics-urls"`

// ExperimentalEnableDistributedTracing indicates if experimental tracing using OpenTelemetry is enabled.
// ExperimentalEnableDistributedTracing enables distributed tracing using OpenTelemetry protocol.
// Deprecated in v3.6 and will be decommissioned in v3.7.
// TODO: delete in v3.7
ExperimentalEnableDistributedTracing bool `json:"experimental-enable-distributed-tracing"`
// ExperimentalDistributedTracingAddress is the address of the OpenTelemetry Collector.
// Can only be set if ExperimentalEnableDistributedTracing is true.
// ExperimentalDistributedTracingAddress is the address of OpenTelemetry collector.
// Deprecated in v3.6 and will be decommissioned in v3.7.
// TODO: delete in v3.7
ExperimentalDistributedTracingAddress string `json:"experimental-distributed-tracing-address"`
// ExperimentalDistributedTracingServiceName is the name of the service.
// Can only be used if ExperimentalEnableDistributedTracing is true.
// Deprecated in v3.6 and will be decommissioned in v3.7.
// TODO: delete in v3.7
ExperimentalDistributedTracingServiceName string `json:"experimental-distributed-tracing-service-name"`
// ExperimentalDistributedTracingServiceInstanceID is the ID key of the service.
// This ID must be unique, as helps to distinguish instances of the same service
// that exist at the same time.
// Can only be used if ExperimentalEnableDistributedTracing is true.
// Deprecated in v3.6 and will be decommissioned in v3.7.
// TODO: delete in v3.7
ExperimentalDistributedTracingServiceInstanceID string `json:"experimental-distributed-tracing-instance-id"`
// ExperimentalDistributedTracingSamplingRatePerMillion is the number of samples to collect per million spans.
// Defaults to 0.
// ExperimentalDistributedTracingSamplingRatePerMillion is the number of samples.
// Deprecated in v3.6 and will be decommissioned in v3.7.
// TODO: delete in v3.7
ExperimentalDistributedTracingSamplingRatePerMillion int `json:"experimental-distributed-tracing-sampling-rate"`

EnableDistributedTracing bool `json:"enable-distributed-tracing"`
DistributedTracingAddress string `json:"distributed-tracing-address"`
DistributedTracingServiceName string `json:"distributed-tracing-service-name"`
DistributedTracingServiceInstanceID string `json:"distributed-tracing-instance-id"`
DistributedTracingSamplingRatePerMillion int `json:"distributed-tracing-sampling-rate"`

// Logger is logger options: currently only supports "zap".
// "capnslog" is removed in v3.5.
Logger string `json:"logger"`
Expand Down Expand Up @@ -654,6 +668,11 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
fs.DurationVar(&cfg.GRPCKeepAliveTimeout, "grpc-keepalive-timeout", cfg.GRPCKeepAliveTimeout, "Additional duration of wait before closing a non-responsive connection (0 to disable).")
fs.BoolVar(&cfg.SocketOpts.ReusePort, "socket-reuse-port", cfg.SocketOpts.ReusePort, "Enable to set socket option SO_REUSEPORT on listeners allowing rebinding of a port already in use.")
fs.BoolVar(&cfg.SocketOpts.ReuseAddress, "socket-reuse-address", cfg.SocketOpts.ReuseAddress, "Enable to set socket option SO_REUSEADDR on listeners allowing binding to an address in `TIME_WAIT` state.")
fs.BoolVar(&cfg.EnableDistributedTracing, "enable-distributed-tracing", false, "Enable distributed tracing using OpenTelemetry protocol")
fs.StringVar(&cfg.DistributedTracingAddress, "distributed-tracing-address", ExperimentalDistributedTracingAddress, "Address for distributed tracing used for OpenTelemetry Tracing")
fs.StringVar(&cfg.DistributedTracingServiceName, "distributed-tracing-service-name", ExperimentalDistributedTracingServiceName, "Configures service name for distributed tracing")
fs.StringVar(&cfg.DistributedTracingServiceInstanceID, "distributed-tracing-instance-id", "", "Configures service instance ID for distributed tracing")
fs.IntVar(&cfg.DistributedTracingSamplingRatePerMillion, "distributed-tracing-sampling-rate", 0, "Number of samples to collect per million spans")

fs.Var(flags.NewUint32Value(cfg.MaxConcurrentStreams), "max-concurrent-streams", "Maximum concurrent streams that each client can open at a time.")

Expand Down Expand Up @@ -755,11 +774,11 @@ func (cfg *Config) AddFlags(fs *flag.FlagSet) {
fs.StringVar(&cfg.Metrics, "metrics", cfg.Metrics, "Set level of detail for exported metrics, specify 'extensive' to include server side grpc histogram metrics")

// experimental distributed tracing
fs.BoolVar(&cfg.ExperimentalEnableDistributedTracing, "experimental-enable-distributed-tracing", false, "Enable experimental distributed tracing using OpenTelemetry Tracing.")
fs.StringVar(&cfg.ExperimentalDistributedTracingAddress, "experimental-distributed-tracing-address", ExperimentalDistributedTracingAddress, "Address for distributed tracing used for OpenTelemetry Tracing (if enabled with experimental-enable-distributed-tracing flag).")
fs.StringVar(&cfg.ExperimentalDistributedTracingServiceName, "experimental-distributed-tracing-service-name", ExperimentalDistributedTracingServiceName, "Configures service name for distributed tracing to be used to define service name for OpenTelemetry Tracing (if enabled with experimental-enable-distributed-tracing flag). 'etcd' is the default service name. Use the same service name for all instances of etcd.")
fs.StringVar(&cfg.ExperimentalDistributedTracingServiceInstanceID, "experimental-distributed-tracing-instance-id", "", "Configures service instance ID for distributed tracing to be used to define service instance ID key for OpenTelemetry Tracing (if enabled with experimental-enable-distributed-tracing flag). There is no default value set. This ID must be unique per etcd instance.")
fs.IntVar(&cfg.ExperimentalDistributedTracingSamplingRatePerMillion, "experimental-distributed-tracing-sampling-rate", 0, "Number of samples to collect per million spans for OpenTelemetry Tracing (if enabled with experimental-enable-distributed-tracing flag).")
fs.BoolVar(&cfg.ExperimentalEnableDistributedTracing, "experimental-enable-distributed-tracing", false, "Enable distributed tracing using OpenTelemetry protocol. "+"Deprecated in v3.6 and will be decommissioned in v3.7. Use --enable-distributed-tracing instead.")
fs.StringVar(&cfg.ExperimentalDistributedTracingAddress, "experimental-distributed-tracing-address", ExperimentalDistributedTracingAddress, "Address for distributed tracing. Deprecated in v3.6 and will be decommissioned in v3.7. Use --distributed-tracing-address instead.")
fs.StringVar(&cfg.ExperimentalDistributedTracingServiceName, "experimental-distributed-tracing-service-name", ExperimentalDistributedTracingServiceName, "Service name for distributed tracing. Deprecated in v3.6 and will be decommissioned in v3.7. Use --distributed-tracing-service-name instead.")
fs.StringVar(&cfg.ExperimentalDistributedTracingServiceInstanceID, "experimental-distributed-tracing-instance-id", "", "Service instance ID for distributed tracing. Deprecated in v3.6 and will be decommissioned in v3.7. Use --distributed-tracing-instance-id instead.")
fs.IntVar(&cfg.ExperimentalDistributedTracingSamplingRatePerMillion, "experimental-distributed-tracing-sampling-rate", 0, "Samples per million for distributed tracing. Deprecated in v3.6 and will be decommissioned in v3.7. Use --distributed-tracing-sampling-rate instead.")

// auth
fs.StringVar(&cfg.AuthToken, "auth-token", cfg.AuthToken, "Specify auth token specific options.")
Expand Down Expand Up @@ -1012,6 +1031,10 @@ func (cfg *Config) Validate() error {
}
}

if cfg.FlagsExplicitlySet["experimental-enable-distributed-tracing"] && cfg.FlagsExplicitlySet["enable-distributed-tracing"] {
return fmt.Errorf("cannot set --experimental-enable-distributed-tracing and --enable-distributed-tracing at the same time")
}

if err := cfg.setupLogging(); err != nil {
return err
}
Expand Down
12 changes: 12 additions & 0 deletions server/etcdmain/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ var (
"experimental-compact-hash-check-enabled": "--experimental-compact-hash-check-enabled is deprecated in 3.6 and will be decommissioned in 3.7. Use '--feature-gates=CompactHashCheck=true' instead.",
"experimental-compact-hash-check-time": "--experimental-compact-hash-check-time is deprecated in 3.6 and will be decommissioned in 3.7. Use '--compact-hash-check-time' instead.",
"experimental-txn-mode-write-with-shared-buffer": "--experimental-txn-mode-write-with-shared-buffer is deprecated in v3.6 and will be decommissioned in v3.7. Use '--feature-gates=TxnModeWriteWithSharedBuffer=true' instead.",
"experimental-enable-distributed-tracing": "--experimental-enable-distributed-tracing is deprecated in 3.6 and will be decommissioned in 3.7. Use --enable-distributed-tracing instead.",
"experimental-distributed-tracing-address": "--experimental-distributed-tracing-address is deprecated in 3.6 and will be decommissioned in 3.7. Use --distributed-tracing-address instead.",
"experimental-distributed-tracing-service-name": "--experimental-distributed-tracing-service-name is deprecated in 3.6 and will be decommissioned in 3.7. Use --distributed-tracing-service-name instead.",
"experimental-distributed-tracing-instance-id": "--experimental-distributed-tracing-instance-id is deprecated in 3.6 and will be decommissioned in 3.7. Use --distributed-tracing-instance-id instead.",
"experimental-distributed-tracing-sampling-rate": "--experimental-distributed-tracing-sampling-rate is deprecated in 3.6 and will be decommissioned in 3.7. Use --distributed-tracing-sampling-rate instead.",
}
)

Expand Down Expand Up @@ -195,6 +200,13 @@ func (cfg *config) parse(arguments []string) error {
}
}
}
if cfg.ec.ExperimentalEnableDistributedTracing {
cfg.ec.EnableDistributedTracing = true
cfg.ec.DistributedTracingAddress = cfg.ec.ExperimentalDistributedTracingAddress
cfg.ec.DistributedTracingServiceName = cfg.ec.ExperimentalDistributedTracingServiceName
cfg.ec.DistributedTracingServiceInstanceID = cfg.ec.ExperimentalDistributedTracingServiceInstanceID
cfg.ec.DistributedTracingSamplingRatePerMillion = cfg.ec.ExperimentalDistributedTracingSamplingRatePerMillion
}

// now logger is set up
return err
Expand Down
168 changes: 168 additions & 0 deletions server/etcdmain/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,3 +653,171 @@ func validateClusteringFlags(t *testing.T, cfg *config) {
t.Errorf("advertise-client-urls = %v, want %v", cfg.ec.AdvertiseClientUrls, wcfg.ec.AdvertiseClientUrls)
}
}

// TODO: delete in v3.7
func TestDistributedTracingFlagMigration(t *testing.T) {
testCases := []struct {
name string
args []string // command line arguments
yamlConfig string // YAML configuration content
expectValues func(*config) error // function to verify expected values
expectErr bool
}{
{
name: "use experimental flags",
args: []string{
"--experimental-enable-distributed-tracing=true",
"--experimental-distributed-tracing-address=localhost:4318",
"--experimental-distributed-tracing-service-name=test-service",
"--experimental-distributed-tracing-instance-id=instance-1",
"--experimental-distributed-tracing-sampling-rate=1000",
},
expectValues: func(cfg *config) error {
if !cfg.ec.EnableDistributedTracing {
return fmt.Errorf("expected EnableDistributedTracing to be true")
}
if cfg.ec.DistributedTracingAddress != "localhost:4318" {
return fmt.Errorf("expected DistributedTracingAddress to be localhost:4318, got %s", cfg.ec.DistributedTracingAddress)
}
if cfg.ec.DistributedTracingServiceName != "test-service" {
return fmt.Errorf("expected DistributedTracingServiceName to be test-service, got %s", cfg.ec.DistributedTracingServiceName)
}
if cfg.ec.DistributedTracingServiceInstanceID != "instance-1" {
return fmt.Errorf("expected DistributedTracingServiceInstanceID to be instance-1, got %s", cfg.ec.DistributedTracingServiceInstanceID)
}
if cfg.ec.DistributedTracingSamplingRatePerMillion != 1000 {
return fmt.Errorf("expected DistributedTracingSamplingRatePerMillion to be 1000, got %d", cfg.ec.DistributedTracingSamplingRatePerMillion)
}
return nil
},
},
{
name: "use regular flags",
args: []string{
"--enable-distributed-tracing=true",
"--distributed-tracing-address=localhost:4319",
"--distributed-tracing-service-name=new-service",
"--distributed-tracing-instance-id=instance-2",
"--distributed-tracing-sampling-rate=2000",
},
expectValues: func(cfg *config) error {
if !cfg.ec.EnableDistributedTracing {
return fmt.Errorf("expected EnableDistributedTracing to be true")
}
if cfg.ec.DistributedTracingAddress != "localhost:4319" {
return fmt.Errorf("expected DistributedTracingAddress to be localhost:4319, got %s", cfg.ec.DistributedTracingAddress)
}
if cfg.ec.DistributedTracingServiceName != "new-service" {
return fmt.Errorf("expected DistributedTracingServiceName to be new-service, got %s", cfg.ec.DistributedTracingServiceName)
}
if cfg.ec.DistributedTracingServiceInstanceID != "instance-2" {
return fmt.Errorf("expected DistributedTracingServiceInstanceID to be instance-2, got %s", cfg.ec.DistributedTracingServiceInstanceID)
}
if cfg.ec.DistributedTracingSamplingRatePerMillion != 2000 {
return fmt.Errorf("expected DistributedTracingSamplingRatePerMillion to be 2000, got %d", cfg.ec.DistributedTracingSamplingRatePerMillion)
}
return nil
},
},
{
name: "config file with experimental flags",
yamlConfig: `
experimental-enable-distributed-tracing: true
experimental-distributed-tracing-address: "localhost:4318"
experimental-distributed-tracing-service-name: "test-service"
experimental-distributed-tracing-instance-id: "instance-1"
experimental-distributed-tracing-sampling-rate: 1000
`,
expectValues: func(cfg *config) error {
if !cfg.ec.EnableDistributedTracing {
return fmt.Errorf("expected EnableDistributedTracing to be true")
}
if cfg.ec.DistributedTracingAddress != "localhost:4318" {
return fmt.Errorf("expected DistributedTracingAddress to be localhost:4318, got %s", cfg.ec.DistributedTracingAddress)
}
if cfg.ec.DistributedTracingServiceName != "test-service" {
return fmt.Errorf("expected DistributedTracingServiceName to be test-service, got %s", cfg.ec.DistributedTracingServiceName)
}
if cfg.ec.DistributedTracingServiceInstanceID != "instance-1" {
return fmt.Errorf("expected DistributedTracingServiceInstanceID to be instance-1, got %s", cfg.ec.DistributedTracingServiceInstanceID)
}
if cfg.ec.DistributedTracingSamplingRatePerMillion != 1000 {
return fmt.Errorf("expected DistributedTracingSamplingRatePerMillion to be 1000, got %d", cfg.ec.DistributedTracingSamplingRatePerMillion)
}
return nil
},
},
{
name: "config file with regular flags",
yamlConfig: `
enable-distributed-tracing: true
distributed-tracing-address: "localhost:4319"
distributed-tracing-service-name: "new-service"
distributed-tracing-instance-id: "instance-2"
distributed-tracing-sampling-rate: 2000
`,
expectValues: func(cfg *config) error {
if !cfg.ec.EnableDistributedTracing {
return fmt.Errorf("expected EnableDistributedTracing to be true")
}
if cfg.ec.DistributedTracingAddress != "localhost:4319" {
return fmt.Errorf("expected DistributedTracingAddress to be localhost:4319, got %s", cfg.ec.DistributedTracingAddress)
}
if cfg.ec.DistributedTracingServiceName != "new-service" {
return fmt.Errorf("expected DistributedTracingServiceName to be new-service, got %s", cfg.ec.DistributedTracingServiceName)
}
if cfg.ec.DistributedTracingServiceInstanceID != "instance-2" {
return fmt.Errorf("expected DistributedTracingServiceInstanceID to be instance-2, got %s", cfg.ec.DistributedTracingServiceInstanceID)
}
if cfg.ec.DistributedTracingSamplingRatePerMillion != 2000 {
return fmt.Errorf("expected DistributedTracingSamplingRatePerMillion to be 2000, got %d", cfg.ec.DistributedTracingSamplingRatePerMillion)
}
return nil
},
},
{
name: "config file with both flags is error",
yamlConfig: `
experimental-enable-distributed-tracing: true
enable-distributed-tracing: true
`,
expectErr: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cfg := newConfig()

if tc.yamlConfig != "" {
tmpfile, err := os.CreateTemp("", "etcd-test-*.yaml")
if err != nil {
t.Fatal(err)
}
defer os.Remove(tmpfile.Name())

if err := os.WriteFile(tmpfile.Name(), []byte(tc.yamlConfig), 0600); err != nil {
t.Fatal(err)
}

tc.args = []string{"--config-file=" + tmpfile.Name()}
}

err := cfg.parse(tc.args)
if tc.expectErr {
if err == nil {
t.Error("expected error but got nil")
}
return
}

if err != nil {
t.Fatal(err)
}

if err := tc.expectValues(cfg); err != nil {
t.Error(err)
}
})
}
}