Configuration Feedback
niemyjski opened this issue · 7 comments
In the latest JavaScript rewrite (workspace branch). We've been talking about how we can simplify configuration in general. Here is a breakdown of that feedback which would cause some code to be deleted 👍🏻 .
Constructor
On a platform like java I think we would know a lot of the defaults that wouldn't change often in terms of services and configuration. Seems like we could default all the items of the Configuration Manager and have a simple empty constructor. The ExceptionlessClient could have a startup method where we pass a Function/Predicate that contains a new configuration object and a user could set any properties on that. Thoughts?
Combine Configuration and Configuration Manager?
Is there any reason we shouldn't combine these and move services into an aggregate root property?
Simplifying of services.
The module collector, request info collector environment collector and error parser probably will never change in most implementations so it doesn't make sense defining services or interfaces for them. If we simply do the work in the plugin we can simplify services configuration all together. We also moved these into a services property to clean up configuration.
Configuration endpoint
We try to load the configuration settings via submission client from a preconfigured config subdomain https://config.exceptionless.io. I noticed this was missing.
Server url
Setting the server url should override the config and heartbeat server urls as well (So if you are self hosting it will set all the servers to the same).
Constructor
On a platform like java I think we would know a lot of the defaults that wouldn't change often in terms of services and configuration. Seems like we could default all the items of the Configuration Manager and have a simple empty constructor. The ExceptionlessClient could have a startup method where we pass a Function/Predicate that contains a new configuration object and a user could set any properties on that. Thoughts?
If you see here:
I am utilizing the builder pattern. So ideally I don't want someone to initialize the ConfigurationManager using new. Because then he/she needs to pass all the values with lots of nulls I guess.
ConfigurationManager.builder().submissionClient(xxx).build();
So here it would be like calling the all argument constructor with all nulls but with xxx as the value for submission client. So if someone wants to customize the configuration manager he/she can do with this builder and pass it here:
So like above, all the classes are following a builder pattern so the client should be built in this way:
ExceptionlessClient.builder().configurationManager(configuraitonManager).build()
So in summary an example where someone wants to customize by adding a new plugin, he/she needs following code:
ConfigurationManager configurationManager =
ConfigurationManager.builder()
.configuration(
Configuration.builder().serverUrl("server-url").apiKey("api-key").build())
.build();
configurationManager.addPlugin(
"plugin-name", 100, (ctx, cm) -> System.out.println("Do something"));
ExceptionlessClient client =
ExceptionlessClient.builder().configurationManager(configurationManager).build();
Let me know if I am not clear enough 😅
Combine Configuration and Configuration Manager?
Is there any reason we shouldn't combine these and move services into an aggregate root property?
The only reason I broke them down is to avoid passing ConfigurationManager to its children such as SettingsManager or SubmissionClient. I am strictly against cyclic dependencies.
Server url
Setting the server url should override the config and heartbeat server urls as well (So if you are self hosting it will set all the servers to the same).
By default during creation if heartbeatServerUrl and configServerUrl are not given then it is configured to serverUrl by default. But I don't think setting serverUrl should update all three URLs. The user should update them separately as it can cause confusion.
Simplifying of services.
The module collector, request info collector environment collector and error parser probably will never change in most implementations so it doesn't make sense defining services or interfaces for them. If we simply do the work in the plugin we can simplify services configuration all together. We also moved these into a services property to clean up configuration.
Configuration endpoint
We try to load the configuration settings via submission client from a preconfigured config subdomain https://config.exceptionless.io. I noticed this was missing.
Server url
Setting the server url should override the config and heartbeat server urls as well (So if you are self hosting it will set all the servers to the same).By default during creation if
heartbeatServerUrlandconfigServerUrlare not given then it is configured toserverUrlby default. But I don't think settingserverUrlshould update all three URLs. The user should update them separately as it can cause confusion.
It should because if you are self hosting would be the only time you would set the serverUrl and at that time you'd want all the endpoints to talk to your self hosted service.
Constructor
On a platform like java I think we would know a lot of the defaults that wouldn't change often in terms of services and configuration. Seems like we could default all the items of the Configuration Manager and have a simple empty constructor. The ExceptionlessClient could have a startup method where we pass a Function/Predicate that contains a new configuration object and a user could set any properties on that. Thoughts?If you see here:
I am utilizing the builder pattern. So ideally I don't want someone to initialize the
ConfigurationManagerusingnew. Because then he/she needs to pass all the values with lots of nulls I guess.ConfigurationManager.builder().submissionClient(xxx).build();So here it would be like calling the all argument constructor with all nulls but with
xxxas the value for submission client. So if someone wants to customize the configuration manager he/she can do with this builder and pass it here:
So like above, all the classes are following a builder pattern so the client should be built in this way:
ExceptionlessClient.builder().configurationManager(configuraitonManager).build()So in summary an example where someone wants to customize by adding a new plugin, he/she needs following code:
ConfigurationManager configurationManager = ConfigurationManager.builder() .configuration( Configuration.builder().serverUrl("server-url").apiKey("api-key").build()) .build(); configurationManager.addPlugin( "plugin-name", 100, (ctx, cm) -> System.out.println("Do something")); ExceptionlessClient client = ExceptionlessClient.builder().configurationManager(configurationManager).build();Let me know if I am not clear enough 😅
That works, Seeing this again I think we should rename configurationManager to configuration. It doesn't manage anything it just hold configuration settings.