Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CosmosDiagnostics: Add active client count #2589

Closed
j82w opened this issue Jul 6, 2021 · 10 comments · Fixed by #3037
Closed

CosmosDiagnostics: Add active client count #2589

j82w opened this issue Jul 6, 2021 · 10 comments · Fixed by #3037
Assignees
Labels
Diagnostics Issues around diagnostics and troubleshooting feature-request New feature or request
Milestone

Comments

@j82w
Copy link
Contributor

j82w commented Jul 6, 2021

The diagnostics currently only has the number of created clients. This issue is to track adding the number of active clients. This way we can see if a user is properly disposing of clients for certain cases.

@j82w j82w added the Diagnostics Issues around diagnostics and troubleshooting label Jul 6, 2021
@j82w j82w added the feature-request New feature or request label Jul 6, 2021
@ealsur
Copy link
Member

ealsur commented Jul 8, 2021

We need to think about scenarios where the customer is doing:

using (CosmosClient client = new CosmosClient(...))

If this is executed concurrently and in high numbers it is still a bad scenario, the number of clients created would be high and the fact that the active clients might be low doesn't mean everything is correct.
Creating and disposing continuously will also lead to Connection limiting or issues, because connections are not closed right away (https://www.aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/).

@j82w
Copy link
Contributor Author

j82w commented Jul 8, 2021

There will be a TotalNumberOfClientsCreated and a ActiveNumberOfClientsCreated. If there is some crazy number of total clients it will still be obvious. The active number will tell if the user is disposing of the client or has N clients active. It may also help in scenarios where users think they are disposing of the clients but are not.

@ezgambac
Copy link

In our scenario we are having the clients in cache where they are removed after a certain time being unused, so we would be creating and deleting them often.

@kirankumarkolli
Copy link
Member

There are genuine scenarios to create multiple clients (ex: multiple accounts). The data needs to dis-ambiguate or at-least have right semantics of interpretation. One option is to have normalized counter like per account #clients (MAX)

@j82w
Copy link
Contributor Author

j82w commented Jan 24, 2022

For troubleshooting I think having a total active count is sufficient. It doesn't matter if they have 100 SDK instances with 100 different accounts vs 100 SDK instance for the same account. Both will run into the same issue of causing to many connections.

@ezgambac
Copy link

We are currently tracking total number of active clients globally, not per account, if that helps as a data point.
We also make an effort to have 1 client per account.

@sourabh1007 sourabh1007 modified the milestones: Jan 2022, Feb 2022 Feb 2, 2022
@sourabh1007
Copy link
Contributor

Recommendation is: Customer should have 1 Client Instance per Account. if User have multiple accounts, then there is no other way except creating multiple client instances.

So We should collect how many clients is being created per account. E.g. Customer have 10 accounts and then TotalNumberOfClientsCreated and ActiveNumberOfClientsCreated will say 10 instances are being created and active (we will consider it as a bad practice), but in actual, nothing can be done to reduce the number of client instance.
So These 2 metrics may mislead us. But, if we collect it for each account then, we will have clear picture if we should even look into this direction or not.

@kirankumarkolli
Copy link
Member

kirankumarkolli commented Feb 8, 2022

UserAgent do already cover some aspects of it.
What extra context do we get through this telemetry?

Or if the # createdClients is not relevant why not change to activeClients in userAgent?

@j82w
Copy link
Contributor Author

j82w commented Feb 8, 2022

The number of created clients is more important than number of active clients. The number of created clients tells if the application is creating multiple instances or recreating instances. If just the number of active instances is recorded it will not be possible to tell if the customer is recreating instance for each call. This is the most common issue observed.

The problem with just capturing total number of clients created is there are scenarios where customers by design recreates instances. This usually occurs is middle tier services where the customer is reselling Cosmos DB. In those scenarios it's not possible to tell how many active clients instance exist. The process having 2 active clients vs 200 is a big difference, and can be helpful in root causing certain issues.

@ezgambac
Copy link

ezgambac commented Feb 8, 2022 via email

@sourabh1007 sourabh1007 moved this from Triage to In Progress in Azure Cosmos SDKs Feb 17, 2022
Repository owner moved this from In Progress to Done in Azure Cosmos SDKs Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Diagnostics Issues around diagnostics and troubleshooting feature-request New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants