diff --git a/.changeset/salty-colts-appear.md b/.changeset/salty-colts-appear.md new file mode 100644 index 000000000000..fdd4c28c524f --- /dev/null +++ b/.changeset/salty-colts-appear.md @@ -0,0 +1,19 @@ +--- +"@fluidframework/core-interfaces": minor +"__section": deprecation +--- + +Deprecate LogLevel.default and LogLevel.error + +`LogLevel.default` and `LogLevel.error` in `@fluidframework/core-interfaces` are deprecated in favor of the semantically clearer `LogLevel.info` and `LogLevel.essential`. + +#### Migration + +The recommended replacement for `LogLevel.default` depends on how the value is used: + +- For an **event's default `logLevel`** (e.g. the `logLevel` argument to `ITelemetryBaseLogger.send`), the recommendation is `LogLevel.essential`. +- For a logger's **default `minLogLevel`** (the threshold that filters events), `LogLevel.info` is the recommendation. + +The replacement for `LogLevel.error` should always be `LogLevel.essential`. + +See [issue #26969](https://github.com/microsoft/FluidFramework/issues/26969) for full guidance and removal tracking (planned for v3.0). diff --git a/packages/common/core-interfaces/api-report/core-interfaces.beta.api.md b/packages/common/core-interfaces/api-report/core-interfaces.beta.api.md index 39a9b3379798..0562ed5986c8 100644 --- a/packages/common/core-interfaces/api-report/core-interfaces.beta.api.md +++ b/packages/common/core-interfaces/api-report/core-interfaces.beta.api.md @@ -333,7 +333,9 @@ export type LogLevel = (typeof LogLevel)[keyof typeof LogLevel]; // @public export interface LogLevelConst { + // @deprecated readonly default: 20; + // @deprecated readonly error: 30; readonly essential: 30; readonly info: 20; diff --git a/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md b/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md index ef43fde81f7b..9378cbc5d36a 100644 --- a/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md +++ b/packages/common/core-interfaces/api-report/core-interfaces.legacy.alpha.api.md @@ -478,7 +478,9 @@ export type LogLevel = (typeof LogLevel)[keyof typeof LogLevel]; // @public export interface LogLevelConst { + // @deprecated readonly default: 20; + // @deprecated readonly error: 30; readonly essential: 30; readonly info: 20; diff --git a/packages/common/core-interfaces/api-report/core-interfaces.legacy.beta.api.md b/packages/common/core-interfaces/api-report/core-interfaces.legacy.beta.api.md index cae6efefbae2..3975eeb54bfd 100644 --- a/packages/common/core-interfaces/api-report/core-interfaces.legacy.beta.api.md +++ b/packages/common/core-interfaces/api-report/core-interfaces.legacy.beta.api.md @@ -465,7 +465,9 @@ export type LogLevel = (typeof LogLevel)[keyof typeof LogLevel]; // @public export interface LogLevelConst { + // @deprecated readonly default: 20; + // @deprecated readonly error: 30; readonly essential: 30; readonly info: 20; diff --git a/packages/common/core-interfaces/api-report/core-interfaces.legacy.public.api.md b/packages/common/core-interfaces/api-report/core-interfaces.legacy.public.api.md index 31e263d9e2c0..2f53560ad17c 100644 --- a/packages/common/core-interfaces/api-report/core-interfaces.legacy.public.api.md +++ b/packages/common/core-interfaces/api-report/core-interfaces.legacy.public.api.md @@ -333,7 +333,9 @@ export type LogLevel = (typeof LogLevel)[keyof typeof LogLevel]; // @public export interface LogLevelConst { + // @deprecated readonly default: 20; + // @deprecated readonly error: 30; readonly essential: 30; readonly info: 20; diff --git a/packages/common/core-interfaces/api-report/core-interfaces.public.api.md b/packages/common/core-interfaces/api-report/core-interfaces.public.api.md index 31e263d9e2c0..2f53560ad17c 100644 --- a/packages/common/core-interfaces/api-report/core-interfaces.public.api.md +++ b/packages/common/core-interfaces/api-report/core-interfaces.public.api.md @@ -333,7 +333,9 @@ export type LogLevel = (typeof LogLevel)[keyof typeof LogLevel]; // @public export interface LogLevelConst { + // @deprecated readonly default: 20; + // @deprecated readonly error: 30; readonly essential: 30; readonly info: 20; diff --git a/packages/common/core-interfaces/src/logger.ts b/packages/common/core-interfaces/src/logger.ts index 563d17182769..8c34de839a4a 100644 --- a/packages/common/core-interfaces/src/logger.ts +++ b/packages/common/core-interfaces/src/logger.ts @@ -81,13 +81,15 @@ export interface LogLevelConst { /** * Default LogLevel - * @remarks Prefer {@link LogLevelConst.info | LogLevel.info} when selecting a level explicitly since this will be deprecated and removed in a future release. + * @deprecated Prefer {@link LogLevelConst.info | LogLevel.info} when selecting a level explicitly to preserve prior treatment. Planned to be removed in 3.0.0. + * @see {@link https://github.com/microsoft/FluidFramework/issues/26969 | Issue #26969} for removal tracking. */ readonly default: 20; /** * To log errors. - * @remarks Prefer {@link LogLevelConst.essential | LogLevel.essential} when selecting a level since this will be deprecated and removed in a future release. + * @deprecated Prefer {@link LogLevelConst.essential | LogLevel.essential} when selecting a level. Planned to be removed in 3.0.0. + * @see {@link https://github.com/microsoft/FluidFramework/issues/26969 | Issue #26969} for removal tracking. */ readonly error: 30; } @@ -120,13 +122,13 @@ export interface ITelemetryBaseLogger { /** * Log a telemetry event, if it meets the appropriate log-level threshold (see {@link ITelemetryBaseLogger.minLogLevel}). * @param event - The event to log. - * @param logLevel - The log level of the event. Default: {@link LogLevelConst.default | LogLevel.default}. + * @param logLevel - The log level of the event. If undefined, the logLevel should be treated as {@link LogLevelConst.essential | LogLevel.essential}. */ send(event: ITelemetryBaseEvent, logLevel?: LogLevel): void; /** * Minimum log level to be logged. - * @defaultValue {@link LogLevelConst.default | LogLevel.default}. + * @defaultValue {@link LogLevelConst.info | LogLevel.info}. */ minLogLevel?: LogLevel; } diff --git a/packages/loader/container-loader/src/test/container.spec.ts b/packages/loader/container-loader/src/test/container.spec.ts index cdeed1b6c545..f3a154e05391 100644 --- a/packages/loader/container-loader/src/test/container.spec.ts +++ b/packages/loader/container-loader/src/test/container.spec.ts @@ -9,7 +9,8 @@ import { TypedEventEmitter, type IProvideLayerCompatDetails, } from "@fluid-internal/client-utils"; -import { AttachState, type IAudience } from "@fluidframework/container-definitions"; +import type { IAudience } from "@fluidframework/container-definitions"; +import { AttachState } from "@fluidframework/container-definitions"; import type { ICriticalContainerError, IContainer, diff --git a/packages/utils/telemetry-utils/api-report/telemetry-utils.legacy.beta.api.md b/packages/utils/telemetry-utils/api-report/telemetry-utils.legacy.beta.api.md index 0d9251f8cc7e..1af6c1b4480a 100644 --- a/packages/utils/telemetry-utils/api-report/telemetry-utils.legacy.beta.api.md +++ b/packages/utils/telemetry-utils/api-report/telemetry-utils.legacy.beta.api.md @@ -34,9 +34,9 @@ export interface ITelemetryLoggerExt extends ITelemetryBaseLogger { // @deprecated sendErrorEvent(event: ITelemetryErrorEventExt, error?: unknown): void; // @deprecated - sendPerformanceEvent(event: ITelemetryPerformanceEventExt, error?: unknown, logLevel?: typeof LogLevel.verbose | typeof LogLevel.default): void; + sendPerformanceEvent(event: ITelemetryPerformanceEventExt, error?: unknown, logLevel?: typeof LogLevel.verbose | typeof LogLevel.info): void; // @deprecated - sendTelemetryEvent(event: ITelemetryGenericEventExt, error?: unknown, logLevel?: typeof LogLevel.verbose | typeof LogLevel.default): void; + sendTelemetryEvent(event: ITelemetryGenericEventExt, error?: unknown, logLevel?: typeof LogLevel.verbose | typeof LogLevel.info): void; } // @beta @legacy (undocumented) diff --git a/packages/utils/telemetry-utils/src/logger.ts b/packages/utils/telemetry-utils/src/logger.ts index f8c2b99612f3..5abf73a0239f 100644 --- a/packages/utils/telemetry-utils/src/logger.ts +++ b/packages/utils/telemetry-utils/src/logger.ts @@ -224,18 +224,18 @@ export abstract class TelemetryLogger implements TelemetryLoggerExt { * * @param event - the event to send * @param error - optional error object to log - * @param logLevel - optional level of the log. It category of event is set as error, - * then the logLevel will be upgraded to be an error. + * @param logLevel - optional level of the log. If the event's category is `error`, + * the logLevel will be upgraded to {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential}. */ public sendTelemetryEvent( event: ITelemetryGenericEventExt, error?: unknown, - logLevel: typeof LogLevel.verbose | typeof LogLevel.default = LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): void { this.sendTelemetryEventCore( { ...event, category: event.category ?? "generic" }, error, - event.category === "error" ? LogLevel.error : logLevel, + event.category === "error" ? LogLevel.essential : (logLevel ?? LogLevel.essential), ); } @@ -244,12 +244,12 @@ export abstract class TelemetryLogger implements TelemetryLoggerExt { * * @param event - the event to send * @param error - optional error object to log - * @param logLevel - optional level of the log. + * @param logLevel - level of the log. */ - protected sendTelemetryEventCore( + private sendTelemetryEventCore( event: ITelemetryGenericEventExt & { category: TelemetryEventCategory }, - error?: unknown, - logLevel?: LogLevel, + error: unknown, + logLevel: LogLevel, ): void { const newEvent = convertToBaseEvent(event); if (error !== undefined) { @@ -280,7 +280,7 @@ export abstract class TelemetryLogger implements TelemetryLoggerExt { category: "error", }, error, - LogLevel.error, + LogLevel.essential, ); } @@ -289,13 +289,13 @@ export abstract class TelemetryLogger implements TelemetryLoggerExt { * * @param event - Event to send * @param error - optional error object to log - * @param logLevel - optional level of the log. It category of event is set as error, - * then the logLevel will be upgraded to be an error. + * @param logLevel - optional level of the log. If the event's category is `error`, + * the logLevel will be upgraded to {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential}. */ public sendPerformanceEvent( event: ITelemetryPerformanceEventExt, error?: unknown, - logLevel: typeof LogLevel.verbose | typeof LogLevel.default = LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): void { const perfEvent = { ...event, @@ -305,7 +305,7 @@ export abstract class TelemetryLogger implements TelemetryLoggerExt { this.sendTelemetryEventCore( perfEvent, error, - perfEvent.category === "error" ? LogLevel.error : logLevel, + perfEvent.category === "error" ? LogLevel.essential : (logLevel ?? LogLevel.essential), ); } @@ -518,8 +518,8 @@ export class ChildLogger extends TelemetryLogger { } private shouldFilterOutEvent(event: ITelemetryBaseEvent, logLevel?: LogLevel): boolean { - const eventLogLevel = logLevel ?? LogLevel.default; - const configLogLevel = this.baseLogger.minLogLevel ?? LogLevel.default; + const eventLogLevel = logLevel ?? LogLevel.essential; + const configLogLevel = this.baseLogger.minLogLevel ?? LogLevel.info; // Filter out in case event log level is below what is wanted in config. return eventLogLevel < configLogLevel; } @@ -533,7 +533,7 @@ export class ChildLogger extends TelemetryLogger { if (this.shouldFilterOutEvent(event, logLevel)) { return; } - this.baseLogger.send(this.prepareEvent(event), logLevel); + this.baseLogger.send(this.prepareEvent(event), logLevel ?? LogLevel.essential); } } @@ -618,7 +618,7 @@ export class MultiSinkLogger extends TelemetryLogger { super(namespace, realProperties); this.loggers = loggers; - this._minLogLevelOfAllLoggers = LogLevel.default; + this._minLogLevelOfAllLoggers = LogLevel.info; this.calculateMinLogLevel(); } @@ -630,7 +630,7 @@ export class MultiSinkLogger extends TelemetryLogger { if (this.loggers.length > 0) { const logLevels: LogLevel[] = []; for (const logger of this.loggers) { - logLevels.push(logger.minLogLevel ?? LogLevel.default); + logLevels.push(logger.minLogLevel ?? LogLevel.info); } this._minLogLevelOfAllLoggers = Math.min(...logLevels) as LogLevel; } @@ -653,10 +653,10 @@ export class MultiSinkLogger extends TelemetryLogger { * * @param event - the event to send to all the registered logger */ - public send(event: ITelemetryBaseEvent): void { + public send(event: ITelemetryBaseEvent, logLevel?: LogLevel): void { const newEvent = this.prepareEvent(event); for (const logger of this.loggers) { - logger.send(newEvent); + logger.send(newEvent, logLevel ?? LogLevel.essential); } } } @@ -693,6 +693,7 @@ export class PerformanceEvent { * @param emitLogs - should this instance emit logs. If set to false, logs will not be emitted to the logger, * but measurements will still be performed and any specified markers will be generated. * @param logLevel - optional {@link LogLevel} for events emitted by this performance event. + * If unspecified, {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential} will be used. * @returns An instance of {@link PerformanceEvent} */ public static start( @@ -700,7 +701,7 @@ export class PerformanceEvent { event: ITelemetryGenericEventExt, markers?: IPerformanceEventMarkers, emitLogs: boolean = true, - logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): PerformanceEvent { return new PerformanceEvent( extractTelemetryLoggerExt(logger), @@ -720,6 +721,7 @@ export class PerformanceEvent { * @param sampleThreshold - events with the same name and category will be sent to the logger * only when we hit this many executions of the task. If unspecified, all events will be sent. * @param logLevel - optional {@link LogLevel} for events emitted by this performance event. + * If unspecified, {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential} will be used. * @returns The results of the executed task * * @remarks Note that if the "same" event (category + eventName) would be emitted by different @@ -733,7 +735,7 @@ export class PerformanceEvent { callback: (event: PerformanceEvent) => T, markers?: IPerformanceEventMarkers, sampleThreshold: number = 1, - logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): T { const perfEvent = PerformanceEvent.start( logger, @@ -762,6 +764,7 @@ export class PerformanceEvent { * @param sampleThreshold - events with the same name and category will be sent to the logger * only when we hit this many executions of the task. If unspecified, all events will be sent. * @param logLevel - optional {@link LogLevel} for events emitted by this performance event. + * If unspecified, {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential} will be used. * @returns The results of the executed task * * @remarks Note that if the "same" event (category + eventName) would be emitted by different @@ -775,7 +778,7 @@ export class PerformanceEvent { callback: (event: PerformanceEvent) => Promise, markers?: IPerformanceEventMarkers, sampleThreshold: number = 1, - logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): Promise { const perfEvent = PerformanceEvent.start( logger, @@ -802,12 +805,12 @@ export class PerformanceEvent { private readonly startTime = performanceNow(); private startMark?: string; - protected constructor( + private constructor( private readonly logger: TelemetryLoggerExt, event: ITelemetryGenericEventExt, private readonly markers: IPerformanceEventMarkers = { end: true, cancel: "generic" }, private readonly emitLogs: boolean = true, - private readonly logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + private readonly logLevel: typeof LogLevel.verbose | typeof LogLevel.info | undefined, ) { this.event = { ...event }; if (this.markers.start) { diff --git a/packages/utils/telemetry-utils/src/mockLogger.ts b/packages/utils/telemetry-utils/src/mockLogger.ts index cf52590cbdeb..2df2dea3df27 100644 --- a/packages/utils/telemetry-utils/src/mockLogger.ts +++ b/packages/utils/telemetry-utils/src/mockLogger.ts @@ -41,7 +41,7 @@ export class MockLogger implements ITelemetryBaseLogger { public readonly minLogLevel: LogLevel; public constructor(minLogLevel?: LogLevel) { - this.minLogLevel = minLogLevel ?? LogLevel.default; + this.minLogLevel = minLogLevel ?? LogLevel.info; } /** @@ -59,7 +59,7 @@ export class MockLogger implements ITelemetryBaseLogger { * {@inheritDoc @fluidframework/core-interfaces#ITelemetryBaseLogger.send} */ public send(event: ITelemetryBaseEvent, logLevel?: LogLevel): void { - if ((logLevel ?? LogLevel.default) >= this.minLogLevel) { + if ((logLevel ?? LogLevel.essential) >= this.minLogLevel) { this._events.push(event); } } diff --git a/packages/utils/telemetry-utils/src/telemetryTypes.ts b/packages/utils/telemetry-utils/src/telemetryTypes.ts index 09270dfcf25f..b444a4a64e44 100644 --- a/packages/utils/telemetry-utils/src/telemetryTypes.ts +++ b/packages/utils/telemetry-utils/src/telemetryTypes.ts @@ -136,14 +136,15 @@ export interface ITelemetryLoggerExt extends ITelemetryBaseLogger { * Send an information telemetry event. * @param event - Event to send. * @param error - Optional error object to log. - * @param logLevel - Optional level of the log. Default: {@link @fluidframework/core-interfaces#LogLevel.essential}. + * @param logLevel - Optional level of the log. If undefined, the logLevel should be treated as {@link @fluidframework/core-interfaces#LogLevel.essential}. + * If the event's category is `error`, the logLevel will be upgraded to {@link @fluidframework/core-interfaces#LogLevel.essential}. * @deprecated This method is being removed without a replacement. * @see {@link https://github.com/microsoft/FluidFramework/issues/26910 | Issue #26910} for details. */ sendTelemetryEvent( event: ITelemetryGenericEventExt, error?: unknown, - logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): void; /** @@ -159,14 +160,15 @@ export interface ITelemetryLoggerExt extends ITelemetryBaseLogger { * Send a performance telemetry event. * @param event - Event to send * @param error - Optional error object to log. - * @param logLevel - Optional level of the log. Default: {@link @fluidframework/core-interfaces#LogLevel.essential}. + * @param logLevel - Optional level of the log. If undefined, the logLevel should be treated as {@link @fluidframework/core-interfaces#LogLevel.essential}. + * If the event's category is `error`, the logLevel will be upgraded to {@link @fluidframework/core-interfaces#LogLevel.essential}. * @deprecated This method is being removed without a replacement. * @see {@link https://github.com/microsoft/FluidFramework/issues/26910 | Issue #26910} for details. */ sendPerformanceEvent( event: ITelemetryPerformanceEventExt, error?: unknown, - logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): void; } @@ -183,12 +185,13 @@ export interface TelemetryLoggerExt extends ITelemetryBaseLogger { * Send an information telemetry event. * @param event - Event to send. * @param error - Optional error object to log. - * @param logLevel - Optional level of the log. Default: {@link @fluidframework/core-interfaces#LogLevel.default}. + * @param logLevel - Optional level of the log. If undefined, the logLevel will be treated as {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential}. + * If the event's category is `error`, the logLevel will be upgraded to {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential}. */ sendTelemetryEvent( event: ITelemetryGenericEventExt, error?: unknown, - logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): void; /** @@ -202,11 +205,12 @@ export interface TelemetryLoggerExt extends ITelemetryBaseLogger { * Send a performance telemetry event. * @param event - Event to send * @param error - Optional error object to log. - * @param logLevel - Optional level of the log. Default: {@link @fluidframework/core-interfaces#LogLevelConst.default | LogLevel.default}. + * @param logLevel - Optional level of the log. If undefined, the logLevel will be treated as {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential}. + * If the event's category is `error`, the logLevel will be upgraded to {@link @fluidframework/core-interfaces#LogLevelConst.essential | LogLevel.essential}. */ sendPerformanceEvent( event: ITelemetryPerformanceEventExt, error?: unknown, - logLevel?: typeof LogLevel.verbose | typeof LogLevel.default, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, ): void; } diff --git a/packages/utils/telemetry-utils/src/test/childLogger.spec.ts b/packages/utils/telemetry-utils/src/test/childLogger.spec.ts index be7e75d132b6..fda2a29d254b 100644 --- a/packages/utils/telemetry-utils/src/test/childLogger.spec.ts +++ b/packages/utils/telemetry-utils/src/test/childLogger.spec.ts @@ -194,16 +194,20 @@ describe("ChildLogger", () => { sent = true; }, - minLogLevel: LogLevel.error, + minLogLevel: LogLevel.essential, }; const childLogger1 = createChildLogger({ logger }); - childLogger1.send({ category: "error", eventName: "testEvent" }, LogLevel.error); - assert(sent, "event should be sent"); + childLogger1.send({ category: "error", eventName: "testEvent" }, LogLevel.essential); + assert(sent, "essential event should be sent"); sent = false; - childLogger1.send({ category: "generic", eventName: "testEvent" }, LogLevel.default); - assert(!sent, "event should not be sent"); + childLogger1.send({ category: "generic", eventName: "testEvent" }, LogLevel.info); + assert(!sent, "info event should not be sent"); + + sent = false; + childLogger1.send({ category: "generic", eventName: "testEvent" }); + assert(sent, "event with undefined logLevel should be sent"); }); it("should receive verbose events with min loglevel set as verbose", () => { @@ -221,11 +225,11 @@ describe("ChildLogger", () => { const childLogger1 = createChildLogger({ logger }); childLogger1.send({ category: "generic", eventName: "testEvent" }, LogLevel.verbose); - assert(sent, "event should be sent"); + assert(sent, "verbose event should be sent"); sent = false; childLogger1.send({ category: "error", eventName: "testEvent" }); - assert(sent, "default event should be sent"); + assert(sent, "event with undefined logLevel should be sent"); }); it("should not receive verbose events with no min loglevel", () => { @@ -241,7 +245,7 @@ describe("ChildLogger", () => { const childLogger1 = createChildLogger({ logger }); childLogger1.send({ category: "error", eventName: "testEvent" }); - assert(sent, "default event should be sent"); + assert(sent, "event with undefined logLevel should be sent"); sent = false; childLogger1.send({ category: "generic", eventName: "testEvent" }, LogLevel.verbose); @@ -257,10 +261,10 @@ describe("ChildLogger", () => { } sent = true; }, - minLogLevel: LogLevel.default, + minLogLevel: LogLevel.info, }; const multiSinkLogger = createMultiSinkLogger({ - loggers: [logger1, new MockLogger(LogLevel.error)], + loggers: [logger1, new MockLogger(LogLevel.essential)], }); const childLogger1 = createChildLogger({ logger: multiSinkLogger, @@ -269,7 +273,50 @@ describe("ChildLogger", () => { childLogger1.send({ category: "generic", eventName: "testEvent" }, LogLevel.verbose); assert(!sent, "verbose event should not be sent"); - childLogger1.send({ category: "generic", eventName: "testEvent" }, LogLevel.default); - assert(sent, "default event should be sent"); + childLogger1.send({ category: "generic", eventName: "testEvent" }, LogLevel.info); + assert(sent, "info event should be sent"); + }); +}); +describe("logLevel forwarding", () => { + function createRecordingSink(minLogLevel?: LogLevel): { + sink: ITelemetryBaseLogger; + recorded: { event: ITelemetryBaseEvent; logLevel: LogLevel | undefined }[]; + } { + const recorded: { event: ITelemetryBaseEvent; logLevel: LogLevel | undefined }[] = []; + const sink: ITelemetryBaseLogger = { + send: (event, logLevel): void => { + recorded.push({ event, logLevel }); + }, + minLogLevel, + }; + return { sink, recorded }; + } + + it("Forwards LogLevel.essential to the sink when `sendTelemetryEvent` omits logLevel", () => { + const { sink, recorded } = createRecordingSink(); + const child = createChildLogger({ logger: sink }); + + child.sendTelemetryEvent({ eventName: "chainDefault" }); + + assert.strictEqual(recorded[0]?.logLevel, LogLevel.essential); + }); + + it("Forwards explicit LogLevel.info to the sink via `sendTelemetryEvent`", () => { + const { sink, recorded } = createRecordingSink(); + const child = createChildLogger({ logger: sink }); + + child.sendTelemetryEvent({ eventName: "chainExplicit" }, undefined, LogLevel.info); + + assert.strictEqual(recorded[0]?.logLevel, LogLevel.info); + }); + + it("Forwards LogLevel.verbose to the sink via `sendTelemetryEvent`", () => { + // You have to add a minLogLevel of verbose to the sink to receive verbose events, otherwise they will be dropped. + const { sink, recorded } = createRecordingSink(LogLevel.verbose); + const child = createChildLogger({ logger: sink }); + + child.sendTelemetryEvent({ eventName: "chainDefault" }, undefined, LogLevel.verbose); + + assert.strictEqual(recorded[0]?.logLevel, LogLevel.verbose); }); }); diff --git a/packages/utils/telemetry-utils/src/test/multiSinkLogger.spec.ts b/packages/utils/telemetry-utils/src/test/multiSinkLogger.spec.ts index 74efa4ec3c14..12a951a33343 100644 --- a/packages/utils/telemetry-utils/src/test/multiSinkLogger.spec.ts +++ b/packages/utils/telemetry-utils/src/test/multiSinkLogger.spec.ts @@ -5,11 +5,34 @@ import { strict as assert } from "node:assert"; +import type { + ITelemetryBaseEvent, + ITelemetryBaseLogger, +} from "@fluidframework/core-interfaces"; import { LogLevel } from "@fluidframework/core-interfaces"; import { type MultiSinkLogger, createChildLogger, createMultiSinkLogger } from "../logger.js"; import { MockLogger } from "../mockLogger.js"; +interface RecordedEntry { + event: ITelemetryBaseEvent; + logLevel: LogLevel | undefined; +} + +function createRecordingSink(minLogLevel: LogLevel = LogLevel.verbose): { + sink: ITelemetryBaseLogger; + recorded: RecordedEntry[]; +} { + const recorded: RecordedEntry[] = []; + const sink: ITelemetryBaseLogger = { + send: (event, logLevel): void => { + recorded.push({ event, logLevel }); + }, + minLogLevel, + }; + return { sink, recorded }; +} + describe("MultiSinkLogger", () => { it("Pushes logs to all sinks", () => { const logger1 = new MockLogger(); @@ -66,14 +89,14 @@ describe("MultiSinkLogger", () => { }); it("MultiSink logger set the logLevel to min logLevel of all loggers", () => { - const logger1 = new MockLogger(LogLevel.error); - const logger2 = new MockLogger(LogLevel.default); + const logger1 = new MockLogger(LogLevel.essential); + const logger2 = new MockLogger(LogLevel.info); const multiSink = createMultiSinkLogger({ loggers: [createChildLogger({ logger: logger1 }), logger2], }); assert.strictEqual( multiSink.minLogLevel, - LogLevel.default, + LogLevel.info, "Min loglevel should be set correctly", ); @@ -94,7 +117,7 @@ describe("MultiSinkLogger", () => { }); assert.strictEqual( multiSink.minLogLevel, - LogLevel.default, + LogLevel.info, "Min loglevel should be set correctly to default", ); }); @@ -107,14 +130,14 @@ describe("MultiSinkLogger", () => { (multiSink as MultiSinkLogger).addLogger(new MockLogger()); assert.strictEqual( multiSink.minLogLevel, - LogLevel.default, + LogLevel.info, "Min loglevel should be set correctly to default", ); - (multiSink as MultiSinkLogger).addLogger(new MockLogger(LogLevel.default)); + (multiSink as MultiSinkLogger).addLogger(new MockLogger(LogLevel.info)); assert.strictEqual( multiSink.minLogLevel, - LogLevel.default, + LogLevel.info, "Min loglevel should be set correctly to default", ); @@ -125,4 +148,93 @@ describe("MultiSinkLogger", () => { "Min loglevel should be set correctly to verbose", ); }); + + describe("logLevel propagation", () => { + it("Forwards LogLevel.essential to every sink when `sendTelemetryEvent` omits logLevel", () => { + const { sink: sinkA, recorded: recordedA } = createRecordingSink(); + const { sink: sinkB, recorded: recordedB } = createRecordingSink(); + const multiSink = createMultiSinkLogger({ loggers: [sinkA, sinkB] }); + + multiSink.sendTelemetryEvent({ eventName: "noLogLevel" }); + + assert.strictEqual(recordedA[0]?.logLevel, LogLevel.essential); + assert.strictEqual(recordedB[0]?.logLevel, LogLevel.essential); + }); + + it("Forwards LogLevel.essential to every sink when `sendPerformanceEvent` omits logLevel", () => { + const { sink: sinkA, recorded: recordedA } = createRecordingSink(); + const { sink: sinkB, recorded: recordedB } = createRecordingSink(); + const multiSink = createMultiSinkLogger({ loggers: [sinkA, sinkB] }); + + multiSink.sendPerformanceEvent({ eventName: "perfNoLogLevel" }); + + assert.strictEqual(recordedA[0]?.logLevel, LogLevel.essential); + assert.strictEqual(recordedB[0]?.logLevel, LogLevel.essential); + }); + + it("Forwards LogLevel.essential to every sink for `sendErrorEvent`", () => { + const { sink: sinkA, recorded: recordedA } = createRecordingSink(); + const { sink: sinkB, recorded: recordedB } = createRecordingSink(); + const multiSink = createMultiSinkLogger({ loggers: [sinkA, sinkB] }); + + multiSink.sendErrorEvent({ eventName: "errorEvent" }); + + assert.strictEqual(recordedA[0]?.logLevel, LogLevel.essential); + assert.strictEqual(recordedB[0]?.logLevel, LogLevel.essential); + }); + + for (const explicitLevel of [LogLevel.verbose, LogLevel.info] as const) { + it(`Forwards explicit logLevel (${explicitLevel}) unchanged through MultiSink to every sink via \`sendTelemetryEvent\``, () => { + const { sink: sinkA, recorded: recordedA } = createRecordingSink(LogLevel.verbose); + const { sink: sinkB, recorded: recordedB } = createRecordingSink(LogLevel.verbose); + const multiSink = createMultiSinkLogger({ loggers: [sinkA, sinkB] }); + + multiSink.sendTelemetryEvent({ eventName: "explicit" }, undefined, explicitLevel); + + assert.strictEqual(recordedA[0]?.logLevel, explicitLevel); + assert.strictEqual(recordedB[0]?.logLevel, explicitLevel); + }); + + it(`Forwards explicit logLevel (${explicitLevel}) unchanged through MultiSink via \`sendPerformanceEvent\``, () => { + const { sink: sinkA, recorded: recordedA } = createRecordingSink(LogLevel.verbose); + const { sink: sinkB, recorded: recordedB } = createRecordingSink(LogLevel.verbose); + const multiSink = createMultiSinkLogger({ loggers: [sinkA, sinkB] }); + + multiSink.sendPerformanceEvent( + { eventName: "explicitPerf" }, + undefined, + explicitLevel, + ); + + assert.strictEqual(recordedA[0]?.logLevel, explicitLevel); + assert.strictEqual(recordedB[0]?.logLevel, explicitLevel); + }); + } + + it("Forwards explicit logLevel through MultiSink -> [Child(sinkA), sinkB]", () => { + const { sink: sinkA, recorded: recordedA } = createRecordingSink(LogLevel.verbose); + const { sink: sinkB, recorded: recordedB } = createRecordingSink(LogLevel.verbose); + const multiSink = createMultiSinkLogger({ + loggers: [createChildLogger({ logger: sinkA }), sinkB], + }); + + multiSink.sendTelemetryEvent({ eventName: "mixedChain" }, undefined, LogLevel.verbose); + + assert.strictEqual(recordedA[0]?.logLevel, LogLevel.verbose); + assert.strictEqual(recordedB[0]?.logLevel, LogLevel.verbose); + }); + + it("Regression: `MultiSinkLogger.send` forwards explicit logLevel to every sink (was previously dropped)", () => { + const { sink: sinkA, recorded: recordedA } = createRecordingSink(LogLevel.verbose); + const { sink: sinkB, recorded: recordedB } = createRecordingSink(LogLevel.verbose); + const multiSink = createMultiSinkLogger({ + loggers: [sinkA, sinkB], + }) as MultiSinkLogger; + + multiSink.send({ category: "generic", eventName: "directSend" }, LogLevel.verbose); + + assert.strictEqual(recordedA[0]?.logLevel, LogLevel.verbose); + assert.strictEqual(recordedB[0]?.logLevel, LogLevel.verbose); + }); + }); }); diff --git a/packages/utils/telemetry-utils/src/test/telemetryLogger.spec.ts b/packages/utils/telemetry-utils/src/test/telemetryLogger.spec.ts index 01041d727b26..584ff6a9c687 100644 --- a/packages/utils/telemetry-utils/src/test/telemetryLogger.spec.ts +++ b/packages/utils/telemetry-utils/src/test/telemetryLogger.spec.ts @@ -6,6 +6,7 @@ import { strict as assert } from "node:assert"; import type { ITelemetryBaseEvent, Tagged } from "@fluidframework/core-interfaces"; +import { LogLevel } from "@fluidframework/core-interfaces"; import { type ITelemetryLoggerPropertyBag, @@ -17,8 +18,10 @@ import type { TelemetryEventPropertyTypeExt } from "../telemetryTypes.js"; class TestTelemetryLogger extends TelemetryLogger { public events: ITelemetryBaseEvent[] = []; - public send(event: ITelemetryBaseEvent): void { + public logLevels: (LogLevel | undefined)[] = []; + public send(event: ITelemetryBaseEvent, logLevel?: LogLevel): void { this.events.push(this.prepareEvent(event)); + this.logLevels.push(logLevel); } } @@ -197,6 +200,58 @@ describe("TelemetryLogger", () => { } }); }); + + describe("logLevel forwarding", () => { + it("`sendTelemetryEvent` without logLevel forwards LogLevel.essential", () => { + const logger = new TestTelemetryLogger(); + logger.sendTelemetryEvent({ eventName: "noLevel" }); + assert.deepStrictEqual(logger.logLevels, [LogLevel.essential]); + }); + + it("`sendTelemetryEvent` with explicit LogLevel.info forwards LogLevel.info", () => { + const logger = new TestTelemetryLogger(); + logger.sendTelemetryEvent({ eventName: "infoLevel" }, undefined, LogLevel.info); + assert.deepStrictEqual(logger.logLevels, [LogLevel.info]); + }); + + it("`sendTelemetryEvent` with category 'error' forwards LogLevel.essential even when info is requested", () => { + const logger = new TestTelemetryLogger(); + logger.sendTelemetryEvent( + { eventName: "errorEvent", category: "error" }, + undefined, + LogLevel.info, + ); + assert.deepStrictEqual(logger.logLevels, [LogLevel.essential]); + }); + + it("`sendPerformanceEvent` without logLevel forwards LogLevel.essential", () => { + const logger = new TestTelemetryLogger(); + logger.sendPerformanceEvent({ eventName: "perfNoLevel" }); + assert.deepStrictEqual(logger.logLevels, [LogLevel.essential]); + }); + + it("`sendPerformanceEvent` with explicit LogLevel.info forwards LogLevel.info", () => { + const logger = new TestTelemetryLogger(); + logger.sendPerformanceEvent({ eventName: "perfInfo" }, undefined, LogLevel.info); + assert.deepStrictEqual(logger.logLevels, [LogLevel.info]); + }); + + it("`sendPerformanceEvent` with category 'error' forwards LogLevel.essential even when info is requested", () => { + const logger = new TestTelemetryLogger(); + logger.sendPerformanceEvent( + { eventName: "perfError", category: "error" }, + undefined, + LogLevel.info, + ); + assert.deepStrictEqual(logger.logLevels, [LogLevel.essential]); + }); + + it("`sendErrorEvent` forwards LogLevel.essential", () => { + const logger = new TestTelemetryLogger(); + logger.sendErrorEvent({ eventName: "errEvent" }); + assert.deepStrictEqual(logger.logLevels, [LogLevel.essential]); + }); + }); }); describe("convertToBasePropertyType", () => { diff --git a/packages/utils/telemetry-utils/src/test/utils.spec.ts b/packages/utils/telemetry-utils/src/test/utils.spec.ts index a6498e693174..af1d3e3f43f9 100644 --- a/packages/utils/telemetry-utils/src/test/utils.spec.ts +++ b/packages/utils/telemetry-utils/src/test/utils.spec.ts @@ -10,6 +10,7 @@ import type { IConfigProviderBase, ITelemetryBaseEvent, } from "@fluidframework/core-interfaces"; +import { LogLevel } from "@fluidframework/core-interfaces"; import type { InternalCoreInterfacesUtilityTypes } from "@fluidframework/core-interfaces/internal"; import type { ITelemetryLoggerExt as ITelemetryLoggerExtInternal } from "@fluidframework/telemetry-utils/internal"; @@ -360,6 +361,75 @@ describe("Sampling", () => { ); } }); + + describe("logLevel forwarding", () => { + function createCapturingLogger(): { + logger: TelemetryLoggerExt; + captures: { method: string; logLevel: LogLevel | undefined }[]; + } { + const captures: { method: string; logLevel: LogLevel | undefined }[] = []; + const logger: TelemetryLoggerExt = { + send: (_event, logLevel): void => { + captures.push({ method: "send", logLevel }); + }, + sendTelemetryEvent: (_event, _error, logLevel): void => { + captures.push({ method: "sendTelemetryEvent", logLevel }); + }, + sendErrorEvent: (_event, _error): void => { + captures.push({ method: "sendErrorEvent", logLevel: undefined }); + }, + sendPerformanceEvent: (_event, _error, logLevel): void => { + captures.push({ method: "sendPerformanceEvent", logLevel }); + }, + }; + const wrapped = mixinMonitoringContext(logger, { + getRawConfig: (): ConfigTypes => undefined, + }).logger; + return { logger: wrapped, captures }; + } + + it("Forwards explicit logLevel through `sendTelemetryEvent` to the wrapped logger", () => { + const { logger, captures } = createCapturingLogger(); + const sampled = createSampledLogger(logger); + + sampled.sendTelemetryEvent({ eventName: "x" }, undefined, LogLevel.verbose); + + assert.deepStrictEqual(captures, [ + { method: "sendTelemetryEvent", logLevel: LogLevel.verbose }, + ]); + }); + + it("Forwards explicit logLevel through `sendPerformanceEvent` to the wrapped logger", () => { + const { logger, captures } = createCapturingLogger(); + const sampled = createSampledLogger(logger); + + sampled.sendPerformanceEvent({ eventName: "x" }, undefined, LogLevel.info); + + assert.deepStrictEqual(captures, [ + { method: "sendPerformanceEvent", logLevel: LogLevel.info }, + ]); + }); + + it("Forwards explicit logLevel through `send` to the wrapped logger", () => { + const { logger, captures } = createCapturingLogger(); + const sampled = createSampledLogger(logger); + + sampled.send({ category: "generic", eventName: "x" }, LogLevel.verbose); + + assert.deepStrictEqual(captures, [{ method: "send", logLevel: LogLevel.verbose }]); + }); + + it("Forwards undefined logLevel when caller omits it", () => { + const { logger, captures } = createCapturingLogger(); + const sampled = createSampledLogger(logger); + + sampled.sendTelemetryEvent({ eventName: "x" }); + + assert.deepStrictEqual(captures, [ + { method: "sendTelemetryEvent", logLevel: undefined }, + ]); + }); + }); }); describe("tagCodeArtifacts", () => { diff --git a/packages/utils/telemetry-utils/src/utils.ts b/packages/utils/telemetry-utils/src/utils.ts index 7f9c8a188846..3dfb49540d04 100644 --- a/packages/utils/telemetry-utils/src/utils.ts +++ b/packages/utils/telemetry-utils/src/utils.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. */ -import type { ITelemetryBaseEvent } from "@fluidframework/core-interfaces"; +import type { ITelemetryBaseEvent, LogLevel } from "@fluidframework/core-interfaces"; import { loggerToMonitoringContext } from "./config.js"; import type { @@ -66,7 +66,7 @@ export function createSampledLogger( monitoringContext.config.getBoolean("Fluid.Telemetry.DisableSampling") ?? false; const sampledLogger = { - send: (event: ITelemetryBaseEvent): void => { + send: (event: ITelemetryBaseEvent, logLevel?: LogLevel): void => { // The sampler uses the following logic for sending events: // 1. If isSamplingDisabled is true, then this means events should be unsampled. Therefore we send the event without any checks. // 2. If isSamplingDisabled is false, then event should be sampled using the event sampler, if the sampler is not defined just send all events, other use the eventSampler.sample() method. @@ -75,31 +75,39 @@ export function createSampledLogger( if (isSamplingDisabled && (skipLoggingWhenSamplingIsDisabled ?? false)) { return; } - logger.send(event); + logger.send(event, logLevel); } }, - sendTelemetryEvent: (event: ITelemetryGenericEventExt): void => { + sendTelemetryEvent: ( + event: ITelemetryGenericEventExt, + error?: unknown, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, + ): void => { if (isSamplingDisabled || eventSampler === undefined || eventSampler.sample()) { if (isSamplingDisabled && (skipLoggingWhenSamplingIsDisabled ?? false)) { return; } - logger.sendTelemetryEvent(event); + logger.sendTelemetryEvent(event, error, logLevel); } }, - sendErrorEvent: (event: ITelemetryGenericEventExt): void => { + sendErrorEvent: (event: ITelemetryGenericEventExt, error?: unknown): void => { if (isSamplingDisabled || eventSampler === undefined || eventSampler.sample()) { if (isSamplingDisabled && (skipLoggingWhenSamplingIsDisabled ?? false)) { return; } - logger.sendErrorEvent(event); + logger.sendErrorEvent(event, error); } }, - sendPerformanceEvent: (event: ITelemetryGenericEventExt): void => { + sendPerformanceEvent: ( + event: ITelemetryGenericEventExt, + error?: unknown, + logLevel?: typeof LogLevel.verbose | typeof LogLevel.info, + ): void => { if (isSamplingDisabled || eventSampler === undefined || eventSampler.sample()) { if (isSamplingDisabled && (skipLoggingWhenSamplingIsDisabled ?? false)) { return; } - logger.sendPerformanceEvent(event); + logger.sendPerformanceEvent(event, error, logLevel); } }, isSamplingDisabled,