Address comments

src/core/server/logging/logging_config.ts

@@ -39,7 +39,7 @@ const ROOT_CONTEXT_NAME = 'root';
  */
 const DEFAULT_APPENDER_NAME = 'default';
 
-const createLevelSchema = schema.oneOf(
+const levelSchema = schema.oneOf(
   [
     schema.literal('all'),
     schema.literal('fatal'),
@@ -55,21 +55,21 @@ const createLevelSchema = schema.oneOf(
   }
 );
 
-const createLoggerSchema = schema.object({
+const loggerSchema = schema.object({
   appenders: schema.arrayOf(schema.string(), { defaultValue: [] }),
   context: schema.string(),
-  level: createLevelSchema,
+  level: levelSchema,
 });
 
 /** @public */
-export type LoggerConfigType = TypeOf<typeof createLoggerSchema>;
+export type LoggerConfigType = TypeOf<typeof loggerSchema>;
 export const config = {
   path: 'logging',
   schema: schema.object({
     appenders: schema.mapOf(schema.string(), Appenders.configSchema, {
       defaultValue: new Map<string, AppenderConfigType>(),
     }),
-    loggers: schema.arrayOf(createLoggerSchema, {
+    loggers: schema.arrayOf(loggerSchema, {
       defaultValue: [],
     }),
     root: schema.object(
@@ -78,7 +78,7 @@ export const config = {
           defaultValue: [DEFAULT_APPENDER_NAME],
           minSize: 1,
         }),
-        level: createLevelSchema,
+        level: levelSchema,
       },
       {
         validate(rawConfig) {
@@ -98,7 +98,7 @@ export const loggerContextConfigSchema = schema.object({
     defaultValue: new Map<string, AppenderConfigType>(),
   }),
 
-  loggers: schema.arrayOf(createLoggerSchema, { defaultValue: [] }),
+  loggers: schema.arrayOf(loggerSchema, { defaultValue: [] }),
 });
 
 /** @public */
@@ -178,11 +178,15 @@ export class LoggingConfig {
    * @param contextConfig
    */
   public extend(contextConfig: LoggerContextConfigType) {
+    // Use a Map to de-dupe any loggers for the same context. contextConfig overrides existing config.
+    const mergedLoggers = new Map<string, LoggerConfigType>([
+      ...this.configType.loggers.map((l) => [l.context, l] as [string, LoggerConfigType]),
+      ...contextConfig.loggers.map((l) => [l.context, l] as [string, LoggerConfigType]),
+    ]);
+
     const mergedConfig: LoggingConfigType = {
       appenders: new Map([...this.configType.appenders, ...contextConfig.appenders]),
-      // Could include duplicate contexts, however the logic in `fillLoggersConfig` should
-      // ensure the latest value wins (from the new config)
-      loggers: [...this.configType.loggers, ...contextConfig.loggers],
+      loggers: [...mergedLoggers.values()],
       root: this.configType.root,
     };
 

src/core/server/logging/logging_service.test.ts

@@ -48,8 +48,16 @@ describe('LoggingService', () => {
       };
 
       setup.configure(['test', 'context'], of(config1, config2));
-      expect(loggingSystem.setContextConfig).toHaveBeenCalledWith(['test', 'context'], config1);
-      expect(loggingSystem.setContextConfig).toHaveBeenCalledWith(['test', 'context'], config2);
+      expect(loggingSystem.setContextConfig).toHaveBeenNthCalledWith(
+        1,
+        ['test', 'context'],
+        config1
+      );
+      expect(loggingSystem.setContextConfig).toHaveBeenNthCalledWith(
+        2,
+        ['test', 'context'],
+        config2
+      );
     });
 
     it('stops forwarding first observable when called a second time', () => {
@@ -66,7 +74,11 @@ describe('LoggingService', () => {
       setup.configure(['test', 'context'], updates$);
       setup.configure(['test', 'context'], of(config1));
       updates$.next(config2);
-      expect(loggingSystem.setContextConfig).toHaveBeenCalledWith(['test', 'context'], config1);
+      expect(loggingSystem.setContextConfig).toHaveBeenNthCalledWith(
+        1,
+        ['test', 'context'],
+        config1
+      );
       expect(loggingSystem.setContextConfig).not.toHaveBeenCalledWith(['test', 'context'], config2);
     });
   });

src/core/server/logging/logging_service.ts

@@ -34,7 +34,7 @@ export interface LoggingServiceSetup {
    *
    * @remarks
    * Assumes that that the `context` property of the individual `logger` items emitted by `config$`
-   * are relative to the `baseContextParts`.
+   * are relative to the plugin's logging context (defaults to `plugins.<plugin_id>`).
    *
    * @example
    * Customize the configuration for the plugins.data.search context.

src/core/server/logging/logging_system.ts

@@ -68,7 +68,7 @@ export class LoggingSystem implements LoggerFactory {
    */
   public upgrade(rawConfig: LoggingConfigType) {
     const config = new LoggingConfig(rawConfig)!;
-    this.applyConfig(config);
+    this.applyBaseConfig(config);
   }
 
   /**
@@ -107,7 +107,7 @@ export class LoggingSystem implements LoggerFactory {
     // If we already have a base config, apply the config. If not, custom context configs
     // will be picked up on next call to `upgrade`.
     if (this.baseConfig) {
-      this.applyConfig(this.baseConfig);
+      this.applyBaseConfig(this.baseConfig);
     }
   }
 
@@ -117,9 +117,7 @@ export class LoggingSystem implements LoggerFactory {
    * @returns Promise that is resolved once all loggers are successfully disposed.
    */
   public async stop() {
-    for (const appender of this.appenders.values()) {
-      await appender.dispose();
-    }
+    await Promise.all([...this.appenders.values()].map((a) => a.dispose()));
 
     await this.bufferAppender.dispose();
 
@@ -153,12 +151,7 @@ export class LoggingSystem implements LoggerFactory {
     return this.getLoggerConfigByContext(config, LoggingConfig.getParentLoggerContext(context));
   }
 
-  private applyConfig(newBaseConfig: LoggingConfig) {
-    // Config update is asynchronous and may require some time to complete, so we should invalidate
-    // config so that new loggers will be using BufferAppender until newly configured appenders are ready.
-    this.baseConfig = undefined;
-    this.computedConfig = undefined;
-
+  private applyBaseConfig(newBaseConfig: LoggingConfig) {
     const computedConfig = [...this.contextConfigs.values()].reduce(
       (baseConfig, contextConfig) => baseConfig.extend(contextConfig),
       newBaseConfig