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

Adding basic request diagnostics for V3 #615

Merged
merged 26 commits into from
Aug 30, 2019

Conversation

simplynaveen20
Copy link
Member

@simplynaveen20 simplynaveen20 commented Jul 31, 2019

This PR will add the basic point operation statistics and query metrics for both type and stream api.
We are exposing GetDiagnostics() on the responses for users to get the diagnostic string.

Point Operation - 
ItemResponse<ItemObject> createResponse = await this.Container.CreateItemAsync<ItemObject>(item: testItem);
string cosmosDiagnosticsString = createResponse.cosmosDiagnostics.ToString();

Query Operator -
FeedIterator<ItemObject> feedIterator = this.Container.GetItemQueryIterator<ItemObject>(
                   sql,
                   requestOptions: requestOptions);

if (feedIterator.HasMoreResults)
{
    FeedResponse<ToDoActivity> iter = await feedIterator.ReadNextAsync();
    string cosmosDiagnosticsString = iter.cosmosDiagnostics.ToString();
}

closes #645

j82w
j82w previously approved these changes Aug 22, 2019
@simplynaveen20 simplynaveen20 changed the title Basic request diagnostics for V3 Adding basic request diagnostics for V3 Aug 22, 2019
@kirankumarkolli
Copy link
Member

Please include the new contracts and usage into the description.

Cosmos DB SDK team automation moved this from Reviewer approved to Review in progress Aug 26, 2019
Copy link
Member

@kirankumarkolli kirankumarkolli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not auto populate always 'PopulateQueryMetrics'?

@kirankumarkolli
Copy link
Member

kirankumarkolli commented Aug 29, 2019

Summary of offline discussion:

public class ResponseMessage
{
   ....
   
   public string GetDiagnostics()
   {
     return this.cosmosDiag.ToString();
   }
}

   Example diagnostic string:
   {
     Summary: E200,
	 Diag: {
		...
	 }
	 .....
   }
   
   internal CosmosDiagnostics
   {
   }

Copy link
Member

@kirankumarkolli kirankumarkolli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refresh contract as listed in comments

Cosmos DB SDK team automation moved this from Review in progress to Reviewer approved Aug 30, 2019
@kirankumarkolli kirankumarkolli merged commit 76e76dd into master Aug 30, 2019
Cosmos DB SDK team automation moved this from Reviewer approved to Done Aug 30, 2019
@kirankumarkolli kirankumarkolli deleted the users/nakumar/requestDiagnosticsV3 branch August 30, 2019 23:58
@AlanMacdonald
Copy link

I realise this has been merged but I can't find any technical documentation on it. I tried to use this to get the total results for paging. Ie user is viewing 1-100 of 1000 items type scenario. However I set up 3 items in the db - the first 2 had property Key1 set to "A" and the third Key1 set to "B". If I search for items where Key1 = "A" and look at the diagnostics object and inside that the query stats then the RetrievedDocumentCount it is 3 and not 2. The third item did not qualify for the query. Is this expected bahaviour? If so I cannot use it to get a total item count for the paging scenario.

This is compounded by https://feedback.azure.com/forums/263030-azure-cosmos-db/suggestions/36142468-make-count-aware-of-indexes never having been implemented because we have to have a second query to count the total number of qualifying items for search criteria and it performs woefully because no indexes are used for the count query so this is causing huge performance issues.

@j82w
Copy link
Contributor

j82w commented Nov 28, 2019

@AlanMacdonald can you create a new issue for this? Commenting on a closed PR is difficult to track. This might be fixed by one of the several changes that will go out in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Adding requestDiagnostic and query metric in V3
4 participants