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

POST /QuickPulseService.svc fails if instrumentation key is set programmatically #573

Closed
ghost opened this issue Feb 23, 2018 · 8 comments
Closed
Assignees
Labels

Comments

@ghost
Copy link

ghost commented Feb 23, 2018

In our Spring-Boot application we set the instrumentation key programmtically using the following code:

TelemetryConfiguration telemetryConfiguration = TelemetryConfiguration.getActive();
telemetryConfiguration.setInstrumentationKey(instrumentationKey);

This code is executed before the "WebRequestTrackingFilter" is registered, so that all HTTP request metrics are properly sent with the correct instrumentation key.

Additionally we explicitly set the instrumentation key on the logback "ApplicationInsightsAppender" so that traces (i.e. logs) are also sent to Application Insights with the correct instrumentation key.

However we noticed that the AI SDK tries to perform a POST request on the following URL every 5 seconds:

POST /QuickPulseService.svc/ping?ikey=null
HTTP/1.1 400 Bad Request

Here the instrumentation key is "null" and thus the request fails.

The question now is:

  • What is the purpose of this call?

  • Is it needed somehow or is it possible to simply disable it?

  • Or is it possible to somehow pass over the instrumentation key, so that this call can also use the programatically set instrumentation key?

And finally: If -for whatever reasons- the instrumentation key is "null", then no attempt should be made in the first place to contact the Application Insights server instance.

Any help/hints welcome.

Thanks.

@dhaval24
Copy link
Contributor

@julandw this is a very interesting situation and I haven't faced it personally. I would be happy to replicate this on my end but few questions to get an understanding : -

  1. Do you use different iKey's for LogBack appender xml then the iKey set via environment variable?
  2. Are you using the latest logback appender jars? If not please ensure that both your core sdk and appender jars reflect same version (2.0.0-BETA ideally)

Now answering your questions : -

  1. The purpose of this call is to enable live stream feature of Application Insights, (https://docs.microsoft.com/en-us/azure/application-insights/app-insights-live-stream). The SDK sends metrics information every 5 seconds to LiveMetrics micro-service on the backend which enables you to see the live feed of your CPU, memory, request per sec, dependency rate like some of the critical metrics in your application.
  2. Yes it can be possible to disable this. You can disable this by setting in your applicationinsights.xml file (however I would not recommend this unless required as this blocks a cool feature :-))
  3. And I totally agree with your point that if instrumentation key = null then there should be no call to the backend! If this is true then would be tracked as a bug and fixed as soon as possible.

Now coming to why in the first place. The indication that QuickPulse is having null instrumentation key is because some how the beans initialization order in the Springboot application leaves the TelemetryConfiguration object semi initialized or not initialized at all and hence when the call to set quickPulse() happens in the TelemetryConfigurationFactory class the problem arise.

All this being said I realize that there is a weak spring boot documentation and hence is causing troubles on onboarding. I apologize but would suggest that you follow the following guide to onboard to spring boot with AI : https://github.com/AzureCAT-GSI/DevCamp/tree/master/HOL/java/06-appinsights

There is one exception though in above :

 @Bean
    public FilterRegistrationBean aiFilterRegistration() {
        FilterRegistrationBean registration = new FilterRegistrationBean();
        registration.setFilter(new WebRequestTrackingFilter("appName"));
        registration.addUrlPatterns("/**");
        registration.setOrder(1);
        return registration;
    } 

    @Bean(name = "WebRequestTrackingFilter")
    public Filter WebRequestTrackingFilter() {
        return new WebRequestTrackingFilter("appName");
    }	

Passing the application name in the WebRequestTrackingFilter and setting the order of filter to 1 is very important.

I hope this helps and if problem persists please let us know.

Also as a Spring Boot user will this help for you : #518 We are in process of creating an amazing starter for SpringBoot which we think could allow in friction free onboarding to spring boot apps. Do you think it would be valuable for ubiquitous usage of AI in your place?

Let me know and hope this was helpful!

@dhaval24 dhaval24 self-assigned this Feb 23, 2018
@ghost
Copy link
Author

ghost commented Feb 26, 2018

Hello dhaval,

thanks for the quick answer!

To answer your questions:

  1. Do you use different iKey's for LogBack appender xml then the iKey set via environment variable?

Actually the basic idea is to set the instrumentation key as Spring application property and not as environment variable, quite similar as suggested in the pull request: #518. This allows a more Spring like configuration being able to easily define different instrumentation keys for different Spring profiles.

So we do not set the instrumentation key as environment variable at, which leads to the described problem.

In our current code, we read the instrumentation key from the Spring application properties file and configure the active TelemetryConfiguration object (if it not already set):

@Value("${" + APPLICATION_INSIGHTS_INSTRUMENTATION_KEY + ":}")
private String instrumentationKey;

TelemetryConfiguration telemetryConfiguration = TelemetryConfiguration.getActive();
if (StringUtils.isEmpty(telemetryConfiguration.getInstrumentationKey())) {
telemetryConfiguration.setInstrumentationKey(instrumentationKey);
}

We do this just before registering the WebRequestTrackingFilter (so that the filter always has the correct instrumentation key).

For the Logback-Appender we explicitly set the same instrumentation key by first retrieving the instrumentation key as Spring property:

and then initialize the Logback-Appender accordingly:

<appender name="AI_APPENDER"
          class="com.microsoft.applicationinsights.logback.ApplicationInsightsAppender">
    <instrumentationKey>${application_insights_ikey}</instrumentationKey>
</appender>

As an alternative you could also set the instrumentation key for the active TelemetryConfiguration instance at the earliest possible time in a ApplicationListener instance, which must be registered with the SpringApplication on startup.

Doing this it is not necessary to explicitly configure the instrumentation key in the Logback-Appender, as the Logback-Appender automatically uses the active TelemetryConfigurtion instance.

The problem with the "QuickPulse" implementation is that the initialization process takes place before the Spring environment (e.g. application properties) is properly set up and thus even before the "ApplicationEnvironmentPreparedEvent" is fired.

At that time the instrumentation key is not (if not set via an environment variable or system property) leading to the original problem.

So basically I see only two possibilities:
a. Either the initialization of the "QuickPulse" services is somehow postponed until I have a chance to read the instrumentation key from the Spring environment and then configure the active TelemetryConfiguration instance, ....

b. or the "QuickPulse" service continuously tries to retrieve a valid instrumentation key from the active TelemetryConfiguration instance until it gets a non-null value.

I would prefer the second option (and maybe there are other options too).

Does this sound reasonable to you?

  1. Are you using the latest logback appender jars? If not please ensure that both your core sdk and appender jars reflect same version (2.0.0-BETA ideally)

I tested it with version 1.0.9 and 2.0.0-BETA (version 1.0.10 seems not to send any traces)

@dhaval24
Copy link
Contributor

@julandw I totally agree that Spring Application developers would like to set iKey in the properties file and then retrive it using @value("$name") annotation. This is specifically because there are different properties file in the application to support different environments like Dev, Test and Prod where user requires different configuration. One example is that in dev time user would ideally configure tomcat properties to auto restart on code change in non static files however this is not a desirable for production. This ability is essentially the Spring Boots powerful capability.

That being said - I think as I mentioned before that some how QuickPulse module is getting an non initialized TelemetryConfiguration instance and hence it complains that there is no instrumentation key. (most probably the reason you mentioned that the Quick pulse module is getting initialized much before than the PropertySourceBean which associates the properties file to application is loaded. Please correct me if I get this wrong here).

As far as the solution to this problem is concerned I would say as follows for your options.

  1. Postpone the initialization of QuickPulse - Yes this seems to be most reliable but in-order to achieve this we need Added application insights starter for spring boot #518. (More on it at the end)
  2. Continuously try to retrieve the valid iKey from the active TC instance. This would essentially mean running a thread indefinitely in side to keep querying this. I would not likely prefer this. It would be resource intensive task and still won't provide any guaranteed way that we will ever get the Ikey. Considering not only SpringBoot scenario but also other frameworks, each behave differently.

So how do we get 1. achieved - Look at https://github.com/gavlyukovskiy/ApplicationInsights-Java/blob/e8f688fa1a11e2aaadae8bf99f4fc273538e3cc7/azure-application-insights-spring-boot-starter/src/main/java/com/microsoft/applicationinsights/boot/ApplicationInsightsTelemetryAutoConfiguration.java#L153
Here the QucikPulse module would be registered as SpringBean which is now specified in the module defined by user and is not a part of SpringBoot module. That means that all the SpringBoot Modules have higher precedence of initialization and would get initialized before the AI beans and therefore by the time AI modules start initialization the PropertySourceBean would already have been initialized and therfore @value() tag will not get the null iKey. Does this make sense?

As of today it will take some time to have the General Availability of AI SpringBoot starter (PR still needs some finishing and behavior testing and more importantly it would start with a Beta which should be fine though).

I would therefore suggest the following work-around, though is not the best and would lead to some duplication and hardcoding. You could create a static constants of iKeys for different profiles inside your ApplicationInsights configuration class (where you have other AI beans) and then based on condition of profile activated you can pick the iKey.

Also, I would be more than happy to try multiple alternatives on my end, if it is possible for you to upload a strip downed version of your application. Hope it helps!

@ghost
Copy link
Author

ghost commented Feb 28, 2018

@dhaval24 OK, I totally agree with you that postponing the initialization of QuickPulse is the better option. I just thought that this option is not so easy to achieve.

I also had a look at the ApplicationInsightsTelemetryAutoConfiguration java class and yes this all makes sense to easily use and configure AI for Spring-Boot.

Maybe one more thought: you have to ensure that the QuickPulse Sopring-Bean is not created before the "TelemetryConfiguration" Spring-Bean (which sets the instrumentation key on the active configuration). You could do this by simply declare a dependency using the @dependsOn annotation.

Thanks for your support and I am eagerly waiting for the Spring-Boot starter libray.

@dhaval24
Copy link
Contributor

@julandw thank you very much for your interest with Application Insights and the work we are doing here. Referencing this issue in the PR so we can keep a track of the suggestion. BTW we released a patch version 2.0.1 to fix invalid POM issue. You can try and see how the experience is. Hopefully this would be much better :)

@gavlyukovskiy
Copy link
Contributor

This is similar issue to what I had with logback. Logback was initialized much before than TelemetryConfiguration at the moment when the TelemetryConfiguration#instrumentationKey is null and to fix this I have created another pull request - #516 which fixes instrumentation key reload mechanism inside TelemetryContext in case when at least 1 event was fired.

Continuously try to retrieve the valid iKey from the active TC instance. This would essentially mean running a thread indefinitely in side to keep querying this.

You don't need such thread to reload instrumentation key, you can do it similar to what I did for logback - check that if instrumentation key is null at the moment of making request and then take new one from active TelemetryConfiguration.

Regarding spring boot auto-configuration you're right, I didn't think about it, though quick pulse is working for sample application, but it looks like coincidence and I will add explicit dependency on TelemetryConfiguration.

P.S. @julandw thanks for pointing @DependsOn annotation, I didn't know about this one and was adding dependent bean into the constructor like this

@Bean
@ConditionalOnMissingBean
public WebRequestTrackingFilter webRequestTrackingFilter(@Value("${spring.application.name:application}") String applicationName,
        // we have implicit dependency on configured telemetryConfiguration here
        @SuppressWarnings("unused") TelemetryConfiguration telemetryConfiguration) {
    return new WebRequestTrackingFilter(applicationName);
}

will change this one to @DependsOn as well ;)

@ghost
Copy link
Author

ghost commented Mar 1, 2018

@gavlyukovskiy Looks easier with the annotation :)

Is there already a (rough) timeline, when PullRequest 518 will be released?

@dhaval24
Copy link
Contributor

@julandw Application Insights SpringBoot starter is now released. #646 is the PR and here is the readme: https://github.com/Microsoft/ApplicationInsights-Java/blob/master/azure-application-insights-spring-boot-starter/README.md

Take a look. This should solve the problem in this issue too!

@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

2 participants