Copilot commented on code in PR #3320: URL: https://github.com/apache/dubbo-go/pull/3320#discussion_r3230886078
########## instance_options_init.go: ########## @@ -0,0 +1,722 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package dubbo + +import ( + "fmt" + "net/url" + "strconv" + "strings" +) + +import ( + getty "github.com/apache/dubbo-getty" + + "github.com/creasty/defaults" + + "github.com/dubbogo/gost/log/logger" + + "github.com/knadh/koanf" + + "go.opentelemetry.io/otel" +) + +import ( + "dubbo.apache.org/dubbo-go/v3/common" + commonCfg "dubbo.apache.org/dubbo-go/v3/common/config" + "dubbo.apache.org/dubbo-go/v3/common/constant" + "dubbo.apache.org/dubbo-go/v3/common/extension" + "dubbo.apache.org/dubbo-go/v3/config_center" + aslimiter "dubbo.apache.org/dubbo-go/v3/filter/adaptivesvc/limiter" + "dubbo.apache.org/dubbo-go/v3/global" + "dubbo.apache.org/dubbo-go/v3/internal" + "dubbo.apache.org/dubbo-go/v3/metrics" + metricsConfigCenter "dubbo.apache.org/dubbo-go/v3/metrics/config_center" + "dubbo.apache.org/dubbo-go/v3/otel/trace" + "dubbo.apache.org/dubbo-go/v3/remoting" +) + +func (rc *InstanceOptions) finalizeGlobalOptionsWithRuntimeActivation(activateRuntime bool) error { + if err := rc.initGlobalApplication(); err != nil { + return err + } + if err := rc.initGlobalCustom(); err != nil { + return err + } + if err := rc.initGlobalProtocols(); err != nil { + return err + } + if err := rc.initGlobalRegistries(); err != nil { + return err + } + if err := rc.initGlobalMetrics(activateRuntime); err != nil { + return err + } + if err := rc.initGlobalOtel(activateRuntime); err != nil { + return err + } + if err := rc.initGlobalRouters(); err != nil { + return err + } + for _, tracingConfig := range rc.Tracing { + if tracingConfig == nil { + continue + } + if err := defaults.Set(tracingConfig); err != nil { + return err + } + if err := commonCfg.Verify(tracingConfig); err != nil { + return err + } + } + if err := rc.initGlobalProvider(activateRuntime); err != nil { + return err + } + if err := rc.initGlobalConsumer(); err != nil { + return err + } + return rc.initGlobalShutdown() +} + +func (rc *InstanceOptions) initGlobalApplication() error { + if rc.Application == nil { + rc.Application = global.DefaultApplicationConfig() + } + if err := defaults.Set(rc.Application); err != nil { + return err + } + if rc.Application.Name == "" { + rc.Application.Name = constant.DefaultDubboApp + } + return commonCfg.Verify(rc.Application) +} + +func (rc *InstanceOptions) initGlobalCustom() error { + if rc.Custom == nil { + rc.Custom = global.DefaultCustomConfig() + } + if rc.Custom.ConfigMap == nil { + rc.Custom.ConfigMap = make(map[string]any) + } + if err := defaults.Set(rc.Custom); err != nil { + return err + } + return commonCfg.Verify(rc.Custom) +} + +func (rc *InstanceOptions) initGlobalProtocols() error { + if len(rc.Protocols) <= 0 { + rc.Protocols = map[string]*global.ProtocolConfig{ + constant.TriProtocol: {}, + } + } + for key, protocolConfig := range rc.Protocols { + if protocolConfig == nil { + protocolConfig = &global.ProtocolConfig{} + rc.Protocols[key] = protocolConfig + } + if err := initGlobalProtocol(protocolConfig); err != nil { + return err + } + } + return nil +} + +func initGlobalProtocol(protocolConfig *global.ProtocolConfig) error { + if err := defaults.Set(protocolConfig); err != nil { + return err + } + if protocolConfig.Name == "" { + protocolConfig.Name = constant.TriProtocol + } + if protocolConfig.Port == "" { + protocolConfig.Port = constant.DefaultTripleProtocolPort + } Review Comment: `initGlobalProtocol` defaults the Triple protocol port to `constant.DefaultTripleProtocolPort` (50051) when `protocolConfig.Port` is empty. This creates inconsistent defaults vs other entry points that use `global.DefaultProtocolConfig()` (which currently sets the TriProtocol port to `constant.DefaultPort` = 20000). Consider centralizing the default TriProtocol port (e.g., update `global.DefaultProtocolConfig` to use the new constant, or derive defaults from `global.DefaultProtocolConfig` here) so `NewServer` and `NewInstance().NewServer()` don't end up listening on different ports by default. ########## global/metric_config.go: ########## @@ -25,9 +25,9 @@ type MetricsConfig struct { Protocol string `default:"prometheus" yaml:"protocol" json:"protocol,omitempty" property:"protocol"` Prometheus *PrometheusConfig `yaml:"prometheus" json:"prometheus" property:"prometheus"` Aggregation *AggregateConfig `yaml:"aggregation" json:"aggregation" property:"aggregation"` - EnableMetadata *bool `default:"true" yaml:"enable-metadata" json:"enable-metadata,omitempty" property:"enable-metadata"` - EnableRegistry *bool `default:"true" yaml:"enable-registry" json:"enable-registry,omitempty" property:"enable-registry"` - EnableConfigCenter *bool `default:"true" yaml:"enable-config-center" json:"enable-config-center,omitempty" property:"enable-config-center"` + EnableMetadata *bool `default:"false" yaml:"enable-metadata" json:"enable-metadata,omitempty" property:"enable-metadata"` + EnableRegistry *bool `default:"false" yaml:"enable-registry" json:"enable-registry,omitempty" property:"enable-registry"` + EnableConfigCenter *bool `default:"false" yaml:"enable-config-center" json:"enable-config-center,omitempty" property:"enable-config-center"` Probe *ProbeConfig `yaml:"probe" json:"probe" property:"probe"` Review Comment: This changes global metrics defaults (`enable-metadata`, `enable-registry`, `enable-config-center`) from true to false. That is a behavior change that can disable collectors for users relying on previous global defaults; if intentional (e.g., aligning with the legacy `config` package defaults), it should be called out in the PR description and/or release notes to avoid a silent breaking change. ########## global/metric_config.go: ########## @@ -52,7 +52,7 @@ type ProbeConfig struct { } type Exporter struct { - Enabled *bool `default:"false" yaml:"enabled" json:"enabled,omitempty" property:"enabled"` + Enabled *bool `default:"true" yaml:"enabled" json:"enabled,omitempty" property:"enabled"` Review Comment: `Exporter.Enabled` default is changed from false to true, which can change runtime behavior when metrics are enabled (exporter will start emitting by default). Please confirm this is intended, and document it alongside other default changes so operators aren't surprised by additional endpoints/traffic. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
