Skip to content

Commit

Permalink
Fix initialization sequence
Browse files Browse the repository at this point in the history
  • Loading branch information
trask committed Aug 8, 2022
1 parent b85b6e8 commit 53cc55c
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public class LazyHttpClient implements HttpClient {
private static final HttpClient INSTANCE = new LazyHttpClient();

public static final CountDownLatch safeToInitLatch = new CountDownLatch(1);

public static volatile String proxyHost;
public static volatile Integer proxyPortNumber;
public static volatile String proxyUsername;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public void afterAgent(AutoConfiguredOpenTelemetrySdk autoConfiguredOpenTelemetr
if (!"java".equals(System.getenv("FUNCTIONS_WORKER_RUNTIME"))) {
// Delay registering and starting AppId retrieval until the connection string becomes
// available for Linux Consumption Plan.
appIdSupplier.startAppIdRetrieval();
appIdSupplier.updateAppId();
}

LazyHttpClient.safeToInitLatch.countDown();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,25 @@ public class AppIdSupplier implements AiAppId.Supplier {
private GetAppIdTask task;
private final Object taskLock = new Object();

private final ConnectionString connectionString;
private final TelemetryClient telemetryClient;

@Nullable private volatile String appId;

// TODO (kryalama) do we still need this AtomicBoolean, or can we use throttling built in to the
// warningLogger?
private static final AtomicBoolean friendlyExceptionThrown = new AtomicBoolean();

public AppIdSupplier(ConnectionString connectionString) {
this.connectionString = connectionString;
public AppIdSupplier(TelemetryClient telemetryClient) {
this.telemetryClient = telemetryClient;
}

public void startAppIdRetrieval() {
public void updateAppId() {
ConnectionString connectionString = telemetryClient.getConnectionString();
if (connectionString == null) {
appId = null;
return;
}

GetAppIdTask newTask;
try {
newTask = new GetAppIdTask(getAppIdUrl(connectionString));
Expand Down Expand Up @@ -102,15 +108,8 @@ static URL getAppIdUrl(ConnectionString connectionString) throws MalformedURLExc

@Override
public String get() {
String instrumentationKey = TelemetryClient.getActive().getInstrumentationKey();
if (instrumentationKey == null) {
// this is possible in Azure Function consumption plan prior to "specialization"
return "";
}

// it's possible the appId returned is null (e.g. async task is still pending or has failed). In
// this case, just
// return and let the next request resolve the ikey.
// this case, just return and let the next request resolve the ikey.
if (appId == null) {
logger.debug("appId has not been retrieved yet (e.g. task may be pending or failed)");
return "";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
package com.microsoft.applicationinsights.agent.internal.init;

import ch.qos.logback.classic.LoggerContext;
import com.azure.monitor.opentelemetry.exporter.implementation.configuration.ConnectionString;
import com.azure.monitor.opentelemetry.exporter.implementation.configuration.StatsbeatConnectionString;
import com.azure.monitor.opentelemetry.exporter.implementation.utils.Strings;
import com.microsoft.applicationinsights.agent.bootstrap.diagnostics.DiagnosticsHelper;
import com.microsoft.applicationinsights.agent.internal.configuration.Configuration;
Expand Down Expand Up @@ -122,18 +120,13 @@ void setConnectionString(@Nullable String connectionString, @Nullable String ins
}

private void setValue(String value) {
ConnectionString connectionString = ConnectionString.parse(value);
telemetryClient.updateConnectionString(connectionString);
telemetryClient.updateStatsbeatConnectionString(
StatsbeatConnectionString.create(connectionString, null, null));
telemetryClient.updateConnectionStrings(value, null, null);
appIdSupplier.updateAppId();

// now that we know the user has opted in to tracing, we need to init the propagator and sampler
DelegatingPropagator.getInstance().setUpStandardDelegate(Collections.emptyList(), false);
// TODO handle APPLICATIONINSIGHTS_SAMPLING_PERCENTAGE
DelegatingSampler.getInstance().setAlwaysOnDelegate();

// start app id retrieval after the connection string becomes available.
appIdSupplier.startAppIdRetrieval();
}

void setWebsiteSiteName(@Nullable String websiteSiteName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@

import static java.util.concurrent.TimeUnit.SECONDS;

import com.azure.monitor.opentelemetry.exporter.implementation.configuration.ConnectionString;
import com.azure.monitor.opentelemetry.exporter.implementation.configuration.StatsbeatConnectionString;
import com.azure.monitor.opentelemetry.exporter.implementation.utils.Strings;
import com.azure.monitor.opentelemetry.exporter.implementation.utils.ThreadPoolUtils;
import com.microsoft.applicationinsights.agent.internal.configuration.Configuration;
import com.microsoft.applicationinsights.agent.internal.configuration.ConfigurationBuilder;
Expand Down Expand Up @@ -103,18 +100,11 @@ public void run() {
if (!newRpConfiguration.connectionString.equals(rpConfiguration.connectionString)) {
logger.debug(
"Connection string from the JSON config file is overriding the previously configured connection string.");
ConnectionString connectionString =
ConnectionString.parse(newRpConfiguration.connectionString);
telemetryClient.updateConnectionString(connectionString);
telemetryClient.updateStatsbeatConnectionString(
StatsbeatConnectionString.create(
connectionString,
configuration.internal.statsbeat.instrumentationKey,
configuration.internal.statsbeat.endpoint));

if (!Strings.isNullOrEmpty(newRpConfiguration.connectionString)) {
appIdSupplier.startAppIdRetrieval();
}
telemetryClient.updateConnectionStrings(
newRpConfiguration.connectionString,
configuration.internal.statsbeat.instrumentationKey,
configuration.internal.statsbeat.endpoint);
appIdSupplier.updateAppId();
}

if (newRpConfiguration.sampling.percentage != rpConfiguration.sampling.percentage) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public void customize(AutoConfigurationCustomizer autoConfiguration) {
BytecodeUtilImpl.samplingPercentage = config.sampling.percentage;
BytecodeUtilImpl.featureStatsbeat = statsbeatModule.getFeatureStatsbeat();

AppIdSupplier appIdSupplier = new AppIdSupplier(telemetryClient.getConnectionString());
AppIdSupplier appIdSupplier = new AppIdSupplier(telemetryClient);
AiAppId.setSupplier(appIdSupplier);

if (config.preview.profiler.enabled) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.azure.monitor.opentelemetry.exporter.implementation.pipeline.TelemetryPipeline;
import com.azure.monitor.opentelemetry.exporter.implementation.pipeline.TelemetryPipelineListener;
import com.azure.monitor.opentelemetry.exporter.implementation.quickpulse.QuickPulse;
import com.azure.monitor.opentelemetry.exporter.implementation.utils.Strings;
import com.azure.monitor.opentelemetry.exporter.implementation.utils.TempDirs;
import com.microsoft.applicationinsights.agent.internal.common.PropertyHelper;
import com.microsoft.applicationinsights.agent.internal.configuration.Configuration;
Expand Down Expand Up @@ -409,13 +410,21 @@ public String getRoleInstance() {
}

// used during Azure Functions placeholder specialization
public void updateConnectionString(ConnectionString connectionString) {
this.connectionString = connectionString;
}

// used during Azure Functions placeholder specialization
public void updateStatsbeatConnectionString(StatsbeatConnectionString statsbeatConnectionString) {
this.statsbeatConnectionString = statsbeatConnectionString;
// and also used by Azure Spring Apps dynamic configuration
public void updateConnectionStrings(
@Nullable String connectionString,
@Nullable String statsbeatInstrumentationKey,
@Nullable String statsbeatEndpoint) {

if (Strings.isNullOrEmpty(connectionString)) {
this.connectionString = null;
this.statsbeatConnectionString = null;
} else {
this.connectionString = ConnectionString.parse(connectionString);
this.statsbeatConnectionString =
StatsbeatConnectionString.create(
this.connectionString, statsbeatInstrumentationKey, statsbeatEndpoint);
}
}

@Nullable
Expand Down Expand Up @@ -510,7 +519,11 @@ public Builder setConnectionStrings(
@Nullable String connectionString,
@Nullable String statsbeatInstrumentationKey,
@Nullable String statsbeatEndpoint) {
if (connectionString != null) {

if (Strings.isNullOrEmpty(connectionString)) {
this.connectionString = null;
this.statsbeatConnectionString = null;
} else {
this.connectionString = ConnectionString.parse(connectionString);
this.statsbeatConnectionString =
StatsbeatConnectionString.create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.argThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;
Expand Down Expand Up @@ -88,7 +87,7 @@ void disableLazySetWithLazySetOptInOffConnectionStringNullInstrumentationKeyNull
lazyConfigurationAccessor.setConnectionString(null, null);

// then
verify(telemetryClient, never()).updateConnectionString(any());
verify(telemetryClient, never()).updateConnectionStrings(any(), any(), any());
}

@Test
Expand All @@ -108,9 +107,7 @@ void disableLazySetWithLazySetOptInOffConnectionStringNotNullInstrumentationKeyN
lazyConfigurationAccessor.setConnectionString(CONNECTION_STRING, null);

// then
verify(telemetryClient)
.updateConnectionString(
argThat(cs -> cs.getInstrumentationKey().equals(INSTRUMENTATION_KEY)));
verify(telemetryClient).updateConnectionStrings(CONNECTION_STRING, null, null);

// when
lazyConfigurationAccessor.setWebsiteSiteName(WEBSITE_SITE_NAME);
Expand All @@ -136,9 +133,7 @@ void enableLazySetWithLazySetOptInOffConnectionStringNullInstrumentationKeyNotNu
lazyConfigurationAccessor.setConnectionString(null, INSTRUMENTATION_KEY);

// then
verify(telemetryClient)
.updateConnectionString(
argThat(cs -> cs.getInstrumentationKey().equals(INSTRUMENTATION_KEY)));
verify(telemetryClient).updateConnectionStrings(CONNECTION_STRING, null, null);
}

@Test
Expand Down Expand Up @@ -176,7 +171,7 @@ void disableLazySetWithLazySetOptInOnConnectionStringNullAndInstrumentationKeyNu
lazyConfigurationAccessor.setConnectionString(null, null);

// then
verify(telemetryClient, never()).updateConnectionString(any());
verify(telemetryClient, never()).updateConnectionStrings(any(), any(), any());
}

@Test
Expand All @@ -196,9 +191,7 @@ void enableLazySetWithLazySetOptInOnConnectionStringNotNullInstrumentationKeyNul
lazyConfigurationAccessor.setConnectionString(CONNECTION_STRING, null);

// then
verify(telemetryClient)
.updateConnectionString(
argThat(cs -> cs.getInstrumentationKey().equals(INSTRUMENTATION_KEY)));
verify(telemetryClient).updateConnectionStrings(CONNECTION_STRING, null, null);
}

@Test
Expand All @@ -218,8 +211,6 @@ void enableLazySetWithLazySetOptInOnConnectionStringNullInstrumentationKeyNotNul
lazyConfigurationAccessor.setConnectionString(null, INSTRUMENTATION_KEY);

// then
verify(telemetryClient)
.updateConnectionString(
argThat(cs -> cs.getInstrumentationKey().equals(INSTRUMENTATION_KEY)));
verify(telemetryClient).updateConnectionStrings(CONNECTION_STRING, null, null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@

import static org.assertj.core.api.Assertions.assertThat;

import com.azure.monitor.opentelemetry.exporter.implementation.configuration.ConnectionString;
import com.azure.monitor.opentelemetry.exporter.implementation.utils.TelemetryUtil;
import com.microsoft.applicationinsights.agent.internal.configuration.Configuration;
import com.microsoft.applicationinsights.agent.internal.configuration.RpConfiguration;
Expand Down Expand Up @@ -80,10 +79,9 @@ void shouldUpdate() throws URISyntaxException {
rpConfiguration.lastModifiedTime = 0;

TelemetryClient telemetryClient = TelemetryClient.createForTest();
ConnectionString connectionString =
ConnectionString.parse("InstrumentationKey=00000000-0000-0000-0000-000000000000");
telemetryClient.updateConnectionString(connectionString);
AppIdSupplier appIdSupplier = new AppIdSupplier(connectionString);
telemetryClient.updateConnectionStrings(
"InstrumentationKey=00000000-0000-0000-0000-000000000000", null, null);
AppIdSupplier appIdSupplier = new AppIdSupplier(telemetryClient);

BytecodeUtilImpl.samplingPercentage = 100;

Expand Down Expand Up @@ -116,10 +114,9 @@ void shouldBePopulatedByEnvVars() throws URISyntaxException {
rpConfiguration.lastModifiedTime = 0;

TelemetryClient telemetryClient = TelemetryClient.createForTest();
ConnectionString connectionString =
ConnectionString.parse("InstrumentationKey=00000000-0000-0000-0000-000000000000");
telemetryClient.updateConnectionString(connectionString);
AppIdSupplier appIdSupplier = new AppIdSupplier(connectionString);
telemetryClient.updateConnectionStrings(
"InstrumentationKey=00000000-0000-0000-0000-000000000000", null, null);
AppIdSupplier appIdSupplier = new AppIdSupplier(telemetryClient);

BytecodeUtilImpl.samplingPercentage = 100;

Expand Down

0 comments on commit 53cc55c

Please sign in to comment.