ILogManager Creation needs to be reevaluated

[mkoscumb]

Problem As we add more configurability and modularity to the SDK, this inevitably leads to more arguments being passed to the methods responsible for constructing an ILogManager. This has already grown hard to sustain and maintain, and will only get worse until it is wrangled.

Proposed Solution Methods responsible for constructing an ILogManager should take a LogManagerConfiguration structure, as of right now this is defined as:

struct LogManagerConfiguration
{
    ILogConfiguration LogConfiguration{};
    IHttpClient* HttpClient{};
    // Others currently in code review.
};

This would be passed to LogManagerProvider's methods (as well as the associated factory functions that generate an ILogManager object) to initialize an ILogManager. This then means that the set of overloads for Initialize() is kept slim and maintainable.

[maxgolov]

I would rather suggest to add the getters and setters on ILogConfiguration, similar to how it's done in lib/config/RuntimeConfig_Default.hpp . Primary Tenant Token already exists on ILogConfiguration, and there are several places in code that fetch the token straight from ILogConfiguration, so you would definitely not be needing an extra parameter for the TenantToken. You do need one for:

• IHttpClient
• IOfflineStorage
• Thread Pool
• possibly for the Data Viewer impl

[mkoscumb]

I wouldn't be opposed to providing getters and setters, should that be the existing style. That said, if and unless there are validation operations performed on those methods, I'm hesitant to see what value could be added.

To the other values. The list was intentionally brief as it is actively growing, based on what was checked in as of now.

[maxgolov]

Let's discuss this on a call today. One other option:

• keep ILogConfiguration as is
• provide a sub-class that implements an additional set of methods, in something like AdvancedConfiguration - with that one having getters / setters. Then instead of using RTTI, we can have a flag on ILogConfiguration config tree, something like config["extras"] or config["advanced"] set to true, and do the cast from ILogConfiguration to AdvancedConfiguration
• implement a set of "SDK Developer" headers that are accessible to devs who want to customize the SDK, override certain behaviors, etc.. basically an extension point and an optional extension that allows to implement that functionality.

Up until this point the users of old "Aria SDK" had no serious practical need to override the http stack, storage layer, thread pool, data viewer, etc.. It was all under assumption that if you want to customize these, you'd implement yet-another-PAL-impl. Now we are moving away from the other PAL impl, but I wonder if we can make that customization an optional extension that is hidden from an average SDK user?

[mkoscumb]

Agreed that the default user case should be as simple as possible if they're not in the business of configuring the SDK. If we've got time I'm good with discussing this after we figure out modules today.

[maxgolov]

@mkoscumb - most components by now hold a reference to ILogConfiguration or RuntimeConfiguration, as m_config , where we can fetch various parameters from there. I'd suggest we don't rework the ILogManager creation, but consider providing nicer getters / setters / checkers on m_config, i.e. check if x.y.z tree node exists, and if not - returning some defaults.. Basically an improvement to getters / setters / checkers of ILogConfiguration as a tree. I'm also thinking maybe we should just permanently get rid of ILogConfiguration variant implementation and use JSON from json.hpp instead. Since in most cases we do require json.hpp for this or the other reason, we'd actually save some workspace if we obsolete our internal in-house Variant.hpp