Changing the name and default status of the feature flag, only redirect if remote logging is enabled

Author: mbruin-NR

Agent/FeatureFlags/NRMAFlags.h

@@ -55,7 +55,7 @@
 
 + (BOOL) shouldEnableBackgroundReporting;
 
-+ (BOOL) shouldEnableRedirectStdOut;
++ (BOOL) shouldEnableAutoCollectLogs;
 
 + (NSArray<NSString*>*) namesForFlags:(NRMAFeatureFlags)flags;
 

Agent/FeatureFlags/NRMAFlags.m

@@ -47,7 +47,8 @@ + (NRMAFeatureFlags) featureFlags
                               NRFeatureFlag_NetworkRequestEvents | 
                               NRFeatureFlag_RequestErrorEvents |
                               NRFeatureFlag_DistributedTracing |
-                              NRFeatureFlag_AppStartMetrics;
+                              NRFeatureFlag_AppStartMetrics |
+                              NRFeatureFlag_AutoCollectLogs;
                   });
     return __flags;
 }
@@ -177,8 +178,8 @@ + (BOOL) shouldEnableBackgroundReporting {
     return ([NRMAFlags featureFlags] & NRFeatureFlag_BackgroundReporting) != 0;
 }
 
-+ (BOOL) shouldEnableRedirectStdOut {
-    return ([NRMAFlags featureFlags] & NRFeatureFlag_RedirectStdOutStdErr) != 0;
++ (BOOL) shouldEnableAutoCollectLogs {
+    return ([NRMAFlags featureFlags] & NRFeatureFlag_AutoCollectLogs) != 0;
 }
 
 + (NSArray<NSString*>*) namesForFlags:(NRMAFeatureFlags)flags {
@@ -243,8 +244,8 @@ + (BOOL) shouldEnableRedirectStdOut {
     if ((flags & NRFeatureFlag_BackgroundReporting) == NRFeatureFlag_BackgroundReporting) {
         [retArray addObject:@"BackgroundReporting"];
     }
-    if ((flags & NRFeatureFlag_RedirectStdOutStdErr) == NRFeatureFlag_RedirectStdOutStdErr) {
-        [retArray addObject:@"RedirectStdOut"];
+    if ((flags & NRFeatureFlag_AutoCollectLogs) == NRFeatureFlag_AutoCollectLogs) {
+        [retArray addObject:@"AutoCollectLogs"];
     }
 
     return retArray;

Agent/General/NewRelicAgentInternal.m

@@ -160,10 +160,6 @@ - (id) initWithCollectorAddress:(NSString*)collectorHost
 
     self = [super init];
     if (self) {
-        
-        if ([NRMAFlags shouldEnableRedirectStdOut]) {
-            [NRAutoLogCollector redirectStandardOutputAndError];
-        }
 
         // NOTE: BackgroundReporting is only enabled for iOS 13+.
 #if !TARGET_OS_WATCH

Agent/Harvester/NRMAHarvester.mm

@@ -22,6 +22,7 @@
 #import "NRMAFlags.h"
 #import "Constants.h"
 #import "NewRelicAgentInternal.h"
+#import "NRAutoLogCollector.h"
 
 #define kNRSupportabilityResponseCode kNRSupportabilityPrefix @"/Collector/ResponseStatusCodes"
 
@@ -772,9 +773,14 @@ - (void) handleLoggingConfigurationUpdate {
     // This if/else chain should only be entered if log_reporting was found in the config
     if (configuration.has_log_reporting_config) {
         if (configuration.log_reporting_enabled) {
-
-            // it is required to enable NRLogTargetFile when using LogReporting.
-            [NRLogger setLogTargets:NRLogTargetConsole | NRLogTargetFile];
+            if ([NRMAFlags shouldEnableAutoCollectLogs]) {
+                [NRAutoLogCollector redirectStandardOutputAndError];
+                // it is required to enable NRLogTargetFile when using LogReporting.
+                [NRLogger setLogTargets:NRLogTargetFile];
+            } else {
+                [NRLogger setLogTargets:NRLogTargetConsole | NRLogTargetFile];
+            }
+            
             // Parse NSString into NRLogLevel
             NRLogLevels level = [NRLogger stringToLevel: configuration.log_reporting_level];
             [NRLogger setRemoteLogLevel:level];
@@ -786,6 +792,9 @@ - (void) handleLoggingConfigurationUpdate {
         // OVERWRITE user selected value for LogReporting.
         else {
              NRLOG_AGENT_DEBUG(@"config: Has log reporting DISABLED");
+            if ([NRMAFlags shouldEnableAutoCollectLogs]) {
+                [NRAutoLogCollector restoreStandardOutputAndError];
+            }
             [NRLogger setLogTargets:NRLogTargetConsole];
 
             [NRMAFlags disableFeatures:NRFeatureFlag_LogReporting];

Agent/Public/NewRelicFeatureFlags.h

@@ -102,6 +102,6 @@ typedef NS_OPTIONS(unsigned long long, NRMAFeatureFlags){
     NRFeatureFlag_NewEventSystem                        = 1 << 20, // Disabled by default
     NRFeatureFlag_OfflineStorage                        = 1 << 21, // Disabled by default
     NRFeatureFlag_BackgroundReporting                   = 1 << 22, // Disabled by default
-    NRFeatureFlag_RedirectStdOutStdErr                  = 1 << 23, // Disabled by default
+    NRFeatureFlag_AutoCollectLogs                       = 1 << 23,
 
 };

Agent/Utilities/NRAutoLogCollector.m

@@ -13,6 +13,7 @@
 int saved_stderr;
 int stdoutPipe[2];
 int stderrPipe[2];
+static BOOL hasRedirectedStdOut = false;
 
 @interface NRAutoLogCollector()
 
@@ -21,6 +22,9 @@ @interface NRAutoLogCollector()
 @implementation NRAutoLogCollector
 
 + (BOOL) redirectStandardOutputAndError {
+    if (hasRedirectedStdOut){
+        return true;
+    }
     // Create pipes for stdout and stderr
     if (pipe(stdoutPipe) == -1 || pipe(stderrPipe) == -1) {
         return false;
@@ -63,6 +67,8 @@ + (BOOL) redirectStandardOutputAndError {
         [NRAutoLogCollector restoreStandardOutputAndError];
     });
 
+    hasRedirectedStdOut = true;
+    
     return true;
 }
 
@@ -85,10 +91,15 @@ + (void) readAndLog:(int) fd {
 }
 
 + (void) restoreStandardOutputAndError {
+    if (!hasRedirectedStdOut){
+        return;
+    }
     dup2(saved_stdout, fileno(stdout));
     dup2(saved_stderr, fileno(stderr));
     close(saved_stdout);
     close(saved_stderr);
+    
+    hasRedirectedStdOut = false;
 }
 
 + (BOOL) isValidTimestamp:(NSString *) timestampString {

Agent/Utilities/NRLogger.m

@@ -277,7 +277,7 @@ - (void)dealloc {
 - (void)addLogMessage:(NSDictionary *)message : (BOOL) agentLogsOn {
     // The static method checks the log level before we get here.
     dispatch_async(logQueue, ^{
-        if ((self->logTargets & NRLogTargetConsole) && (![NRMAFlags shouldEnableRedirectStdOut])) {
+        if ((self->logTargets & NRLogTargetConsole) && (![NRMAFlags shouldEnableAutoCollectLogs])) {
             NSLog(@"NewRelic(%@,%p):\t%@:%@\t%@\n\t%@",
                   [NewRelicInternalUtils agentVersion],
                   [NSThread currentThread],

Test Harness/NRTestApp/NRTestApp/AppDelegate.swift

@@ -17,6 +17,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
 #if DEBUG
         // The New Relic agent is set to log at NRLogLevelInfo by default, debug logging should only be used for debugging when all agent logs are desired.
         NRLogger.setLogLevels(NRLogLevelDebug.rawValue)
+        NewRelic.disableFeatures([NRMAFeatureFlags.NRFeatureFlag_AutoCollectLogs])
 #endif
 
         // To enable or disable feature flags in New Relic iOS Agent.
@@ -34,8 +35,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate {
         // Note: Disabled by default. Enable or disable (default) flag to enable background reporting.
         // NewRelic.enableFeatures([NRMAFeatureFlags.NRFeatureFlag_BackgroundReporting])
 
-        // Note: Disabled by default. Enable or disable (default) flag to enable auto collect of stdout.
-        //NewRelic.enableFeatures([NRMAFeatureFlags.NRFeatureFlag_RedirectStdOutStdErr])
         NewRelic.saltDeviceUUID(true)
 
        // NewRelic.replaceDeviceIdentifier("myDeviceId")

Tests/Unit-Tests/NewRelicAgentTests/Uncategorized/NRLoggerTests.m

@@ -255,7 +255,6 @@ - (void) testRemoteLogLevels {
     XCTAssertEqual(foundCount, 3, @"Three remote messages should be found.");
 }
 
-#if !TARGET_OS_WATCH
 - (void) testAutoCollectedLogs {
     [NRMAFlags enableFeatures: NRFeatureFlag_RedirectStdOutStdErr];
     // Set the remote log level to debug.
@@ -318,5 +317,4 @@ - (void) testAutoCollectedLogs {
     XCTAssertEqual(foundCount, 5, @"Five remote messages should be found.");
     [NRMAFlags disableFeatures: NRFeatureFlag_RedirectStdOutStdErr];
 }
-#endif
 @end