-
Notifications
You must be signed in to change notification settings - Fork 462
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
Exception on startup due to telemetry dependencies on modules removed from Java 11 #598
Comments
@rupert654 how about you reference jaxb in your application? Would that help as workaround. Currently with java 11 it removes jaxb from the core java platform and Application Insights SDK depends on that. Ideally, @Incarnation-p-lee I agree with @rupert654 that you should have single property to disable telemetry for all azure starters. |
Hi @dhaval24, That does indeed work as a workaround (I mention the library that Spring Boot recommends in the issue above). However, it has more consequences. This particular library, and indeed another dependency pulled in by applicationinsights-core (grpc-netty-shaded), are fat jars which cause errors when running on Tomcat and using JSPs because Tomcat's class path scanning break on shaded jars. I then have to do more work (not much but more), to fix this new issue. That isn't really the fault of this library, but the real issue here is this library pulls in a lot of complexity to do something that is not a core part of its function and disabling it does not properly disable it. I could fork this library, remove the telemetry and it would provide a better experience for developers. It's one thing to include opt-out tracking but its another for your opt-out tracking to then break your own library (or ask developers to jump through hoops to stop it from breaking). A much cleaner solution would be to be able to exclude a Maven package, which would automatically remove all the relevant auto-configuration and unneeded dependencies. I'm happy to raise a PR if this approach is acceptable? |
@rupert654 yes, I agree that Application Insights, is complex sdk and is used for monitoring applications. In general it is a good practice to reduce the dependencies in the library to avoid classpath hells. @Incarnation-p-lee can comment more, but I believe there was some conversation that in future this dependency would be removed and the azure-spring-starters will rely on application insights rest api as the purpose right now is to just collect Business telemetry. |
@rupert654 Could you please have a try from our newest release version ? Thanks a lot. |
@Incarnation-p-lee I've just tested on 2.1.6 and this all seems to be fixed. Thanks very much! My jars are much lighter now! |
Environment
Summary
On startup, the following exception is thrown when running with the Spring Boot Maven Plugin:
Reproduce steps
I've created a sample repository to reproduce the bug.
Please note that I have disabled telemetry using the property:
Expected Results
To not see that exception.
Actual Results
The above exception occurs in the startup logs.
Workaround
Adding back these Java APIs (removed in Java 11) fixed the problem.
Recommendations
The root cause of this problem is microsoft/ApplicationInsights-Java#674.
As far as I can work out, this dependency is pulled in purely to enable telemetry. That's fine but I've got it disabled and it's still breaking my application!
In your README, you say:
The core problem here is this telemetry feature:
Can I please ask that you offer a more robust solution to disabling the telemetry?
Firstly, if you just had a single property to disable telemetry, instead of one per starter, then you could place a condition on all auto-configuration relating to Telemetry e.g. TelemetryProxyConfiguration. This would at least ensure that no code relating to Telemetry ran which could otherwise cause errors (like this one) even though it had been disabled.
Secondly, you could package up all the telemetry code into its own library, and place any dependencies relating to telemetry (like application-insights) into that library. This would make opting out of everything much easier, as consumers could just exclude that library in Maven e.g. something like this:
Currently, I can't even exclude applicationinsights-core in Maven and TelemetryProxyConfiguration in Spring because AADOAuth2AutoConfiguration has a dependency on TelemetryProxy regardless of whether telemetry has been disabled.
The text was updated successfully, but these errors were encountered: