Skip to content

Add timeout on profiler settings pull #4350

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add timeout on profiler settings pull #4350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...in/java/com/microsoft/applicationinsights/agent/internal/configuration/Configuration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,6 +1530,7 @@ public static class ProfilerConfiguration {
public boolean enableDiagnostics = false;
public boolean enableRequestTriggering = false;
public List<RequestTrigger> requestTriggerEndpoints = new ArrayList<>();
public int serviceProfilerHttpTimeoutSeconds = 30;
}

public static class GcEventConfiguration {
Expand Down
8 changes: 6 additions & 2 deletions .../java/com/microsoft/applicationinsights/agent/internal/profiler/ProfilingInitializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.azure.core.http.HttpPipeline;
import com.azure.core.http.policy.DefaultRedirectStrategy;
import com.azure.core.http.policy.RedirectPolicy;
import com.azure.core.http.policy.TimeoutPolicy;
import com.azure.monitor.opentelemetry.autoconfigure.implementation.utils.SystemInformation;
import com.azure.monitor.opentelemetry.autoconfigure.implementation.utils.ThreadPoolUtils;
import com.microsoft.applicationinsights.agent.internal.common.FriendlyException;
Expand All @@ -23,6 +24,7 @@
import java.io.File;
import java.net.MalformedURLException;
import java.net.URL;
import java.time.Duration;
import java.util.Arrays;
import java.util.HashSet;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -127,7 +129,8 @@ private synchronized void performInit() {
3,
"Location",
new HashSet<>(
Arrays.asList(HttpMethod.GET, HttpMethod.HEAD, HttpMethod.POST)))));
Arrays.asList(HttpMethod.GET, HttpMethod.HEAD, HttpMethod.POST)))),
new TimeoutPolicy(Duration.ofSeconds(configuration.serviceProfilerHttpTimeoutSeconds)));

serviceProfilerExecutorService =
Executors.newScheduledThreadPool(
Expand All @@ -140,7 +143,8 @@ private synchronized void performInit() {
getServiceProfilerFrontEndPoint(configuration),
telemetryClient.getInstrumentationKey(),
httpPipeline,
userAgent);
userAgent,
configuration.serviceProfilerHttpTimeoutSeconds);

// Monitor service remains alive permanently due to scheduling an periodic config pull
startPollingForConfigUpdates();
Expand Down
24 changes: 20 additions & 4 deletions .../microsoft/applicationinsights/agent/internal/profiler/service/ServiceProfilerClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.time.Duration;
import java.util.Date;
import java.util.UUID;
import javax.annotation.Nullable;
Expand All @@ -40,21 +41,25 @@ public class ServiceProfilerClient {
private final URL hostUrl;
private final String instrumentationKey;
private final HttpPipeline httpPipeline;
private final int httpTimeoutSeconds;
@Nullable private final String userAgent;

public ServiceProfilerClient(
URL hostUrl,
String instrumentationKey,
HttpPipeline httpPipeline,
@Nullable String userAgent) {
@Nullable String userAgent,
int httpTimeoutSeconds) {
this.hostUrl = hostUrl;
this.instrumentationKey = instrumentationKey;
this.httpPipeline = httpPipeline;
this.userAgent = userAgent;
this.httpTimeoutSeconds = httpTimeoutSeconds;
}

public ServiceProfilerClient(URL hostUrl, String instrumentationKey, HttpPipeline httpPipeline) {
this(hostUrl, instrumentationKey, httpPipeline, null);
public ServiceProfilerClient(
URL hostUrl, String instrumentationKey, HttpPipeline httpPipeline, int httpTimeoutSeconds) {
this(hostUrl, instrumentationKey, httpPipeline, null, httpTimeoutSeconds);
}

/** Obtain permission to upload a profile to service profiler. */
Expand Down Expand Up @@ -144,7 +149,18 @@ public Mono<ProfilerConfiguration> getSettings(Date oldTimeStamp) {

HttpRequest request = new HttpRequest(HttpMethod.GET, requestUrl);

return httpPipeline.send(request).flatMap(response -> handle(response, requestUrl));
return httpPipeline
.send(request)
.timeout(
Duration.ofSeconds(httpTimeoutSeconds),
Mono.error(
new HttpResponseException(
"Timed out after "
+ httpTimeoutSeconds
+ " seconds while waiting for response from "
+ requestUrl,
null)))
Comment on lines +154 to +162
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need timeout both here and via TimeoutPolicy?

.flatMap(response -> handle(response, requestUrl));
}

private static Mono<ProfilerConfiguration> handle(HttpResponse response, URL requestUrl) {
Expand Down
6 changes: 4 additions & 2 deletions ...a/com/microsoft/applicationinsights/agent/internal/profiler/config/ConfigServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ void pullSettings() throws IOException, InterruptedException {
new ServiceProfilerClient(
new URL("https://agent.azureserviceprofiler.net/"),
"00000000-0000-0000-0000-000000000000",
getHttpPipeline());
getHttpPipeline(),
30);

ConfigService configService = new ConfigService(serviceProfilerClient);

Expand Down Expand Up @@ -56,7 +57,8 @@ void badServiceResponseDoesNotProvideReturn() throws MalformedURLException {
new ServiceProfilerClient(
new URL("https://agent.azureserviceprofiler.net/"),
"00000000-0000-0000-0000-000000000000",
getHttpPipeline());
getHttpPipeline(),
30);

ConfigService configService = new ConfigService(serviceProfilerClient);
Mono<ProfilerConfiguration> result = configService.pullSettings();
Expand Down
6 changes: 4 additions & 2 deletions ...microsoft/applicationinsights/agent/internal/profiler/upload/UploadServiceSimpleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ void roleNameIsCorrectlyAddedToMetaData() throws IOException {
new ServiceProfilerClient(
new URL("https://agent.azureserviceprofiler.net/"),
"00000000-0000-0000-0000-000000000000",
httpPipeline);
httpPipeline,
30);

File tmpFile = createFakeJfrFile();
UUID appId = UUID.randomUUID();
Expand Down Expand Up @@ -93,7 +94,8 @@ void uploadWithoutFileThrows() throws MalformedURLException {
new ServiceProfilerClient(
new URL("https://agent.azureserviceprofiler.net/"),
"00000000-0000-0000-0000-000000000000",
httpPipeline);
httpPipeline,
30);

UUID appId = UUID.randomUUID();
UUID profileId = UUID.randomUUID();
Expand Down
3 changes: 2 additions & 1 deletion ...a/com/microsoft/applicationinsights/agent/internal/profiler/upload/UploadServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ void uploadFileGoodPathReturnsExpectedResponse() throws IOException {
new ServiceProfilerClient(
new URL("https://agent.azureserviceprofiler.net/"),
"00000000-0000-0000-0000-000000000000",
httpPipeline);
httpPipeline,
30);

File tmpFile = createFakeJfrFile();
UUID appId = UUID.randomUUID();
Expand Down