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.

Sign up for GitHub

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

Users/davetheunissen/query metrics option #38

Closed
wants to merge 3 commits into from
Closed

Users/davetheunissen/query metrics option #38

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
371b41f
Add PopulateQueryMetrics property to CosmosQueryRequestOptions
Feb 1, 2019
62e7e4d
Make CosmosQueryResponse Resources property public
Feb 1, 2019
84d0cb4
Add unit test for PopulateQueryMetrics feed option
Feb 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Microsoft.Azure.Cosmos/src/RequestOptions/CosmosQueryRequestOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,17 @@ public class CosmosQueryRequestOptions : CosmosRequestOptions
/// </remarks>
public virtual ConsistencyLevel? ConsistencyLevel { get; set; }

/// <summary>
/// Gets or sets the populate query metrics request option for document query requests in the Azure Cosmos DB service.
/// </summary>
/// <remarks>
/// <para>
/// PopulateQueryMetrics is used to enable/disable getting metrics relating to query execution on document query requests.
/// </para>
/// </remarks>
public bool PopulateQueryMetrics { get; set; }


/// <summary>
/// Gets or sets the number of concurrent operations run client side during
/// parallel query execution in the Azure Cosmos DB service.
Expand Down Expand Up @@ -153,6 +164,7 @@ internal FeedOptions ToFeedOptions()
EnableScanInQuery = this.EnableScanInQuery,
EnableLowPrecisionOrderBy = this.EnableLowPrecisionOrderBy,
MaxBufferedItemCount = this.MaxBufferedItemCount,
PopulateQueryMetrics = this.PopulateQueryMetrics
};
}

Expand Down
6 changes: 5 additions & 1 deletion Microsoft.Azure.Cosmos/src/Resource/QueryResponses/CosmosQueryResponse.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ namespace Microsoft.Azure.Cosmos
/// <typeparam name="T"></typeparam>
public class CosmosQueryResponse<T> : IEnumerable<T>
{
private IEnumerable<T> Resources;
/// <summary>
/// Contains the metadata from the response headers from the Azure Cosmos DB call
/// </summary>
public IEnumerable<T> Resources;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this being made public? The CosmosQueryResponse already implements IEnumerable.

Copy link
Author

@davetheunissen davetheunissen Feb 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

As far as I can tell, there is no way to cast the Resources object to CosmosQueryResponse<T> because internally it is of Type FeedResponse<T>.

The only alternate I can see is to create a public property in CosmosQueryResponse that returns FeedOptions<T> from Resources. Not sure that this is great option though?

If you have an example of how this might work, I'm happy to learn and ammend the PR etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kirankumarkolli any suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't the CosmosResultSetIterator work to iterate with any type, like it works for CreateItemQuery?

Copy link
Author

@davetheunissen davetheunissen Feb 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

If I create a CosmosResultSetIterator<FeedReponse<T>> FetchNextSetAsync throws an exception because the document coming back from CosmosDB doesn't match that type.

CosmosResultSetIterator<T> returns an enumerable collection of CosmosQueryResponse. CosmosQueryResponse<T> returns an enumerator of Resources<FeedResponse<T>>. The FeedResponse includes the meta properties about the request so we don't need to enumerate Resources, we just need access to the FeedResponse<T> properties.

image

Given that FeedResponse<T> is an internal class, there isn't a way to create a public accessor property without changing the internal scope.

If there is an issue with making Resources public then I'll need to try something else to get at the FeedResponse properties.

Appreciate any help or thoughts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should there be a (new) public IQueryMetrics interface that IFeedResponse<T> would inherit from, and that's the interface we would use to expose query metrics from CosmosQueryResponse<T> through the Resources property? (although I don't think "Resources" would be a good name for that purpose)

Copy link
Author

@davetheunissen davetheunissen Feb 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The IFeedResponse<T> interface already contains all the properties that we'd like to expose... incl. query metrics... however that's also scoped as internal. I'll have another crack at this on the weekend.


private bool HasMoreResults;

/// <summary>
Expand Down
33 changes: 33 additions & 0 deletions Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.EmulatorTests/CosmosItemTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,39 @@ public async Task ValidateMaxItemCountOnItemQuery()
}
}

/// <summary>
/// Validate that the PopulateQueryMetrics feed option returns query metrics
/// </summary>
/// <returns></returns>
[TestMethod]
public async Task ValidatePopulateQueryMetricsOnQuery()
{
// create a random set of data
IList<ToDoActivity> items = await CreateRandomItems(10, randomPartitionKey: false);

try
{
var query = new CosmosSqlQueryDefinition("SELECT * FROM toDoActivity");
var resultSetIterator = this.Container.Items.CreateItemQuery<ToDoActivity>(query, "TBD", requestOptions: new CosmosQueryRequestOptions() { PopulateQueryMetrics = true });
while (resultSetIterator.HasMoreResults)
{
var result = await resultSetIterator.FetchNextSetAsync();
var resources = (FeedResponse<ToDoActivity>)result.Resources;

// assert that query metrics were populated in the response.
Assert.IsTrue(resources.QueryMetrics.Count() > 0);
}
}
finally
{
foreach (ToDoActivity item in items)
{
CosmosResponseMessage deleteResponse = await this.Container.Items.DeleteItemStreamAsync(item.status, item.id);
deleteResponse.Dispose();
}
}
}

/// <summary>
/// Validate that the max item count works correctly.
/// </summary>
Expand Down