From a20bd3850a5eed2d521c2d7a4e025487239c266f Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Wed, 17 Nov 2021 12:30:06 -0800 Subject: [PATCH 01/18] Check readonly for StatusFile --- .../agent/bootstrap/diagnostics/status/StatusFile.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index 460d340692f..0cfc4dace41 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -140,7 +140,12 @@ private StatusFile() {} // visible for testing static boolean shouldWrite() { - return DiagnosticsHelper.useAppSvcRpIntegrationLogging(); + return DiagnosticsHelper.useAppSvcRpIntegrationLogging() && !isReadOnlyFileSystem(); + } + + private static boolean isReadOnlyFileSystem() { + File dir = new File(directory); + return dir.canRead() && !dir.canWrite(); } public static void putValueAndWrite(String key, T value) { From e1841e01ce9c0b9702d40b143c6beace07edf07c Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Wed, 17 Nov 2021 12:30:23 -0800 Subject: [PATCH 02/18] Enable StatusFileTest --- .../agent/bootstrap/diagnostics/status/StatusFileTests.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java index 815ee34fbc2..f4f45ef3ac3 100644 --- a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java +++ b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java @@ -38,7 +38,6 @@ import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; @@ -46,8 +45,6 @@ import uk.org.webcompere.systemstubs.jupiter.SystemStub; import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension; -// FIXME (trask) failing in CI on deleting the the @TempDir -@Disabled @ExtendWith(SystemStubsExtension.class) class StatusFileTests { From 1c516b3e386c9ca25853b65614bbb0e85710238b Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Wed, 17 Nov 2021 16:30:03 -0800 Subject: [PATCH 03/18] Address a comment --- .../diagnostics/status/StatusFile.java | 27 ++++++++++--------- .../diagnostics/status/StatusFileTests.java | 4 +++ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index 0cfc4dace41..8d75ce087fd 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -84,6 +84,9 @@ public class StatusFile { // visible for testing static String directory; + // visible for testing + static boolean shouldWrite; + private static final Object lock = new Object(); // guarded by lock @@ -109,6 +112,14 @@ public class StatusFile { logDir = initLogDir(); directory = logDir + STATUS_FILE_DIRECTORY; + shouldWrite = + DiagnosticsHelper.useAppSvcRpIntegrationLogging() && new File(directory).canWrite(); + if (!shouldWrite) { + LoggerFactory.getLogger(StatusFile.class) + .info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + directory); + } } private static Thread newThread(Runnable r) { @@ -138,22 +149,12 @@ public static String getLogDir() { private StatusFile() {} - // visible for testing - static boolean shouldWrite() { - return DiagnosticsHelper.useAppSvcRpIntegrationLogging() && !isReadOnlyFileSystem(); - } - - private static boolean isReadOnlyFileSystem() { - File dir = new File(directory); - return dir.canRead() && !dir.canWrite(); - } - public static void putValueAndWrite(String key, T value) { putValueAndWrite(key, value, true); } public static void putValueAndWrite(String key, T value, boolean loggingInitialized) { - if (!shouldWrite()) { + if (!shouldWrite) { return; } CONSTANT_VALUES.put(key, value); @@ -161,7 +162,7 @@ public static void putValueAndWrite(String key, T value, boolean loggingInit } public static void putValue(String key, T value) { - if (!shouldWrite()) { + if (!shouldWrite) { return; } CONSTANT_VALUES.put(key, value); @@ -173,7 +174,7 @@ public static void write() { @SuppressWarnings("SystemOut") private static void write(boolean loggingInitialized) { - if (!shouldWrite()) { + if (!shouldWrite) { return; } WRITER_THREAD.submit( diff --git a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java index f4f45ef3ac3..fd0ad8ff14e 100644 --- a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java +++ b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java @@ -142,6 +142,8 @@ private void runWriteFileTest(boolean enabled) throws Exception { assertThat(tempFolder.list()).isEmpty(); StatusFile.directory = tempFolder.getAbsolutePath(); + StatusFile.shouldWrite = + DiagnosticsHelper.useAppSvcRpIntegrationLogging() && tempFolder.canWrite(); StatusFile.write(); pauseForFileWrite(); @@ -188,6 +190,8 @@ void putValueAndWriteOverwritesCurrentFile() throws Exception { DiagnosticsTestHelper.setIsAppSvcAttachForLoggingPurposes(true); StatusFile.directory = tempFolder.getAbsolutePath(); + StatusFile.shouldWrite = + DiagnosticsHelper.useAppSvcRpIntegrationLogging() && tempFolder.canWrite(); assertThat(tempFolder.isDirectory()).isTrue(); assertThat(tempFolder.list()).isEmpty(); StatusFile.write(); From c6417ae0fe0e38aef6d33d01717310a1f24bc31d Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 13:50:34 -0800 Subject: [PATCH 04/18] Check shouldWrite at callers --- .../diagnostics/etw/EtwAppender.java | 23 +++++++++++++++---- .../diagnostics/status/StatusFile.java | 21 ++--------------- .../diagnostics/status/StatusFileTests.java | 10 ++++++-- .../agent/internal/init/MainEntryPoint.java | 9 +++++++- .../TelemetryClientClassFileTransformer.java | 9 +++++++- .../agent/internal/statsbeat/Feature.java | 3 ++- 6 files changed, 47 insertions(+), 28 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java index 70fbdae6d27..08d99886ebb 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java @@ -62,14 +62,29 @@ public void start() { LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME).error(message, e); addError(message, e); - StatusFile.putValue("EtwProviderInitialized", "false"); - StatusFile.putValue("EtwProviderError", e.getLocalizedMessage()); - StatusFile.write(); + if (StatusFile.shouldWrite) { + StatusFile.putValue("EtwProviderInitialized", "false"); + StatusFile.putValue("EtwProviderError", e.getLocalizedMessage()); + StatusFile.write(); + } else { + LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) + .info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } return; // appender fails to start } - StatusFile.putValueAndWrite("EtwProviderInitialized", "true"); + if (StatusFile.shouldWrite) { + StatusFile.putValueAndWrite("EtwProviderInitialized", "true"); + } else { + LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) + .info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } + super.start(); } diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index 8d75ce087fd..d276e2aa89a 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -81,11 +81,9 @@ public class StatusFile { // visible for testing static String logDir; - // visible for testing - static String directory; + public static String directory; - // visible for testing - static boolean shouldWrite; + public static boolean shouldWrite; private static final Object lock = new Object(); @@ -114,12 +112,6 @@ public class StatusFile { directory = logDir + STATUS_FILE_DIRECTORY; shouldWrite = DiagnosticsHelper.useAppSvcRpIntegrationLogging() && new File(directory).canWrite(); - if (!shouldWrite) { - LoggerFactory.getLogger(StatusFile.class) - .info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - directory); - } } private static Thread newThread(Runnable r) { @@ -154,17 +146,11 @@ public static void putValueAndWrite(String key, T value) { } public static void putValueAndWrite(String key, T value, boolean loggingInitialized) { - if (!shouldWrite) { - return; - } CONSTANT_VALUES.put(key, value); write(loggingInitialized); } public static void putValue(String key, T value) { - if (!shouldWrite) { - return; - } CONSTANT_VALUES.put(key, value); } @@ -174,9 +160,6 @@ public static void write() { @SuppressWarnings("SystemOut") private static void write(boolean loggingInitialized) { - if (!shouldWrite) { - return; - } WRITER_THREAD.submit( new Runnable() { @Override diff --git a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java index fd0ad8ff14e..64a1636f496 100644 --- a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java +++ b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java @@ -175,10 +175,16 @@ void doesNotWriteIfNotAppService() throws Exception { StatusFile.directory = tempFolder.getAbsolutePath(); assertThat(tempFolder.isDirectory()).isTrue(); assertThat(tempFolder.list()).isEmpty(); - StatusFile.write(); + StatusFile.shouldWrite = + DiagnosticsHelper.useAppSvcRpIntegrationLogging() && tempFolder.canWrite(); + if (StatusFile.shouldWrite) { + StatusFile.write(); + } pauseForFileWrite(); assertThat(tempFolder.list()).isEmpty(); - StatusFile.putValueAndWrite("shouldNot", "write"); + if (StatusFile.shouldWrite) { + StatusFile.putValueAndWrite("shouldNot", "write"); + } pauseForFileWrite(); assertThat(tempFolder.list()).isEmpty(); } diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java index 4a99e6589cd..84068e4c107 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java @@ -117,7 +117,14 @@ public static void start(Instrumentation instrumentation, File javaagentFile) { } finally { try { - StatusFile.putValueAndWrite("AgentInitializedSuccessfully", success, startupLogger != null); + if (StatusFile.shouldWrite) { + StatusFile.putValueAndWrite( + "AgentInitializedSuccessfully", success, startupLogger != null); + } else { + startupLogger.info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } } catch (Throwable t) { if (startupLogger != null) { startupLogger.error("Error writing status.json", t); diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java index ec8ad227467..2d4c3f225b7 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java @@ -86,7 +86,14 @@ public class TelemetryClientClassFileTransformer implements ClassFileTransformer return null; } - StatusFile.putValueAndWrite("SDKPresent", true); + if (StatusFile.shouldWrite) { + StatusFile.putValueAndWrite( + "SDKPresent", true); // TODO (heya) track this via FeatureStatsbeat + } else { + logger.info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } try { ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS); TelemetryClientClassVisitor cv = new TelemetryClientClassVisitor(cw); diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java index d5d501e2a5c..25c8b218997 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java @@ -53,7 +53,8 @@ enum Feature { APACHE_CAMEL_DISABLED(22), // preview instrumentation, apache camel is ON by default in OTEL AKKA_DISABLED(23), // preview instrumentation, akka is ON by default in OTEL PROPAGATION_DISABLED(24), - PLAY_DISABLED(25); // preview instrumentation, play is ON by default in OTEL + PLAY_DISABLED(25), // preview instrumentation, play is ON by default in OTEL + SDK_PRESENT(26); // sdk is used with the combination of the Java auto-instrumentaiton agent private static final Map javaVendorFeatureMap; From fd953945f4ddf23bc23fb932f2af9a20fdef5848 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 13:56:01 -0800 Subject: [PATCH 05/18] Revert Feature.java --- .../applicationinsights/agent/internal/statsbeat/Feature.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java index 25c8b218997..d5d501e2a5c 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/Feature.java @@ -53,8 +53,7 @@ enum Feature { APACHE_CAMEL_DISABLED(22), // preview instrumentation, apache camel is ON by default in OTEL AKKA_DISABLED(23), // preview instrumentation, akka is ON by default in OTEL PROPAGATION_DISABLED(24), - PLAY_DISABLED(25), // preview instrumentation, play is ON by default in OTEL - SDK_PRESENT(26); // sdk is used with the combination of the Java auto-instrumentaiton agent + PLAY_DISABLED(25); // preview instrumentation, play is ON by default in OTEL private static final Map javaVendorFeatureMap; From 68812bdd278e2f6524cf7b5459727ab9928bb997 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 14:01:15 -0800 Subject: [PATCH 06/18] Only log in app service env --- .../diagnostics/etw/EtwAppender.java | 20 +++++++++++-------- .../agent/internal/init/MainEntryPoint.java | 8 +++++--- .../TelemetryClientClassFileTransformer.java | 9 ++++++--- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java index 08d99886ebb..fbb92c24a01 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java @@ -67,10 +67,12 @@ public void start() { StatusFile.putValue("EtwProviderError", e.getLocalizedMessage()); StatusFile.write(); } else { - LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) - .info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); + if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { + LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) + .info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } } return; // appender fails to start @@ -79,10 +81,12 @@ public void start() { if (StatusFile.shouldWrite) { StatusFile.putValueAndWrite("EtwProviderInitialized", "true"); } else { - LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) - .info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); + if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { + LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) + .info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } } super.start(); diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java index 84068e4c107..c7253d40460 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java @@ -121,9 +121,11 @@ public static void start(Instrumentation instrumentation, File javaagentFile) { StatusFile.putValueAndWrite( "AgentInitializedSuccessfully", success, startupLogger != null); } else { - startupLogger.info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); + if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { + startupLogger.info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } } } catch (Throwable t) { if (startupLogger != null) { diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java index 2d4c3f225b7..0cbcb4b7b4e 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java @@ -47,6 +47,7 @@ import static net.bytebuddy.jar.asm.Opcodes.NEW; import static net.bytebuddy.jar.asm.Opcodes.RETURN; +import com.microsoft.applicationinsights.agent.bootstrap.diagnostics.DiagnosticsHelper; import com.microsoft.applicationinsights.agent.bootstrap.diagnostics.status.StatusFile; import java.lang.instrument.ClassFileTransformer; import java.security.ProtectionDomain; @@ -90,9 +91,11 @@ public class TelemetryClientClassFileTransformer implements ClassFileTransformer StatusFile.putValueAndWrite( "SDKPresent", true); // TODO (heya) track this via FeatureStatsbeat } else { - logger.info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); + if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { + logger.info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + StatusFile.directory); + } } try { ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS); From 0007b9c9a3407fed3a8cf502cf5c0717ad5ffff0 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 15:06:21 -0800 Subject: [PATCH 07/18] Fix lgtm --- .../applicationinsights/agent/internal/init/MainEntryPoint.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java index c7253d40460..fa825255269 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java @@ -121,7 +121,7 @@ public static void start(Instrumentation instrumentation, File javaagentFile) { StatusFile.putValueAndWrite( "AgentInitializedSuccessfully", success, startupLogger != null); } else { - if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { + if (DiagnosticsHelper.useAppSvcRpIntegrationLogging() && startupLogger != null) { startupLogger.info( "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", StatusFile.directory); From be0c91154f2ab15b75a6130d7b12e82f199820eb Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 16:13:40 -0800 Subject: [PATCH 08/18] Refactor --- .../diagnostics/etw/EtwAppender.java | 27 +++-------------- .../diagnostics/status/StatusFile.java | 30 ++++++++++++++++++- .../agent/internal/init/MainEntryPoint.java | 12 ++------ .../TelemetryClientClassFileTransformer.java | 14 ++------- 4 files changed, 38 insertions(+), 45 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java index fbb92c24a01..70fbdae6d27 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/etw/EtwAppender.java @@ -62,33 +62,14 @@ public void start() { LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME).error(message, e); addError(message, e); - if (StatusFile.shouldWrite) { - StatusFile.putValue("EtwProviderInitialized", "false"); - StatusFile.putValue("EtwProviderError", e.getLocalizedMessage()); - StatusFile.write(); - } else { - if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { - LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) - .info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); - } - } + StatusFile.putValue("EtwProviderInitialized", "false"); + StatusFile.putValue("EtwProviderError", e.getLocalizedMessage()); + StatusFile.write(); return; // appender fails to start } - if (StatusFile.shouldWrite) { - StatusFile.putValueAndWrite("EtwProviderInitialized", "true"); - } else { - if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { - LoggerFactory.getLogger(DiagnosticsHelper.DIAGNOSTICS_LOGGER_NAME) - .info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); - } - } - + StatusFile.putValueAndWrite("EtwProviderInitialized", "true"); super.start(); } diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index d276e2aa89a..e949694cf5d 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -41,6 +41,7 @@ import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import okio.BufferedSink; import okio.Okio; import org.checkerframework.checker.nullness.qual.Nullable; @@ -83,7 +84,9 @@ public class StatusFile { public static String directory; - public static boolean shouldWrite; + static boolean shouldWrite; + + private static final AtomicBoolean alreadyLogged = new AtomicBoolean(); private static final Object lock = new Object(); @@ -93,6 +96,8 @@ public class StatusFile { // guarded by lock private static BufferedSink buffer; + public static Logger startupLogger; + private static final ThreadPoolExecutor WRITER_THREAD = new ThreadPoolExecutor( 1, 1, 750L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), StatusFile::newThread); @@ -141,16 +146,35 @@ public static String getLogDir() { private StatusFile() {} + private static void logReadOnlyOnce() { + if (!alreadyLogged.get()) { + if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { + startupLogger.info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + directory); + alreadyLogged.set(true); + } + } + } + public static void putValueAndWrite(String key, T value) { putValueAndWrite(key, value, true); } public static void putValueAndWrite(String key, T value, boolean loggingInitialized) { + if (!shouldWrite) { + logReadOnlyOnce(); + return; + } CONSTANT_VALUES.put(key, value); write(loggingInitialized); } public static void putValue(String key, T value) { + if (!shouldWrite) { + logReadOnlyOnce(); + return; + } CONSTANT_VALUES.put(key, value); } @@ -160,6 +184,10 @@ public static void write() { @SuppressWarnings("SystemOut") private static void write(boolean loggingInitialized) { + if (!shouldWrite) { + logReadOnlyOnce(); + return; + } WRITER_THREAD.submit( new Runnable() { @Override diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java index fa825255269..a69b357878c 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/init/MainEntryPoint.java @@ -89,6 +89,7 @@ public static void start(Instrumentation instrumentation, File javaagentFile) { rpConfiguration = RpConfigurationBuilder.create(agentPath); configuration = ConfigurationBuilder.create(agentPath, rpConfiguration); startupLogger = configureLogging(configuration.selfDiagnostics, agentPath); + StatusFile.startupLogger = startupLogger; ConfigurationBuilder.logConfigurationWarnMessages(); MDC.put(DiagnosticsHelper.MDC_PROP_OPERATION, "Startup"); // TODO convert to agent builder concept @@ -117,16 +118,7 @@ public static void start(Instrumentation instrumentation, File javaagentFile) { } finally { try { - if (StatusFile.shouldWrite) { - StatusFile.putValueAndWrite( - "AgentInitializedSuccessfully", success, startupLogger != null); - } else { - if (DiagnosticsHelper.useAppSvcRpIntegrationLogging() && startupLogger != null) { - startupLogger.info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); - } - } + StatusFile.putValueAndWrite("AgentInitializedSuccessfully", success, startupLogger != null); } catch (Throwable t) { if (startupLogger != null) { startupLogger.error("Error writing status.json", t); diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java index 0cbcb4b7b4e..be6e4f99f42 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java @@ -47,7 +47,6 @@ import static net.bytebuddy.jar.asm.Opcodes.NEW; import static net.bytebuddy.jar.asm.Opcodes.RETURN; -import com.microsoft.applicationinsights.agent.bootstrap.diagnostics.DiagnosticsHelper; import com.microsoft.applicationinsights.agent.bootstrap.diagnostics.status.StatusFile; import java.lang.instrument.ClassFileTransformer; import java.security.ProtectionDomain; @@ -87,16 +86,9 @@ public class TelemetryClientClassFileTransformer implements ClassFileTransformer return null; } - if (StatusFile.shouldWrite) { - StatusFile.putValueAndWrite( - "SDKPresent", true); // TODO (heya) track this via FeatureStatsbeat - } else { - if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { - logger.info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - StatusFile.directory); - } - } + // TODO (heya) track this via FeatureStatsbeat + StatusFile.putValueAndWrite("SDKPresent", true); + try { ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS); TelemetryClientClassVisitor cv = new TelemetryClientClassVisitor(cw); From f44734698b7b82e97556b260f6f7a135e1f096cd Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 16:16:18 -0800 Subject: [PATCH 09/18] Revert directory --- .../agent/bootstrap/diagnostics/status/StatusFile.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index e949694cf5d..81fcda8eb1b 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -82,8 +82,10 @@ public class StatusFile { // visible for testing static String logDir; - public static String directory; + // visible for testing + static String directory; + // visible for testing static boolean shouldWrite; private static final AtomicBoolean alreadyLogged = new AtomicBoolean(); From 4ecf71c61e1408a953efff50439811e4384e349b Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 16:20:41 -0800 Subject: [PATCH 10/18] Update test --- .../bootstrap/diagnostics/status/StatusFileTests.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java index 64a1636f496..67c6874c657 100644 --- a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java +++ b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java @@ -177,14 +177,10 @@ void doesNotWriteIfNotAppService() throws Exception { assertThat(tempFolder.list()).isEmpty(); StatusFile.shouldWrite = DiagnosticsHelper.useAppSvcRpIntegrationLogging() && tempFolder.canWrite(); - if (StatusFile.shouldWrite) { - StatusFile.write(); - } + StatusFile.write(); pauseForFileWrite(); assertThat(tempFolder.list()).isEmpty(); - if (StatusFile.shouldWrite) { - StatusFile.putValueAndWrite("shouldNot", "write"); - } + StatusFile.putValueAndWrite("shouldNot", "write"); pauseForFileWrite(); assertThat(tempFolder.list()).isEmpty(); } From ac3ff83e698aedaa710c4a4c58ce967679f05d2f Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 18:10:50 -0800 Subject: [PATCH 11/18] Prevent too early to call DiagnosticsHelper.userAppSvcRpIntegrationLogging --- .../diagnostics/status/StatusFile.java | 17 ++++++++++------- .../diagnostics/status/StatusFileTests.java | 6 ------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index 81fcda8eb1b..87964a1c497 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -85,8 +85,7 @@ public class StatusFile { // visible for testing static String directory; - // visible for testing - static boolean shouldWrite; + private static final boolean isReadOnly; private static final AtomicBoolean alreadyLogged = new AtomicBoolean(); @@ -117,8 +116,8 @@ public class StatusFile { logDir = initLogDir(); directory = logDir + STATUS_FILE_DIRECTORY; - shouldWrite = - DiagnosticsHelper.useAppSvcRpIntegrationLogging() && new File(directory).canWrite(); + File dir = new File(directory); + isReadOnly = dir.canRead() && !dir.canWrite(); } private static Thread newThread(Runnable r) { @@ -159,12 +158,16 @@ private static void logReadOnlyOnce() { } } + private static boolean shouldWrite() { + return DiagnosticsHelper.useAppSvcRpIntegrationLogging() && !isReadOnly; + } + public static void putValueAndWrite(String key, T value) { putValueAndWrite(key, value, true); } public static void putValueAndWrite(String key, T value, boolean loggingInitialized) { - if (!shouldWrite) { + if (!shouldWrite()) { logReadOnlyOnce(); return; } @@ -173,7 +176,7 @@ public static void putValueAndWrite(String key, T value, boolean loggingInit } public static void putValue(String key, T value) { - if (!shouldWrite) { + if (!shouldWrite()) { logReadOnlyOnce(); return; } @@ -186,7 +189,7 @@ public static void write() { @SuppressWarnings("SystemOut") private static void write(boolean loggingInitialized) { - if (!shouldWrite) { + if (!shouldWrite()) { logReadOnlyOnce(); return; } diff --git a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java index 67c6874c657..f4f45ef3ac3 100644 --- a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java +++ b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java @@ -142,8 +142,6 @@ private void runWriteFileTest(boolean enabled) throws Exception { assertThat(tempFolder.list()).isEmpty(); StatusFile.directory = tempFolder.getAbsolutePath(); - StatusFile.shouldWrite = - DiagnosticsHelper.useAppSvcRpIntegrationLogging() && tempFolder.canWrite(); StatusFile.write(); pauseForFileWrite(); @@ -175,8 +173,6 @@ void doesNotWriteIfNotAppService() throws Exception { StatusFile.directory = tempFolder.getAbsolutePath(); assertThat(tempFolder.isDirectory()).isTrue(); assertThat(tempFolder.list()).isEmpty(); - StatusFile.shouldWrite = - DiagnosticsHelper.useAppSvcRpIntegrationLogging() && tempFolder.canWrite(); StatusFile.write(); pauseForFileWrite(); assertThat(tempFolder.list()).isEmpty(); @@ -192,8 +188,6 @@ void putValueAndWriteOverwritesCurrentFile() throws Exception { DiagnosticsTestHelper.setIsAppSvcAttachForLoggingPurposes(true); StatusFile.directory = tempFolder.getAbsolutePath(); - StatusFile.shouldWrite = - DiagnosticsHelper.useAppSvcRpIntegrationLogging() && tempFolder.canWrite(); assertThat(tempFolder.isDirectory()).isTrue(); assertThat(tempFolder.list()).isEmpty(); StatusFile.write(); From 4ac653904ce0470d18ca133c6971b9344855193b Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 18:25:48 -0800 Subject: [PATCH 12/18] Remove a space in telemetry name --- .../agent/internal/statsbeat/NetworkStatsbeat.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java index 50367196994..d12fe1c9878 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java @@ -35,7 +35,7 @@ public class NetworkStatsbeat extends BaseStatsbeat { private static final String REQUEST_SUCCESS_COUNT_METRIC_NAME = "Request Success Count"; - private static final String REQUEST_FAILURE_COUNT_METRIC_NAME = "Requests Failure Count "; + private static final String REQUEST_FAILURE_COUNT_METRIC_NAME = "Requests Failure Count"; private static final String REQUEST_DURATION_METRIC_NAME = "Request Duration"; private static final String RETRY_COUNT_METRIC_NAME = "Retry Count"; private static final String THROTTLE_COUNT_METRIC_NAME = "Throttle Count"; From cc09aea97cfe8e9c64738a8f9d4072c898c1c572 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Thu, 18 Nov 2021 18:26:42 -0800 Subject: [PATCH 13/18] Remove plural --- .../agent/internal/statsbeat/NetworkStatsbeat.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java index d12fe1c9878..ca057359c31 100644 --- a/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java +++ b/agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/statsbeat/NetworkStatsbeat.java @@ -35,7 +35,7 @@ public class NetworkStatsbeat extends BaseStatsbeat { private static final String REQUEST_SUCCESS_COUNT_METRIC_NAME = "Request Success Count"; - private static final String REQUEST_FAILURE_COUNT_METRIC_NAME = "Requests Failure Count"; + private static final String REQUEST_FAILURE_COUNT_METRIC_NAME = "Request Failure Count"; private static final String REQUEST_DURATION_METRIC_NAME = "Request Duration"; private static final String RETRY_COUNT_METRIC_NAME = "Retry Count"; private static final String THROTTLE_COUNT_METRIC_NAME = "Throttle Count"; From 84455b1158e6f020cc0910ad087713b5b55d4627 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Fri, 19 Nov 2021 13:32:40 -0800 Subject: [PATCH 14/18] simplify a bit --- .../diagnostics/status/StatusFile.java | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index 87964a1c497..d045f7ada58 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -147,19 +147,20 @@ public static String getLogDir() { private StatusFile() {} - private static void logReadOnlyOnce() { - if (!alreadyLogged.get()) { - if (DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { - startupLogger.info( - "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", - directory); - alreadyLogged.set(true); - } - } - } - private static boolean shouldWrite() { - return DiagnosticsHelper.useAppSvcRpIntegrationLogging() && !isReadOnly; + if (!DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { + return false; + } + if (!isReadOnly) { + return true; + } + // read-only app services, want to log warning once in this case + if (!alreadyLogged.getAndSet(true)) { + startupLogger.info( + "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", + directory); + } + return false; } public static void putValueAndWrite(String key, T value) { @@ -168,7 +169,6 @@ public static void putValueAndWrite(String key, T value) { public static void putValueAndWrite(String key, T value, boolean loggingInitialized) { if (!shouldWrite()) { - logReadOnlyOnce(); return; } CONSTANT_VALUES.put(key, value); @@ -177,7 +177,6 @@ public static void putValueAndWrite(String key, T value, boolean loggingInit public static void putValue(String key, T value) { if (!shouldWrite()) { - logReadOnlyOnce(); return; } CONSTANT_VALUES.put(key, value); @@ -190,7 +189,6 @@ public static void write() { @SuppressWarnings("SystemOut") private static void write(boolean loggingInitialized) { if (!shouldWrite()) { - logReadOnlyOnce(); return; } WRITER_THREAD.submit( From 77fc12e1df7d8986350269655fdbd34785e7a02b Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Fri, 19 Nov 2021 14:21:20 -0800 Subject: [PATCH 15/18] Check parent dir --- .../agent/bootstrap/diagnostics/status/StatusFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index d045f7ada58..ac68fda2c6d 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -116,7 +116,7 @@ public class StatusFile { logDir = initLogDir(); directory = logDir + STATUS_FILE_DIRECTORY; - File dir = new File(directory); + File dir = new File(logDir); isReadOnly = dir.canRead() && !dir.canWrite(); } From 41986634c2a3068de4b552aab292f1c67456ef4c Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Fri, 19 Nov 2021 17:12:52 -0800 Subject: [PATCH 16/18] Add null check --- .../agent/bootstrap/diagnostics/status/StatusFile.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index ac68fda2c6d..853974c63c8 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -97,7 +97,7 @@ public class StatusFile { // guarded by lock private static BufferedSink buffer; - public static Logger startupLogger; + @Nullable public static Logger startupLogger; private static final ThreadPoolExecutor WRITER_THREAD = new ThreadPoolExecutor( @@ -155,7 +155,7 @@ private static boolean shouldWrite() { return true; } // read-only app services, want to log warning once in this case - if (!alreadyLogged.getAndSet(true)) { + if (startupLogger != null && !alreadyLogged.getAndSet(true)) { startupLogger.info( "Detected running on a read-only file system. Status json file won't be created. If this is unexpected, please check that process has write access to the directory: {}", directory); From a24ac250fdcbcfa8d777c39494b2592010ba14a8 Mon Sep 17 00:00:00 2001 From: Helen Yang Date: Mon, 29 Nov 2021 16:04:06 -0800 Subject: [PATCH 17/18] Use writable instead of isReadOnly and disable StatusFileTest --- .../agent/bootstrap/diagnostics/status/StatusFile.java | 6 +++--- .../agent/bootstrap/diagnostics/status/StatusFileTests.java | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index 853974c63c8..caa79f07adb 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -85,7 +85,7 @@ public class StatusFile { // visible for testing static String directory; - private static final boolean isReadOnly; + private static final boolean writable; private static final AtomicBoolean alreadyLogged = new AtomicBoolean(); @@ -117,7 +117,7 @@ public class StatusFile { logDir = initLogDir(); directory = logDir + STATUS_FILE_DIRECTORY; File dir = new File(logDir); - isReadOnly = dir.canRead() && !dir.canWrite(); + writable = dir.canWrite(); } private static Thread newThread(Runnable r) { @@ -151,7 +151,7 @@ private static boolean shouldWrite() { if (!DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { return false; } - if (!isReadOnly) { + if (!writable) { return true; } // read-only app services, want to log warning once in this case diff --git a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java index f4f45ef3ac3..815ee34fbc2 100644 --- a/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java +++ b/agent/agent-bootstrap/src/test/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFileTests.java @@ -38,6 +38,7 @@ import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.io.TempDir; @@ -45,6 +46,8 @@ import uk.org.webcompere.systemstubs.jupiter.SystemStub; import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension; +// FIXME (trask) failing in CI on deleting the the @TempDir +@Disabled @ExtendWith(SystemStubsExtension.class) class StatusFileTests { From 7bb5451c600bc3a310d57dc30deb89c35f609b0d Mon Sep 17 00:00:00 2001 From: Helen Y <56097766+heyams@users.noreply.github.com> Date: Tue, 30 Nov 2021 11:12:45 -0800 Subject: [PATCH 18/18] Fix negation Co-authored-by: Trask Stalnaker --- .../agent/bootstrap/diagnostics/status/StatusFile.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java index caa79f07adb..c0fbcf09db7 100644 --- a/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java +++ b/agent/agent-bootstrap/src/main/java/com/microsoft/applicationinsights/agent/bootstrap/diagnostics/status/StatusFile.java @@ -151,7 +151,7 @@ private static boolean shouldWrite() { if (!DiagnosticsHelper.useAppSvcRpIntegrationLogging()) { return false; } - if (!writable) { + if (writable) { return true; } // read-only app services, want to log warning once in this case