Adam
/
AyCode.Core
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity
AyCode.Core/AyCode.Core/docs/LOGGING/LOGGING_TODO.md

8.5 KiB
Raw Blame History

Logger — TODO

Priority legend

  • P0 blocker · P1 important · P2 nice-to-have · P3 idea

ACCORE-LOG-T-H6Y4: Fix the writer-ctor mismatch for DI-injected writers

Priority: P2 · Type: Bug fix · Related: LOGGING_ISSUES.md#accore-log-i-k7m2

Options:

  • A) Provide a fallback (AppType, LogLevel, string?) ctor on consumer writers that have DI-heavy primary ctors, with DI resolution via service-locator
  • B) Skip writers that don't match the Activator-ctor signature, with a single consolidated warning (not per-init spam)
  • C) Deprecate the config-reading writer-instantiation path entirely in favour of DI registration

Eliminate the per-startup Console.Error noise regardless.

ACCORE-LOG-T-N2D8: Expose AcEnv.AppConfiguration setter for consumer init

Priority: P2 · Type: Feature · Related: LOGGING_ISSUES.md#accore-log-i-r9p3

Allow AcEnv.SetConfiguration(IConfiguration) so consumer Program.cs can do AcEnv.SetConfiguration(builder.Configuration) at startup. Enables config-reading pattern on MAUI/WASM without filesystem assumptions. Backward-compat: fall back to filesystem if no explicit set.

ACCORE-LOG-T-R7L3: Unify or clearly separate config-reading and DI-based patterns

Priority: P2 · Type: Docs / Refactor · Related: LOGGING_ISSUES.md#accore-log-i-l4n8

Decide the canonical direction:

  • (a) Deprecate config-reading pattern → all consumers migrate to DI factory
  • (b) Keep both, with compile-time guidance (analyzer / XML doc [Obsolete] hints / decision tree in LOGGING.md)
  • (c) Merge: DI-factory internally falls back to config-reading when TLogger doesn't match the Activator ctor shape

ACCORE-LOG-T-F4S6: Per-writer LogLevel via appsettings

Priority: P2 · Type: Feature

Extend AcLoggerOptions with per-writer LogLevel overrides. Example shape:

"AyCode": {
    "Logger": {
        "LogLevel": "Info",
        "WriterLevels": { "BrowserConsoleLogWriter": "Debug", "SignaRClientLogItemWriter": "Warning" }
    }
}

Factory applies overrides when constructing writers. Currently writer-LogLevel is hardcoded in writer ctors.

ACCORE-LOG-T-J9G2: Unify default LogLevel across setup paths

Priority: P1 · Type: Behaviour decision + cleanup · Related: LOGGING_ISSUES.md#accore-log-i-b2h5

Decide which default wins (AcLoggerBase.cs:18 = Error vs AcLoggerOptions.cs:27 = Info) and make both paths consistent. Recommendation: Info — matches the DI/Options path already in use for modern consumers (MAUI, WASM, ASP.NET Core) and is the common developer expectation.

Change: drop the field initializer in AcLoggerBase (or set it to Info). Update LOGGING.md with a single "Default LogLevel = Info" line so this can't drift again.

ACCORE-LOG-T-B8K5: Fix AcConsoleLogWriter.Initialize() double-run

Priority: P3 · Type: Bug fix · Related: LOGGING_ISSUES.md#accore-log-i-x7q1

Remove the redundant Initialize() call from the parameterless ctor body — the : this(null) chain already invokes it. No behaviour change; removes wasted Console.ForegroundColor assignment.

ACCORE-LOG-T-X1V4: Fix ILogger.IsEnabled(MsLogLevel.None) wrongly reporting enabled

Priority: P2 · Type: Bug fix · Related: LOGGING_ISSUES.md#accore-log-i-v3j6

Add if (logLevel == MsLogLevel.None) return false; at the top of AcLoggerBase.IsEnabled. One-line fix. Optionally add a unit test covering all six MS LogLevel values → expected bool.

ACCORE-LOG-T-M7P2: Cleanup batch (low-risk micro-refactor, one commit)

Priority: P3 · Type: Cleanup · Related: LOGGING_ISSUES.md#accore-log-i-t8f2

Single commit covering:

  • Remove unused usings in AcLoggerBase.cs (System.Security.AccessControl, System.Net.Mime.MediaTypeNames)
  • Extract the default-category magic string "..." (at least 3 occurrences — AcLoggerBase.cs:36,76, writer side) into a public const AcLoggerBase.DefaultCategory = "..." with XML doc explaining it's the "unknown category" sentinel, NOT a magic string. Update all call-sites.
  • Fix misleading comment at AcLoggerBase.cs:210 — current comment says "fallback null" but code assigns "Log". Make comment match code.
  • Decide on the commented-out batch block in AcLogItemWriterBase.cs:90-119: either delete (git history preserves) or convert to a single // TODO: see LOGGING_TODO.md#todo-XX marker with a new TODO entry for the batch work.

ACCORE-LOG-T-L3T8: Fail-fast ctor validation in AddAcLoggerFactory<TLogger>

Priority: P2 · Type: Feature

AcLoggerServiceExtensions.AddAcLoggerFactory<TLogger> uses Activator.CreateInstance(typeof(TLogger), AppType, LogLevel, categoryName, writers) only on first logger resolution. Ctor-mismatch therefore surfaces only at first log call, not at services.BuildServiceProvider() time.

Add a one-time reflection check at registration: typeof(TLogger).GetConstructor([typeof(AppType), typeof(LogLevel), typeof(string), typeof(IAcLogWriterBase[])]) — if null, throw InvalidOperationException with the expected signature in the message. Cost: one reflection call per registration. Benefit: app fails to start (loud) instead of logging being silently broken (quiet).

ACCORE-LOG-T-Q6Z1: Writer exception isolation

Priority: P2 · Type: Resilience

Currently AcLoggerBase.Info(...) and friends do LogWriters.ForEach(x => x.Info(...)). A throwing writer (e.g. SignaRClientLogItemWriter during network blip, AcDbLogItemWriter during a DB outage) takes down the fan-out → subsequent writers in the list never see the message.

Wrap each writer invocation in try { ... } catch (Exception ex) { Console.Error.WriteLine($"Writer {x.GetType().Name} failed: {ex.Message}"); }. Optional: escalate to a circuit-breaker pattern (N failures → disable that writer for M seconds) to avoid Console.Error flood on persistent outages.

Must NOT log to AcLoggerBase itself inside the catch (reentrancy / infinite loop risk).

ACCORE-LOG-T-W4H9: Migrate server-side NopCommerce plugin logger setup to DI-factory

Priority: P2 · Type: Consumer refactor · Related: LOGGING_ISSUES.md#accore-log-i-m4c9, ../../../AyCode.Services/docs/SIGNALR/SIGNALR_TODO.md#accore-sig-t-m5l6

Align the server-side plugin (Nop.Plugin.Misc.AIPlugin/Infrastructure/PluginNopStartup.cs) with the client-side setup pattern already used by MAUI / Web / Web.Client consumers.

Target diff

// In PluginNopStartup.ConfigureServices:

// 1. Bind Logger options from appsettings.json (replaces AcEnv.AppConfiguration path)
services.Configure<AcLoggerOptions>(configuration.GetSection("AyCode:Logger"));

// 2. Register writers (already present)
services.AddScoped<IAcLogWriterBase, ConsoleLogWriter>();
services.AddScoped<IAcLogWriterBase, NopLogWriter>();

// 3. Register the logger factory — replaces direct `new Logger(...)` calls
services.AddAcLoggerFactory<Logger>();

// 4. In the SignalR registration — drop the manual Logger construction
.AddAcBinaryProtocol(opts =>
{
    opts.ProtocolMode = BinaryProtocolMode.AsyncSegment;
    // Remove:  opts.Logger = new Logger(nameof(AyCodeBinaryHubProtocol));
    //         — AcSignalRProtocolExtensions.BuildProtocol auto-resolves ILogger<AcBinaryHubProtocol> from DI
});

Consequences / checklist

  • Consumers of new Logger(...) elsewhere in the plugin code must switch to injecting Func<string, Logger> (DI factory).
  • ACCORE-LOG-I-K7M2 Console.Error noise disappears once the config-reading path is no longer invoked.
  • Legacy AyCode:Logger:LogWriters[] array in appsettings.json becomes unused — decide: (a) remove (breaking if any other consumer still reads it), (b) keep for back-compat with a comment "superseded by DI, see LOGGING.md".
  • Update Nop.Plugin.Misc.AIPlugin/docs/SIGNALR/README.md:22 — current text still describes services.AddSingleton<IHubProtocol>(new AcBinaryHubProtocol()) which has NOT been the actual registration for months.
  • Verify server startup log — AcLoggerOptions must bind cleanly (no "missing section" warnings from IOptionsMonitor).

Why this is a separate TODO from ACCORE-LOG-T-R7L3

ACCORE-LOG-T-R7L3 is about the framework-level decision (keep both patterns vs deprecate legacy). ACCORE-LOG-T-W4H9 is the consumer-level execution of that decision for the server plugin — independent of whether ACCORE-LOG-T-R7L3 ultimately deprecates the legacy path or keeps it alive. Migrating the server plugin removes ACCORE-LOG-I-K7M2 noise regardless of ACCORE-LOG-T-R7L3's outcome.