search

a8m / documentdb

Go driver for Microsoft Azure DocumentDB
MIT License
33 stars 25 forks source link

feat: add User-Agent and app id #35

Closed christopheranderson closed 3 years ago

christopheranderson commented 3 years ago

Objective:

Goal of this PR is to add a User-Agent and all for app id to be set. This helps with supportability by Cosmos DB support & some telemetry tooling.

Changes:

christopheranderson commented 3 years ago

I changed my approach with this iteration to avoid joining the different parts each request (because I also didn't want to ReadBuildInfo on every request). Hopefully that ends up being a net improvement.

Open to feedback on pushing UserAgent up to Client and setting it from documentdb - I felt this was a better approach than putting UserAgent on Config, which adds public surface area. It's possible there's a cleaner way than having to add an argument to DefaultHeaders, though.

artursouza commented 3 years ago

Dapr is interested in this feature.

greenie-msft commented 3 years ago

@a8m @christopheranderson - Is this in a position to be merged?

Thank you.

a8m commented 3 years ago

@a8m @christopheranderson - Is this in a position to be merged?

Thank you.

PR was approved. @christopheranderson, do you want to merge it?

artursouza commented 3 years ago

@a8m I don't have permission to merge this. @christopheranderson ?

a8m commented 3 years ago

@artursouza, please accept the invitation link.

Let’s wait to get OK from @christopheranderson as he wanted to test it locally before merging it.

artursouza commented 3 years ago

@artursouza, please accept the invitation link.

Let’s wait to get OK from @christopheranderson as he wanted to test it locally before merging it.

Just did. Thanks for the reminder.

a8m commented 3 years ago

Any updates @christopheranderson?