Fix status exception in a readonly file system (#1967)

* Check readonly for StatusFile

* Enable StatusFileTest

* Address a comment

* Check shouldWrite at callers

* Revert Feature.java

* Only log in app service env

* Fix lgtm

* Refactor

* Revert directory

* Update test

* Prevent too early to call DiagnosticsHelper.userAppSvcRpIntegrationLogging

* Remove a space in telemetry name

* Remove plural

* simplify a bit

* Check parent dir

* Add null check

* Use writable instead of isReadOnly and disable StatusFileTest

* Fix negation

Co-authored-by: Trask Stalnaker <[email protected]>

Co-authored-by: Trask Stalnaker <[email protected]>

Author: heyams

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;
@@ -84,6 +85,10 @@ public class StatusFile {
   // visible for testing
   static String directory;
 
+  private static final boolean writable;
+
+  private static final AtomicBoolean alreadyLogged = new AtomicBoolean();
+
   private static final Object lock = new Object();
 
   // guarded by lock
@@ -92,6 +97,8 @@ public class StatusFile {
   // guarded by lock
   private static BufferedSink buffer;
 
+  @Nullable public static Logger startupLogger;
+
   private static final ThreadPoolExecutor WRITER_THREAD =
       new ThreadPoolExecutor(
           1, 1, 750L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), StatusFile::newThread);
@@ -109,6 +116,8 @@ public class StatusFile {
 
     logDir = initLogDir();
     directory = logDir + STATUS_FILE_DIRECTORY;
+    File dir = new File(logDir);
+    writable = dir.canWrite();
   }
 
   private static Thread newThread(Runnable r) {
@@ -138,9 +147,20 @@ public static String getLogDir() {
 
   private StatusFile() {}
 
-  // visible for testing
-  static boolean shouldWrite() {
-    return DiagnosticsHelper.useAppSvcRpIntegrationLogging();
+  private static boolean shouldWrite() {
+    if (!DiagnosticsHelper.useAppSvcRpIntegrationLogging()) {
+      return false;
+    }
+    if (writable) {
+      return true;
+    }
+    // read-only app services, want to log warning once in this case
+    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);
+    }
+    return false;
   }
 
   public static <T> void putValueAndWrite(String key, T value) {

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

agent/agent-tooling/src/main/java/com/microsoft/applicationinsights/agent/internal/legacysdk/TelemetryClientClassFileTransformer.java

@@ -86,7 +86,9 @@ public class TelemetryClientClassFileTransformer implements ClassFileTransformer
       return null;
     }
 
+    // TODO (heya) track this via FeatureStatsbeat
     StatusFile.putValueAndWrite("SDKPresent", true);
+
     try {
       ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_MAXS);
       TelemetryClientClassVisitor cv = new TelemetryClientClassVisitor(cw);

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";