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

AI Interceptor overwrite default configuration of Spring MVC #422

Closed
mkaszub opened this issue Sep 11, 2017 · 6 comments
Closed

AI Interceptor overwrite default configuration of Spring MVC #422

mkaszub opened this issue Sep 11, 2017 · 6 comments
Assignees
Labels
Milestone

Comments

@mkaszub
Copy link

mkaszub commented Sep 11, 2017

According to the documentation of Spring Boot it automaticly configure Spring MVC.

If you want to keep Spring Boot MVC features, and you just want to add additional MVC configuration (interceptors, formatters, view controllers etc.) you can add your own @configuration class of type WebMvcConfigurerAdapter, but without @EnableWebMvc

If we take look at the applicationinsights.web pacakge there is a class called InterceptorRegistry

https://github.com/Microsoft/ApplicationInsights-Java/blob/420ace24b444a6da61110fcbec7c98e1a3a63c73/web/src/main/java/com/microsoft/applicationinsights/web/spring/internal/InterceptorRegistry.java#L33

So it looks like @EnableWebMvc annotation breaks auto configuration feature.

Reproduction :

  • Add static content under resource folder
  • Add reference to the application insights web package

Expected:

  • Spring Boot Autoconfiguration will configure static content for serving

Actual:

  • Static content is not served

Additional links:
https://stackoverflow.com/questions/46099116/applicationinsights-breaks-default-spring-boot-configuration-for-static-content

@beckylino beckylino added this to the 1.0.11 milestone Sep 13, 2017
@dhaval24 dhaval24 modified the milestones: 1.0.11, future Oct 19, 2017
@dhaval24
Copy link
Contributor

@mkaszub I totally agree with the documentation part that we should not use @enablewebmvc here as per the SpringBoot docs. However, the tradition spring frameworks still requires this I believe. Removing this annotation will fix the problem for SpringBoot but at the same time break traditional Spring. We need to think more about this. Do you have a potential solution which can possibly work in both the scenarios ?

cc: @gavlyukovskiy we should look at this too before making we finalize starter.

Apologies for getting so late in the game here @mkaszub

@gavlyukovskiy
Copy link
Contributor

I didn't use this class in the starter itself, it won't be applied unless user explicitly imports InterceptorRegistry.

Although inside the starter we can auto-configure RequestNameHandlerInterceptorAdapter instead, so that it will be included if user enabled web mvc.

@mkaszub did you explicitly import this class?

@dhaval24
Copy link
Contributor

@gavlyukovskiy I don't think that @mkaszub used starter. This issue I believe was present way before we started working on starter. I think it is a part of web jar. I haven't verified this on my own, but I suspect based on the documentation it would break spring auto config. Also I think the same functionality is well achieved by the filter, I really did not found a specific use case of this InterceptorRegistry. What are your thoughts?

@gavlyukovskiy
Copy link
Contributor

I understood that starter was not used, I mean spring boot will not include this configuration by default and fact of existing of this configuration won't cause any conflicts unless it was explicitly imported by user in their application.

Probably you have somewhere tutorial that asks to import this configuration? If so it will, obviously, break auto-configuration.

@dhaval24
Copy link
Contributor

I see I thought that this would by having the tag @EnableWebMvC and @Configuration Boot would auto import this. If it isn't going to auto import then we can leave it as it is and do not worry. I don't think so I have seen any docs / tutorial around this. Atleast I am not aware of it.

@dhaval24
Copy link
Contributor

@mkaszub we have removed the @EnableWebMvc annotation from the Interceptor Registry from 2.2.0 version of SDK as it was breaking springboot. I am closing this issue as it should solve this issue. Please reopen if needed.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants