Avoid mutating options object in mixinAttributor

[Abe27342]

Description

This reduces potential for future bugs--no reason that the context options need to be mutated here.

[ChumpChief]

This is dodging the readonly qualifier on IContainerContext.options. We should either protect that readonly promise, or else modify that interface to allow what we think should be permissible.

This decision probably depends on the overall future of ILoaderOptions - if we really think about it as "options on the loader", then individual containers obviously shouldn't be modifying it because it doesn't belong to them. But really, it seems we have a collection of "options for a specific container specified by the host" that are sneaking onto that interface for convenience but should really be passed through some other channel. LoaderHeader is the best option for that purpose currently available (though it's not good) - we could probably come up with better options too.

I'm not super familiar with the usage of this mixin, but the usage of ILoaderOptions also implies that it's the host (not the container author) that will be controlling if/when to use this - is that desirable? I would have expected it to be fully in the hands of the container author on first instinct.

[Abe27342]

yeah, I don't think it should ultimately live here either. This PR was more of a band-aid to make things slightly less worse in the interim--I agree that ideally we want the container author to specify policy around attribution.

The DDS-specific attribution configuration are probably best put on options in those DDS's factories (search for createInsertOnlyAttributionPolicy for example). I was thinking this object might be better put on an extension of the container runtime options. This would accomplish the goal of putting things in the container author's hands, but it hits a snag because previous logic here made use of the fact that ILoaderOptions gets plumbed through to dataStoreRuntime's options, whereas analogous container-level options don't. It seems odd to me that the loader options make it that far in the first place.

Taking a step back, there's really 2 concerns:

1. Any part of the code which wants to use framework attribution needs to depend on some enablement strategy for compatibility--in the current world, this is accomplished with the boolean being modified in this PR. There needs to be a bit of state accessible to the container runtime and DDSes within that container which informs them whether to attribute ops that happen, and that bit of state needs to be derived from the container author setting as well as whether the document is new or existing (the container author setting should only apply to new containers)
2. We'd only like to bundle some attribution-specific parts of the code for people that care (i.e. ever use attribution).

(2) generally seems nicely solvable by above mentioned strategies. (1)

[anthony-murphy]

i think this function could directly take all the necessary components to enable attribution, as this is called by the runtime factory/container runtime author. that would allow complete decoupling from loader options.

[Abe27342]

that works for making this class aware of whether attribution is enabled, but doesn't give a clear path for DDSes to know

[anthony-murphy]

the mixin should override the options property on the containerRuntime. it shouldn't modify anything on the context

[Abe27342]

FYI--I think the right path forward here is something like #15397, but there is some work that needs to be settled before that's fully achievable. I put a topic on the next office hours to talk about some of that. This moves things off the loader options / container runtime options entirely, so sidesteps the issue.

I'm going to close this PR as it seems like the bandaid fix isn't very high-value and I'd need to do some refactoring to get tests passing (they fail because, as Tony suggested, mucking with the context doesn't work, probably due to it not being plain old data)