search

mahdighorbanpour / serilog-sinks-azurecosmosdb

A Serilog sink that writes to Azure CosmosDB and supports PartitionKey for better performance.
Apache License 2.0
4 stars 5 forks source link

This package violates CosmosClient singleton rules #6

Open tcsidevtools opened 2 years ago

tcsidevtools commented 2 years ago

There should be only one instance of CosmosClient per application. By not allowing us to pass in our own client, using this package means essentially, you are maintaining two CosmosClients or more. In a microservice environment this can really balloon up. (7 services, 7 clients, using this package =14 clients).

I make the following recommendations -Allow the ability to pass in your own CosmosClient -Allow the ability to pass in your own options to the CosmosBuilder -At the very minimum, SSL to the CosmosBuilder should be togglable. Local networked CosmosDB testing environments don't have -good solutions for SSL see here Microsoft recommends turning off SSL validation at the application level in these instances. Since that is not an option for this package, Its impossible to test it in these situations

I am unsure if this package is being maintained. I will fork these changes to my own branch for the moment. However if the owner of this repo would like me to make a pull request I will do so.

Arcalise08 commented 2 years ago

These changes have been made and are available here.

tghamm commented 2 years ago

@tcsidevtools I believe the owner is still accepting PRs. I think this is a solid change. @mahdighorbanpour is this something you'd consider merging? I'd like to round out my open PR as well as part of the same release if so.

tcsidevtools commented 2 years ago

@tghamm I've no problem submitting a PR. But I'd like to hear from the maintainer before we do so.