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

Add Support for Azure TokenCredential Authentication #7

Open irwin08 opened 2 years ago

irwin08 commented 2 years ago

My organization requires the use of identity-based authentication instead of a simple authorization key when working with cosmos. This is one possible implementation meeting these requirements.

One thing to note is that the signature of the AzureCosmosDb factory method has been altered to take a TokenCredential as well as an authorization key, with both having defaults of null. So, anyone using the default values for everything, and just the old required positional parameters should have no issues. However, if someone was using passing arguments positionally after authorizationKey, this would be a breaking change. However, the alternative is to pass in the TokenCredential argument at the very end of the method, which feels very unnatural.

tghamm commented 2 years ago

@irwin08 is this commit necessary if the changes referenced in #6 are merged? There they suggest providing your own client because of a Singleton policy, which seems like it would give you the control you want over construction of the client entirely. To be clear, I'm not the maintainer, I just did a lot of the work on 2.0. Your use case makes total sense, just wondering if their commit kills two birds with one stone?

irwin08 commented 2 years ago

@tghamm Sorry for the delayed response. Yes, I think if the #6 is merged, that will be fine and handle my use case. So I am happy with that solution. I think the only real advantage of my solution is it's a tiny bit faster to use and get going for someone who doesn't want to get bogged down in the details of making your own client. (However this is marginal at best, it isn't that much more work.)

So yes I agree with you and support #6 over my PR unless there are objections to #6 for some reason.