From 52e5f1f5743de4e00026c5714e29c53805c1999b Mon Sep 17 00:00:00 2001 From: Matt Rakow Date: Thu, 16 Feb 2023 11:39:08 -0800 Subject: [PATCH] Remove baseLogger and scopeOverride options from Loader/Container (#14216) This change removes the features added in #13779 and #14114, as we've determined they are not actually needed. --- api-report/container-loader.api.md | 4 - .../container-definitions/src/loader.ts | 6 -- .../loader/container-loader/src/container.ts | 90 +++++++------------ .../loader/container-loader/src/loader.ts | 10 +-- 4 files changed, 32 insertions(+), 78 deletions(-) diff --git a/api-report/container-loader.api.md b/api-report/container-loader.api.md index e3400e75f555..529ed7130ecd 100644 --- a/api-report/container-loader.api.md +++ b/api-report/container-loader.api.md @@ -127,25 +127,21 @@ export interface ICodeDetailsLoader extends Partial; private service: IDocumentService | undefined; @@ -615,7 +593,7 @@ export class Container } public readonly options: ILoaderOptions; private get scope() { - return this._scopeOverride ?? this.loader.services.scope; + return this.loader.services.scope; } private get codeLoader() { return this.loader.services.codeLoader; @@ -637,7 +615,6 @@ export class Container }); this.clientDetailsOverride = config.clientDetailsOverride; - this._scopeOverride = config.scopeOverride; this._resolvedUrl = config.resolvedUrl; if (config.canReconnect !== undefined) { this._canReconnect = config.canReconnect; @@ -651,41 +628,36 @@ export class Container }`; // Need to use the property getter for docId because for detached flow we don't have the docId initially. // We assign the id later so property getter is used. - this.subLogger = ChildLogger.create( - // If a baseLogger was provided, use it; otherwise use the loader's logger. - config.baseLogger ?? loader.services.subLogger, - undefined, - { - all: { - clientType, // Differentiating summarizer container from main container - containerId: uuid(), - docId: () => this._resolvedUrl?.id ?? undefined, - containerAttachState: () => this._attachState, - containerLifecycleState: () => this._lifecycleState, - containerConnectionState: () => ConnectionState[this.connectionState], - serializedContainer: config.serializedContainerState !== undefined, - }, - // we need to be judicious with our logging here to avoid generating too much data - // all data logged here should be broadly applicable, and not specific to a - // specific error or class of errors - error: { - // load information to associate errors with the specific load point - dmInitialSeqNumber: () => this._deltaManager?.initialSequenceNumber, - dmLastProcessedSeqNumber: () => this._deltaManager?.lastSequenceNumber, - dmLastKnownSeqNumber: () => this._deltaManager?.lastKnownSeqNumber, - containerLoadedFromVersionId: () => this.loadedFromVersion?.id, - containerLoadedFromVersionDate: () => this.loadedFromVersion?.date, - // message information to associate errors with the specific execution state - // dmLastMsqSeqNumber: if present, same as dmLastProcessedSeqNumber - dmLastMsqSeqNumber: () => this.deltaManager?.lastMessage?.sequenceNumber, - dmLastMsqSeqTimestamp: () => this.deltaManager?.lastMessage?.timestamp, - dmLastMsqSeqClientId: () => this.deltaManager?.lastMessage?.clientId, - dmLastMsgClientSeq: () => this.deltaManager?.lastMessage?.clientSequenceNumber, - connectionStateDuration: () => - performance.now() - this.connectionTransitionTimes[this.connectionState], - }, + this.subLogger = ChildLogger.create(loader.services.subLogger, undefined, { + all: { + clientType, // Differentiating summarizer container from main container + containerId: uuid(), + docId: () => this._resolvedUrl?.id ?? undefined, + containerAttachState: () => this._attachState, + containerLifecycleState: () => this._lifecycleState, + containerConnectionState: () => ConnectionState[this.connectionState], + serializedContainer: config.serializedContainerState !== undefined, }, - ); + // we need to be judicious with our logging here to avoid generating too much data + // all data logged here should be broadly applicable, and not specific to a + // specific error or class of errors + error: { + // load information to associate errors with the specific load point + dmInitialSeqNumber: () => this._deltaManager?.initialSequenceNumber, + dmLastProcessedSeqNumber: () => this._deltaManager?.lastSequenceNumber, + dmLastKnownSeqNumber: () => this._deltaManager?.lastKnownSeqNumber, + containerLoadedFromVersionId: () => this.loadedFromVersion?.id, + containerLoadedFromVersionDate: () => this.loadedFromVersion?.date, + // message information to associate errors with the specific execution state + // dmLastMsqSeqNumber: if present, same as dmLastProcessedSeqNumber + dmLastMsqSeqNumber: () => this.deltaManager?.lastMessage?.sequenceNumber, + dmLastMsqSeqTimestamp: () => this.deltaManager?.lastMessage?.timestamp, + dmLastMsqSeqClientId: () => this.deltaManager?.lastMessage?.clientId, + dmLastMsgClientSeq: () => this.deltaManager?.lastMessage?.clientSequenceNumber, + connectionStateDuration: () => + performance.now() - this.connectionTransitionTimes[this.connectionState], + }, + }); // Prefix all events in this file with container-loader this.mc = loggerToMonitoringContext(ChildLogger.create(this.subLogger, "Container")); diff --git a/packages/loader/container-loader/src/loader.ts b/packages/loader/container-loader/src/loader.ts index 6a37720df861..0fd8863ee22b 100644 --- a/packages/loader/container-loader/src/loader.ts +++ b/packages/loader/container-loader/src/loader.ts @@ -460,13 +460,7 @@ export class Loader implements IHostLoader { } private canCacheForRequest(headers: IRequestHeader): boolean { - return ( - this.cachingEnabled && - headers[LoaderHeader.cache] !== false && - // LoaderHeader.baseLogger and LoaderHeader.scopeOverride must create a new Container to apply the overrides. - headers["fluid-base-logger"] === undefined && - headers["fluid-scope-override"] === undefined - ); + return this.cachingEnabled && headers[LoaderHeader.cache] !== false; } private parseHeader(parsed: IParsedUrl, request: IRequest) { @@ -504,8 +498,6 @@ export class Loader implements IHostLoader { resolvedUrl: resolved, version: request.headers?.[LoaderHeader.version] ?? undefined, loadMode: request.headers?.[LoaderHeader.loadMode], - baseLogger: request.headers?.["fluid-base-logger"], - scopeOverride: request.headers?.["fluid-scope-override"], }, pendingLocalState, this.protocolHandlerBuilder,