feat(container-loader): Allow setting the container's logger at load time

[tylerbutler]

Feature: set container logger at load time

Developers can now specify a logger that should be used by a container when it is loaded instead of always using the Loader's logger (Loader.services.subLogger).

Use cases and scenarios

Today a container's logger is set based on the logger used in the Loader. This is not ideal in nested container scenarios, where a container is loading a data store within another container, which is itself within another container. Apps/hosts often augment the logger from the loader when loading a container, and when loading a nested container, it makes sense to provide an augmented logger to the container directly rather than using the one from the loader.

Implementation details

This PR enables the scenario by adding a fluid-base-logger request header to the container load request. If set, then the container's subLogger will be set to the header value. If the override isn't set, then the container will use the loader's logger as it does today.

For completeness, we should add a header to the LoaderHeaders enum. Doing so is a breaking change, though, so that will be done in a future commit.

[leeviana]

Is it a breaking change to add a new enum value? I would expect all usages to be against specific enums but maybe there's something I'm not thinking of.

[tylerbutler]

Enums are a pain from a back-compat perspective. Adding a new value creates type incompatibility between the enum type. For example in our type testing code, this blows up if I add an enum value:

declare function get_current_EnumDeclaration_LoaderHeader():
    TypeOnly<current.LoaderHeader>;
declare function use_old_EnumDeclaration_LoaderHeader(
    use: TypeOnly<old.LoaderHeader>);
use_old_EnumDeclaration_LoaderHeader(
    get_current_EnumDeclaration_LoaderHeader());

Argument of type 'TypeOnly<import("/home/tylerbu/code/release-1/packages/common/container-definitions/src/loader").LoaderHeader>' is not assignable to parameter of type 'TypeOnly<import("/home/tylerbu/code/release-1/node_modules/@fluidframework/container-definitions-previous/dist/loader").LoaderHeader>'.
  Type 'LoaderHeader.cache' is not assignable to type 'TypeOnly<LoaderHeader>'.

[leeviana]

ahh interesting, right, because one definition does not fit into the other. I wonder if we should come up with a general coding recommendation to avoid these

[ssimic2]

Yes, enums are not mapping to native JS. I think we have explored two alternatives: string unions (mostly on string types) and objects pattern. ex: https://github.com/microsoft/FluidFramework/pull/10149/files. I don't think we have concluded on one pattern fits all.

[msfluid-bot]

⯅ @fluid-example/bundle-size-tests: +140 Bytes

Metric Name	Baseline Size	Compare Size	Size Diff
aqueduct.js	425.53 KB	425.53 KB	■ No change
connectionState.js	680 Bytes	680 Bytes	■ No change
containerRuntime.js	220.09 KB	220.09 KB	■ No change
loader.js	158.66 KB	158.8 KB	⯅ +140 Bytes
map.js	43.85 KB	43.85 KB	■ No change
matrix.js	136.17 KB	136.17 KB	■ No change
odspDriver.js	91.14 KB	91.14 KB	■ No change
odspPrefetchSnapshot.js	42.43 KB	42.43 KB	■ No change
sharedString.js	156.93 KB	156.93 KB	■ No change
Total Size	1.35 MB	1.35 MB	⯅ +140 Bytes

---

Baseline commit: 5d071c6

Generated by 🚫 dangerJS against a902e91

[vladsud]

Signing-off but starting a thread on future of Loader as concept.
Let's have a discussion first if we want to keep adding here, or start removing :)

[tylerbutler]

I'm going to merge this because that will "lock in" the change and make it easiest to port to the release branch (less churn to worry about). If it's needed in a patch porting it should be straightforward and quick. Otherwise, we have until next week to change our minds and back it out before our internal 3.1 release.