Azure / azure-rest-api-specs

The source for REST API specifications for Microsoft Azure.
MIT License
2.63k stars 5.08k forks source link

[Microsoft.CostManagement] Query_Usage missing pagination in specification #23405

Closed jkroepke closed 1 year ago

jkroepke commented 1 year ago

Hi,

The API REST Call $scope/providers/Microsoft.CostManagement/query supports pagination, but its currently specified on the API docs.

I could re-validate this by manually calling the API with curl:

$ curl https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=2 | jq .properties.nextLink
https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=2&$skiptoken=AQAAAA%3D%3D

My answer contains more than 2 results and a nextLink with skip token provided by the API.

In conclusion, since the API Docs doesn't have this documented, downstream SDKs are not supporting pagination on that API Call, too. This results into issues that only 1000 elements can be queried.

navba-MSFT commented 1 year ago

@jkroepke Apologies for the late reply. Thanks for reaching out to us and reporting this issue. Could you please check if the below workaround sample code helps ?

https://gist.github.com/JoramM/c0538593c80822e37f6cd6d0d2c0c65f

navba-MSFT commented 1 year ago

@jkroepke I wanted to do quick follow-up to check if you had a chance to look at my above comment. Please let us know if you had any updates on this. Awaiting your reply.

jkroepke commented 1 year ago

@navba-MSFT Thanks for looking into this.

The code you provides contains some type mismatches. I'm using go 1.20.

Here an updated version based on you code.

https://github.com/jkroepke/homelab/blob/571c16491f2f2205a3d52e5ce046aa7adfb8a1d2/go/azure-cost-query-pagination/main.go

I adding some logging and error handling to see it's working.

Running

export AZURE_CLIENT_ID=".." 
export AZURE_CLIENT_SECRET="...
export AZURE_SUBSCRIPTION_ID="..."
go run main.go

returns

2023/04/12 12:42:55 https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=100
2023/04/12 12:42:58 È:200 OK
2023/04/12 12:43:01 https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=100&$skiptoken=AQAAAA%3D%3D
2023/04/12 12:43:04 È:200 OK
2023/04/12 12:43:07 https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=100&$skiptoken=AgAAAA%3D%3D
2023/04/12 12:43:09 È:200 OK
2023/04/12 12:43:12 https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=100&$skiptoken=AwAAAA%3D%3D
2023/04/12 12:43:15 È:200 OK
2023/04/12 12:43:18 https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=100&$skiptoken=BAAAAA%3D%3D
2023/04/12 12:43:18 ƭ:429 Too Many Requests
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x126e469]

goroutine 1 [running]:
main.main()
        /Users/jkr/IdeaProjects/private/homelab/go/azure-cost-query-pagination/main.go:96 +0xc29
exit status 2

In general it works. Its clearly visible, that pagination may works. I use $top=100 here as additional parameter to force pagination.

However sometimes I'm hitting the rate limits. As I know. Cost Management has very low and specific rate limits. Thats one of the reason, I would like to use the Azure SDK instead doing the requests manually, because the SDK implements some auto-retry mechanism.

However, with $top=1000, it works fine.

jkr@joe-nb azure-cost-query-pagination % go run main.go
2023/04/12 12:48:28 https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=1000
2023/04/12 12:48:32 È:200 OK
2023/04/12 12:48:35 https://management.azure.com/subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/providers/Microsoft.CostManagement/query?api-version=2021-10-01&$top=1000&$skiptoken=AQAAAA%3D%3D
2023/04/12 12:48:39 È:200 OK
2023/04/12 12:48:42 [0.10439673312 2.0230313e+07 /subscriptions/e1608e24-0728-4efd-ba5b-a05693b53c5a/resourcegroups/jok-default/providers/microsoft.compute/disks/bastion-linux-root EUR]
...
2023/04/12 12:48:42 1391
navba-MSFT commented 1 year ago

@jkroepke I am discussing this with the Product Owners, I will update once I hear from them.

jkroepke commented 1 year ago

@weidongxu-microsoft @navba-MSFT Maybe you can bundle your power here.

Thanks a lot for moving forward this!

weidongxu-microsoft commented 1 year ago

The problem is the design of the service response does not match the agreed pageable solution. https://github.com/Azure/autorest/blob/main/docs/extensions/readme.md#x-ms-pageable Hence SDK not able to support this unexpected payload.

The expected response for a pageable would be (pagination on value)

value: [...]
nextLink: https://...

But cost-management returns this

id: ...
name: ...
type: ...
properties:
  nextLink: https://
  columns: [...]
  rows: [...]

And they expect the pagination happens on the properties.rows.

One can see the 2 response is not compatible at all.


One solution I can think of, is to ask backend to add "skipToken" parameter to API, and include "skipToken" in response if "nextLink" available. And user would need to do the loop by themselves, when "skipToken" exists in response, use it to send another request for next page.

The undesirable part of this solution is that it exposes the implementation detail of server-side paging. Cost-management then cannot switch to another mechanism other than "skipToken".

Aware that I am SDK person. I don't have influence on how cost-management design their API.

jkroepke commented 1 year ago

Thanks for the clarifications!

One solution I can think of, is to ask backend to add "skipToken" parameter to API, and include "skipToken" in response if "nextLink" available.

If you take a look at https://github.com/Azure/azure-rest-api-specs/issues/23405#issuecomment-1505064440, the API already support $skipToken and behavior you present (like getting the new page) is already present.

Seems like full paginations exists, but not in a standard Azure API way.

weidongxu-microsoft commented 1 year ago

Thanks for the clarifications!

One solution I can think of, is to ask backend to add "skipToken" parameter to API, and include "skipToken" in response if "nextLink" available.

If you take a look at #23405 (comment), the API already support $skipToken and behavior you present (like getting the new page) is already present.

Seems like full paginations exists, but not in a standard Azure API way.

Yes. It exists in backend, but it is not exposed to swagger, and hence SDK won't have it in API.

Also current the skipToken can only be extracted from nextLink in response, which is not very simple for user not familiar with URL parsing and escaping.

Yes, full pagination exists, but it does not conform to the shared requirement by SDK, and hence SDK not able to support that customized pagination.

navba-MSFT commented 1 year ago

@weidongxu-microsoft I have an active email thread going with the Service team. I will loop you there for awareness. Currently I am waiting for their reply.

navba-MSFT commented 1 year ago

@jkroepke I had a discussion with the Cost Management Query API product owners. Sharing the update here.

Most ARM APIs deal with Creating/Updating/Getting/Deleting an azure resource, so a get response is usually in the form of a “value” array of objects that represent the resource(s) information. Example:

image

In our case, we do not do the above operations on a resource, we simply fetch the aggregated cost of a resource based o what the customer specifies (and I am sure, as customers of the API, you already know that).

Hence, our response in unconventional:

image

So due to the above we cannot add the x-ms-pageable field in the REST API specs swagger. And since this is not available in the swagger, the SDK cannot provide this functionality.

So in short, the ONLY workaround here is to invoke the Cost management query API, Get the NextLink marker and enumerate through it until the provided nextLink is null. Samples are here and here. Hope this helps.