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.

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

Ribbon config without clientName not applies to all clients. #1741

Closed
charlesvhe opened this issue Mar 1, 2017 · 3 comments
Closed

Ribbon config without clientName not applies to all clients. #1741

charlesvhe opened this issue Mar 1, 2017 · 3 comments
Assignees
Milestone

Comments

@charlesvhe
Copy link

charlesvhe commented Mar 1, 2017

I wanna change default load banlancer rule with below configuration

ribbon.NFLoadBalancerRuleClassName=charles.sc.consumer.WeightedMetadataRule

but it not work, I must assign serviceId like below

someService.ribbon.NFLoadBalancerRuleClassName=charles.sc.consumer.WeightedMetadataRule

I can register WeightedMetadataRule as a spring bean, but it will disable all client's customization config.

org.springframework.cloud.netflix.ribbon.RibbonClientConfiguration code sniper below:

@Bean
@ConditionalOnMissingBean
public IRule ribbonRule(IClientConfig config) {
	if (this.propertiesFactory.isSet(IRule.class, name)) {
		return this.propertiesFactory.get(IRule.class, config, name);
	}
	ZoneAvoidanceRule rule = new ZoneAvoidanceRule();
	rule.initWithNiwsConfig(config);
	return rule;
}

for this purpose: config default setting, and clients can have it's own setting replace default.

I extends org.springframework.cloud.netflix.ribbon.PropertiesFactory, and change below method:

public String getClassName(Class clazz, String name) {
	if (this.classToProperty.containsKey(clazz)) {
		String classNameProperty = this.classToProperty.get(clazz);
		String className = environment.getProperty(name + "." + NAMESPACE + "." + classNameProperty);
		if(!StringUtil.hasText(className )){
			className = environment.getProperty(NAMESPACE + "." + classNameProperty);
		}
		return className;
	}
	return null;
}

It works fine for me. But PropertiesFactory fields are all private, it seems not want we extend this class, have there any other grace way?

@ryanjbaxter
Copy link
Contributor

I think you can specify a default configuration class that applies to all clients by using @RibbonClients. See this test https://github.com/spring-cloud/spring-cloud-netflix/blob/758ff920fc71d9a2aaf41d1250d3cc5ddd46b870/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/test/RibbonClientDefaultConfigurationTestsConfig.java. I dont see this documented, so I think we might need to tweak our docs. Let me know if this works for you.

@spencergibb
Copy link
Member

Both updating the docs and his change are both reasonable.

@ryanjbaxter ryanjbaxter self-assigned this Mar 1, 2017
@charlesvhe
Copy link
Author

charlesvhe commented Mar 2, 2017

Thanks, @RibbonClients worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants