java.lang.IllegalArgumentException: Metrics port cannot be null #13684

bitgorust · 2024-01-23T02:07:52Z

I have searched the issues of this repository and believe that this is not a duplicate.

Environment

Dubbo version: 3.2.9
Operating System version: CentOS 8
Java version: Oracle JDK 8u221

Steps to reproduce this issue

set system property dubbo.metrics.enabled=false
define dubbo references by exporting ReferenceConfig.get() as Beans
start

Pls. provide [GitHub address] to reproduce this issue.

Expected Behavior

Project starts successfully. References created without metrics configuration.

Actual Behavior

References created with metrics enabled, but port is null.

Exception trace:

2024-01-22 17:55:47.207 |-ERROR [main] org.apache.dubbo.config.deploy.DefaultApplicationDeployer []-[traceId:] -|  [DUBBO] Dubbo Application[1.1](qijian-rec-service-41-prod) an exception occurred when handle starting event, dubbo version: 3.2.9, current host: 10.5.139.33, error code: 5-14. This may be caused by , go to https://dubbo.apache.org/faq/5/14 to find instructions. 
java.lang.IllegalArgumentException: Metrics port cannot be null
	at org.apache.dubbo.common.utils.Assert.notEmptyString(Assert.java:33) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.AbstractInterfaceConfig.appendMetricsCompatible(AbstractInterfaceConfig.java:280) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ServiceConfig.buildAttributes(ServiceConfig.java:662) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ServiceConfig.doExportUrlsFor1Protocol(ServiceConfig.java:587) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ServiceConfig.doExportUrls(ServiceConfig.java:579) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ServiceConfig.doExport(ServiceConfig.java:540) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ServiceConfig.export(ServiceConfig.java:340) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ServiceConfigBase.export(ServiceConfigBase.java:422) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.metadata.ConfigurableMetadataServiceExporter.export(ConfigurableMetadataServiceExporter.java:76) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.metadata.ExporterDeployListener.onModuleStarted(ExporterDeployListener.java:85) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.deploy.DefaultApplicationDeployer.exportMetadataService(DefaultApplicationDeployer.java:1263) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.deploy.DefaultApplicationDeployer.prepareApplicationInstance(DefaultApplicationDeployer.java:764) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.deploy.DefaultApplicationDeployer.checkState(DefaultApplicationDeployer.java:1149) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.deploy.DefaultApplicationDeployer.notifyModuleChanged(DefaultApplicationDeployer.java:1137) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.deploy.DefaultModuleDeployer.onModuleStarted(DefaultModuleDeployer.java:363) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.deploy.DefaultModuleDeployer.startSync(DefaultModuleDeployer.java:188) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.deploy.DefaultModuleDeployer.start(DefaultModuleDeployer.java:156) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ReferenceConfig.get(ReferenceConfig.java:235) ~[dubbo-3.2.9.jar:3.2.9]
	at org.apache.dubbo.config.ReferenceConfigBase.get(ReferenceConfigBase.java:395) ~[dubbo-3.2.9.jar:3.2.9]

Analysis

In

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java

Lines 274 to 277 in 2cf54f7

    
           @Deprecated 
        
           protected void appendMetricsCompatible(Map<String, String> map) { 
        
               MetricsConfig metricsConfig = getConfigManager().getMetrics().orElse(null); 
        
               if (metricsConfig != null) {

(though it is marked as deprecated but is still used now), MetricsConfig is got by getConfigManager().getMetrics().orElse(null). Dive in we can know that the instance actually comes from

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/context/AbstractConfigManager.java

Lines 294 to 296 in 2cf54f7

    
           protected <C extends AbstractConfig> Map<String, C> getConfigsMap(String configType) { 
        
               return (Map<String, C>) configsCache.getOrDefault(configType, emptyMap()); 
        
           }

. It is added here

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/context/AbstractConfigManager.java

Lines 554 to 566 in 2cf54f7

    
           if (ConfigurationUtils.hasSubProperties(configurationMaps, AbstractConfig.getTypePrefix(cls))) { 
        
               T config; 
        
               try { 
        
                   config = createConfig(cls, scopeModel); 
        
                   config.refresh(); 
        
               } catch (Exception e) { 
        
                   throw new IllegalStateException( 
        
                           "create default config instance failed, type:" + cls.getSimpleName()); 
        
               } 
        
               this.addConfig(config); 
        
               tmpConfigs.add(config); 
        
           }

because the statement ConfigurationUtils.hasSubProperties(configurationMaps, AbstractConfig.getTypePrefix(cls)) is true here, which I think it should do more than just looking for properties with dubbo.metrics prefix.

Workaround

Now we work it around by adding org.apache.dubbo:dubbo-spring-boot-observability-starter to avoid null port exceptions.

Solution

One reasonable solution I think is to add one more condition after

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/context/AbstractConfigManager.java

Line 554 in 2cf54f7

    
           if (ConfigurationUtils.hasSubProperties(configurationMaps, AbstractConfig.getTypePrefix(cls))) {

to check if contains ${prefix}.enabled=false configuration. Or maybe some solutions are on the way as appendMetricsCompatible is deprecated.

The text was updated successfully, but these errors were encountered:

yuluo-yx · 2024-01-24T03:16:06Z

If need to fix it, please assign to me.

bitgorust · 2024-01-26T02:17:56Z

@songxiaosheng @yuluo-yx

Since I've read Configuration Item Reference Manual (BTW, English version seems not up-to-date neither well typeset) and code relative to MetricsConfig / AggregationConfig / PrometheusConfig, I think L280 and L282 are no longer necessary here.

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java

Lines 279 to 283 in 2cf54f7

    
           if (!StringUtils.isEquals(protocol, PROTOCOL_PROMETHEUS)) { 
        
               Assert.notEmptyString(metricsConfig.getPort(), "Metrics port cannot be null"); 
        
               map.put("metrics.protocol", protocol); 
        
               map.put("metrics.port", metricsConfig.getPort()); 
        
           }

MetricsFilter is marked deprecated. And even it is useful now, metrics.protocol and metrics.port have their own default values respectively.
port in MetricsConfig is also marked deprecated. Nor used in nested PrometheusConfig and AggregationConfig
enabled flag only appears in MetricsConfig and TracingConfig according to Configuration Item Reference Manual. In MetricsConfig, enabled is actually for AggregationConfig. In TracingConfig, enabled has its default value. So now I think maybe checking enabled flag when adding config in AbstractConfigManager is not a good choice.

Hope this helps.

songxiaosheng · 2024-01-26T10:09:43Z

@bitgorust The ports related to indicators have been migrated to the QOS service, and outdated configurations will gradually be removed in the future

bitgorust · 2024-01-29T00:26:36Z

@bitgorust The ports related to indicators have been migrated to the QOS service, and outdated configurations will gradually be removed in the future

@songxiaosheng Thanks for telling your plan. So maybe we can simply drop L280 and L282 here instead of kind of breaking changes like #13687?

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java

Lines 279 to 283 in 5c0b1de

    
           if (!StringUtils.isEquals(protocol, PROTOCOL_PROMETHEUS)) { 
        
               Assert.notEmptyString(metricsConfig.getPort(), "Metrics port cannot be null"); 
        
               map.put("metrics.protocol", protocol); 
        
               map.put("metrics.port", metricsConfig.getPort()); 
        
           }

songxiaosheng · 2024-01-30T01:09:19Z

@bitgorust The ports related to indicators have been migrated to the QOS service, and outdated configurations will gradually be removed in the future

@songxiaosheng Thanks for telling your plan. So maybe we can simply drop L280 and L282 here instead of kind of breaking changes like #13687?

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java

Lines 279 to 283 in 5c0b1de

if (!StringUtils.isEquals(protocol, PROTOCOL_PROMETHEUS)) {

Assert.notEmptyString(metricsConfig.getPort(), "Metrics port cannot be null");

map.put("metrics.protocol", protocol);

map.put("metrics.port", metricsConfig.getPort());

}

you can try it

bitgorust · 2024-01-30T02:27:03Z

@bitgorust The ports related to indicators have been migrated to the QOS service, and outdated configurations will gradually be removed in the future

@songxiaosheng Thanks for telling your plan. So maybe we can simply drop L280 and L282 here instead of kind of breaking changes like #13687?

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java

Lines 279 to 283 in 5c0b1de

if (!StringUtils.isEquals(protocol, PROTOCOL_PROMETHEUS)) {

Assert.notEmptyString(metricsConfig.getPort(), "Metrics port cannot be null");

map.put("metrics.protocol", protocol);

map.put("metrics.port", metricsConfig.getPort());

}

you can try it

Pull request is subbmited: #13705

songxiaosheng · 2024-01-31T13:33:10Z

How about deleting the outdated methods and testing them out

bitgorust · 2024-02-01T01:19:33Z

How about deleting the outdated methods and testing them out

Glad to be part of the future. I think following code can be deleted. There will be another PR later.

dubbo/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java

Line 666 in 29b0c72

appendMetricsCompatible(map);

dubbo/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java

Line 445 in 29b0c72

appendMetricsCompatible(map);

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java

Lines 270 to 286 in 29b0c72

    
               /** 
        
                * @deprecated After metrics config is refactored. 
        
                * This method should no longer use and will be deleted in the future. 
        
                */ 
        
               @Deprecated 
        
               protected void appendMetricsCompatible(Map<String, String> map) { 
        
                   MetricsConfig metricsConfig = getConfigManager().getMetrics().orElse(null); 
        
                   if (metricsConfig != null) { 
        
                       String protocol = Optional.ofNullable(metricsConfig.getProtocol()).orElse(PROTOCOL_PROMETHEUS); 
        
                       if (!StringUtils.isEquals(protocol, PROTOCOL_PROMETHEUS)) { 
        
                           Assert.notEmptyString(metricsConfig.getPort(), "Metrics port cannot be null"); 
        
                           map.put("metrics.protocol", protocol); 
        
                           map.put("metrics.port", metricsConfig.getPort()); 
        
                       } 
        
                   } 
        
               }

bitgorust · 2024-02-01T02:45:14Z

How about deleting the outdated methods and testing them out

Glad to be part of the future. I think following code can be deleted. There will be another PR later.

dubbo/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ServiceConfig.java

Line 666 in 29b0c72

appendMetricsCompatible(map);

dubbo/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ReferenceConfig.java

Line 445 in 29b0c72

appendMetricsCompatible(map);

dubbo/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java

Lines 270 to 286 in 29b0c72

/**

* @deprecated After metrics config is refactored.

* This method should no longer use and will be deleted in the future.

*/

@Deprecated

protected void appendMetricsCompatible(Map<String, String> map) {

MetricsConfig metricsConfig = getConfigManager().getMetrics().orElse(null);

if (metricsConfig != null) {

String protocol = Optional.ofNullable(metricsConfig.getProtocol()).orElse(PROTOCOL_PROMETHEUS);

if (!StringUtils.isEquals(protocol, PROTOCOL_PROMETHEUS)) {

Assert.notEmptyString(metricsConfig.getPort(), "Metrics port cannot be null");

map.put("metrics.protocol", protocol);

map.put("metrics.port", metricsConfig.getPort());

}

}

}

@songxiaosheng Here is the new one: #13717

* fix: delete outdated metrics port assertions * chore: remove outdated port attribute in metricsType

bitgorust added the type/need-triage Need maintainers to triage label Jan 23, 2024

songxiaosheng assigned yuluo-yx Jan 24, 2024

yuluo-yx mentioned this issue Jan 24, 2024

fix: fix #13684 #13687

Closed

bitgorust mentioned this issue Jan 30, 2024

Fixes #13684: remove deprecated port non-null assertion in case start failure #13705

Closed

bitgorust mentioned this issue Feb 1, 2024

Fixes #13684: delete outdated metrics port assertions #13717

Merged

AlbumenJ closed this as completed in #13717 Feb 2, 2024

AlbumenJ pushed a commit that referenced this issue Feb 2, 2024

Fixes #13684: delete outdated metrics port assertions (#13717)

05cecc5

* fix: delete outdated metrics port assertions * chore: remove outdated port attribute in metricsType

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

java.lang.IllegalArgumentException: Metrics port cannot be null #13684

java.lang.IllegalArgumentException: Metrics port cannot be null #13684

bitgorust commented Jan 23, 2024

yuluo-yx commented Jan 24, 2024

bitgorust commented Jan 26, 2024 •

edited

Loading

songxiaosheng commented Jan 26, 2024

bitgorust commented Jan 29, 2024 •

edited

Loading

songxiaosheng commented Jan 30, 2024

bitgorust commented Jan 30, 2024

songxiaosheng commented Jan 31, 2024

bitgorust commented Feb 1, 2024

bitgorust commented Feb 1, 2024

java.lang.IllegalArgumentException: Metrics port cannot be null #13684

java.lang.IllegalArgumentException: Metrics port cannot be null #13684

Comments

bitgorust commented Jan 23, 2024

Environment

Steps to reproduce this issue

Expected Behavior

Actual Behavior

Analysis

Workaround

Solution

yuluo-yx commented Jan 24, 2024

bitgorust commented Jan 26, 2024 • edited Loading

songxiaosheng commented Jan 26, 2024

bitgorust commented Jan 29, 2024 • edited Loading

songxiaosheng commented Jan 30, 2024

bitgorust commented Jan 30, 2024

songxiaosheng commented Jan 31, 2024

bitgorust commented Feb 1, 2024

bitgorust commented Feb 1, 2024

bitgorust commented Jan 26, 2024 •

edited

Loading

bitgorust commented Jan 29, 2024 •

edited

Loading