enh(internal): debug log api

[buenaflor]

📜 Description

Improves the debug logging dx

💡 Motivation and Context

Currently the debug logging api is tied to the options which has few caveats

• by design not usable in isolates
• cumbersome to use, need to inject the logger or the options instance everywhere
• no clear logger name separation per package since there is only one instance

New design:

• no need to inject
• one top-level instance per package with the proper name:
  • e.g final debugLogger = SentryDebugLogger('sentry.dio') that is then used within the dio package
• isolate-compatible, you can continue using the debugLogger instance, you only need to run SentryDebugLogger.configure in the new isolate

💚 How did you test it?

Manual and unit test

🔮 Next steps

Step-by-step replace existing API

#skip-changelog

[github-actions]

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

• packages/flutter/lib/src/native/java/android_replay_recorder.dart

[github-actions]

Android Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	432.62 ms	434.61 ms	1.99 ms
Size	13.93 MiB	15.18 MiB	1.25 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
75284dc	512.39 ms	530.87 ms	18.48 ms
0fb3800	465.64 ms	536.77 ms	71.13 ms
b6c8720	457.41 ms	519.04 ms	61.63 ms
e04b24b	504.72 ms	516.43 ms	11.71 ms
6e7d494	397.35 ms	378.91 ms	-18.43 ms
6ad8fc4	489.92 ms	484.96 ms	-4.96 ms
575ebaa	478.00 ms	585.76 ms	107.76 ms
cba2765	405.98 ms	422.02 ms	16.04 ms
5a95d04	378.92 ms	364.33 ms	-14.59 ms
793f4dc	462.68 ms	544.21 ms	81.53 ms

App size

Revision	Plain	With Sentry	Diff
75284dc	13.93 MiB	14.93 MiB	1.00 MiB
0fb3800	6.54 MiB	7.69 MiB	1.15 MiB
b6c8720	6.54 MiB	7.69 MiB	1.15 MiB
e04b24b	13.93 MiB	15.00 MiB	1.06 MiB
6e7d494	13.93 MiB	15.06 MiB	1.13 MiB
6ad8fc4	13.93 MiB	15.06 MiB	1.13 MiB
575ebaa	6.54 MiB	7.69 MiB	1.15 MiB
cba2765	13.93 MiB	15.18 MiB	1.25 MiB
5a95d04	13.93 MiB	15.06 MiB	1.13 MiB
793f4dc	6.54 MiB	7.69 MiB	1.15 MiB

Previous results on branch: enh/debug-logging

Startup times

Revision	Plain	With Sentry	Diff
3b8e07f	376.26 ms	372.73 ms	-3.52 ms
cd107e8	434.74 ms	478.64 ms	43.90 ms

App size

Revision	Plain	With Sentry	Diff
3b8e07f	13.93 MiB	15.18 MiB	1.25 MiB
cd107e8	13.93 MiB	15.18 MiB	1.25 MiB

[github-actions]

iOS Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1253.04 ms	1257.33 ms	4.29 ms
Size	5.53 MiB	5.96 MiB	443.41 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
4298701	1243.56 ms	1262.29 ms	18.72 ms
bbdbcb9	1262.70 ms	1261.86 ms	-0.84 ms
827bf09	1261.86 ms	1276.41 ms	14.55 ms
426fbfd	1257.49 ms	1257.06 ms	-0.43 ms
6ad8fc4	1263.70 ms	1266.06 ms	2.36 ms
8775665	1234.17 ms	1230.04 ms	-4.13 ms
a69a51f	1231.73 ms	1233.15 ms	1.42 ms
6f47800	1247.52 ms	1259.37 ms	11.85 ms
c8596a6	1234.11 ms	1241.19 ms	7.08 ms
73a3c38	1263.37 ms	1277.90 ms	14.53 ms

App size

Revision	Plain	With Sentry	Diff
4298701	20.70 MiB	22.46 MiB	1.76 MiB
bbdbcb9	5.53 MiB	6.02 MiB	501.33 KiB
827bf09	7.86 MiB	9.44 MiB	1.58 MiB
426fbfd	5.53 MiB	6.01 MiB	488.17 KiB
6ad8fc4	5.53 MiB	6.01 MiB	487.65 KiB
8775665	5.53 MiB	6.02 MiB	502.13 KiB
a69a51f	5.53 MiB	6.01 MiB	487.38 KiB
6f47800	7.86 MiB	9.44 MiB	1.58 MiB
c8596a6	7.86 MiB	9.44 MiB	1.58 MiB
73a3c38	7.86 MiB	9.44 MiB	1.58 MiB

Previous results on branch: enh/debug-logging

Startup times

Revision	Plain	With Sentry	Diff
3b8e07f	1260.80 ms	1261.63 ms	0.83 ms
cd107e8	1243.78 ms	1255.02 ms	11.25 ms

App size

Revision	Plain	With Sentry	Diff
3b8e07f	5.53 MiB	5.96 MiB	444.18 KiB
cd107e8	5.53 MiB	5.96 MiB	443.44 KiB

[denrase]

I think SentryInternalLogger would be a more appropriate name.

[denrase]

Is this ever changing during runtime? I don't understand why we now have both an instance and static vars. If we go the instance, can't we just call `Logger(name, minLevel)'? If it needs to be enabled later, we can just set it on the instance.

So either static only, or instance only, no need to mix both.

[buenaflor]

I don't understand why we now have both an instance and static vars

the reason why it's an instance is so we have the benefit of creating multiple logger instance with the same configuration within the same isolate

the configuration is static per isolate, I don't think we will ever have a use case where we want multiple loggers with different minLevels

Example: Flutter

const logger = SentryDebugLogger(sentry.flutter)

// SentryDebugLogger.configure(...) called in SentryOptions
logger.info('info in main isolate)' // by default uses the main isolate static config

// spawn a new isolate
// within that isolate
// SentryDebugLogger.configure(...) called directly after isolate has spawned
logger.info('info in other isolate') // you can still use that same logger instance

---

• if we went the route with static only we would lose the ability to directly associate a package to a specific logger

  • SentryDebugLogger.info(name: 'sentry.flutter', message: 'xyz')
  • and repeating name: 'sentry.flutter', hundreds of time is error prone

• if we went the route with instance only we would need to create a new logger instance in each new isolate which can get messy quickly because then you would have to differentiate between the main isolate logger and the current isolate logger and the code interchanges heavily where it's easy to use the wrong one (take a look at android replay recorder and envelope sender)

Logger(name, minLevel)

this doesn't work because you can only configure the logger through the static function

[buenaflor]

what we could do is to separate the config and have only the config be static so we don't mix it into the log class

/// Per-isolate logging configuration.
@internal
class SentryLogConfig {
  static bool isEnabled = false;
  static SentryLevel minLevel = SentryLevel.warning;

  /// Configure logging for the current isolate.
  static void configure({
    required bool isEnabled,
    SentryLevel minLevel = SentryLevel.warning,
  }) {
    SentryLogConfig.isEnabled = isEnabled;
    SentryLogConfig.minLevel = minLevel;
  }
}


// within the logger:
@pragma('vm:prefer-inline')
void _log(SentryLevel level, String message, {Object? error, StackTrace? stackTrace}) {
  if (!SentryLogConfig.isEnabled) return;
  if (level.ordinal < SentryLogConfig.minLevel.ordinal) return;
  
  dev.log(
    '[${level.name}] $message',
    name: _name,
    level: level.toDartLogLevel(),
    error: error,
    stackTrace: stackTrace,
    time: DateTime.now(),
  );
}

alternative 1 - update the config on the instance:

const logger = SentryDebugLogger(sentry.flutter);

// later at some point
logger.updateConfig(isEnabled: xyz, name: 'sentry.flutter');

caveat: we would need to update the config for every new logger

alternative 2 - update the config using a callback on a global config instance:

const loggerConfig = LoggerConfig() // updated later at some point when Sentry initializes
final logger = SentryDebugLogger(sentry.flutter, () => loggerConfig);

[denrase]

Is there a case where we would log an error + stacktrace as debug? Same for info and probably also warning.

[buenaflor]

hm not sure, just added it for consistency