These are chat archives for spring-cloud/spring-cloud

12th
Jun 2017
allbutone
@SilentNight_R_twitter
Jun 12 2017 03:26

@dsyer in my spring boot project (1.3.8.RELEASE), i place all Transactional and Caching annotation on interface, which according to spring reference is not suggested, but it's a large project, I can't refactor and move all mentioned annotations to concrete class now, two days ago, I try to integrate spring cloud (Brixton.SR7), but i find that all Transactional and Caching annotations on interfaces are not recognized any more, after debugging i find code below:

package org.springframework.cloud.netflix.metrics;
public class ServoEnvironmentPostProcessor implements EnvironmentPostProcessor {
    @Override
    public void postProcessEnvironment(ConfigurableEnvironment environment,
            SpringApplication application) {
        if (ClassUtils.isPresent("com.netflix.servo.monitor.Monitors", null)) {
            // Make spring AOP default to target class so RestTemplates can be customized
            addDefaultProperty(environment, "spring.aop.proxyTargetClass", "true");//set to true, this is the reason.
        }
    }
}

in above code, spring.aop.proxyTargetClass was set to true which conflicts with default behavior of AopAutoConfiguration below:

@Configuration
@ConditionalOnClass({ EnableAspectJAutoProxy.class, Aspect.class, Advice.class })
@ConditionalOnProperty(prefix = "spring.aop", name = "auto", havingValue = "true", matchIfMissing = true)
public class AopAutoConfiguration {

    @Configuration
    @EnableAspectJAutoProxy(proxyTargetClass = false)
    @ConditionalOnProperty(prefix = "spring.aop", name = "proxy-target-class", havingValue = "false", matchIfMissing = true)
    public static class JdkDynamicAutoProxyConfiguration {
    }

    @Configuration
    @EnableAspectJAutoProxy(proxyTargetClass = true)
    @ConditionalOnProperty(prefix = "spring.aop", name = "proxy-target-class", havingValue = "true", matchIfMissing = false)
    public static class CglibAutoProxyConfiguration {
    }
}

I thought spring.aop.proxy-target-class was set to false and consistent throughout whole project, but apparently not, so I'm confused now, any idea ?

I' been searing for similar questions, but no luck, so i hope to get some hints from the author @dsyer of org.springframework.cloud.netflix.metrics.ServoEnvironmentPostProcessor , thx in advance
allbutone
@SilentNight_R_twitter
Jun 12 2017 03:37
and @spencergibb any suggestions ?
Dave Syer
@dsyer
Jun 12 2017 08:04
@SilentNight_R_twitter I guess you need to either exclude servo from the classpath (it was replaced by atlas anyway), or set the proxy-target-class property to false, or both. It's not really clear why you haven't tried those things already, or if you did, what the outcome was.
allbutone
@SilentNight_R_twitter
Jun 12 2017 08:06
@dsyer I've tried setting spring.aop.proxy-target-class = false, it works
but I don't know what servo is and not sure if some side effects will come along
Dave Syer
@dsyer
Jun 12 2017 08:08
If you don't know what it is, then you can't possibly be using it.
You might as well exclude it from the classpath as well
allbutone
@SilentNight_R_twitter
Jun 12 2017 08:09
so you mean I can safely exclude servo from the classpath without worrying about potential side effects ?
Dave Syer
@dsyer
Jun 12 2017 08:09
It was replaced by atlas anyway.
But you are on an old version, so you might not have that choice. Anyway, it doesn't sound like you care, if you weren't using them.
allbutone
@SilentNight_R_twitter
Jun 12 2017 08:12
ok, thank you,I'm still a newbie learning spring cloud
got a lot fun from learning it.
Dave Syer
@dsyer
Jun 12 2017 08:13
You're welcome
Patrick Cornelißen
@pcornelissen
Jun 12 2017 14:10
ok, so regarding the spring social proxy problem :-)
So a release of spring social is not planned for the near future?
Dave Syer
@dsyer
Jun 12 2017 14:11
Unlikely
And even if it was, I wouldn't expect them to change the visibility of a class that you don't need to extend.
Patrick Cornelißen
@pcornelissen
Jun 12 2017 14:12
Hmm, ok but isn’t an AOP problem reason enough to raise the visibility? ;-)
Dave Syer
@dsyer
Jun 12 2017 14:12
Not my decision.
I wouldn't think so
It's kind of tricky. And not really a Spring Cloud question.
Although Spring Cloud is forcing you to face up to the situation, and make a choice.
Patrick Cornelißen
@pcornelissen
Jun 12 2017 14:13
strategy wise there are no plans to abandon spring-social, right? (I know, not your direct responsibility, not maybe you heard something somewhere?)
Dave Syer
@dsyer
Jun 12 2017 14:14
All we can do here really is make the defaults easy for the majority of people (I think 2 people have hit this problem since we created Spring Cloud in 2014
Servo is also obsolete now, so in the next release of Spring Cloud Netflix we'll probably flip the defaults.
Spring Social isn't dead
Just doesn't have a lot of momentum
If there was a big community behind it, it might be different.
(There isn't)
Patrick Cornelißen
@pcornelissen
Jun 12 2017 14:18
I see, thanks! If servo is obsolete anyway I’ll have a look if excluding it fixes it, then I’ll go this route
Patrick Cornelißen
@pcornelissen
Jun 12 2017 14:43
Maybe I should add a chapter in the spring boot course I’m creating for packt publishing to boot spring-social ;)
Dave Syer
@dsyer
Jun 12 2017 14:49
It's not a huge community, like I said.
Patrick Cornelißen
@pcornelissen
Jun 12 2017 16:02
I have removed all dependencies to servo, I only see
[INFO] | | | - io.reactivex:rxnetty-servo:jar:0.4.9:runtime
as remaining dependency and I have
aop.proxy-target-class: false
in the application.yaml
Then I still get
The bean 'restTemplate' could not be injected as a 'org.springframework.web.client.RestTemplate' because it is a JDK dynamic proxy that implements:
    org.springframework.web.client.RestOperations
looks like "cloud.netflix.metrics.enabled: false“ is still necessary even with the servo excludes
Dave Syer
@dsyer
Jun 12 2017 16:34
How about opening an issue?
Patrick Cornelißen
@pcornelissen
Jun 12 2017 16:59
because it still requires the property?
Dave Syer
@dsyer
Jun 12 2017 18:11
Yes. For starters.