opentelemetry-collector-contrib
opentelemetry-collector-contrib copied to clipboard
[receiver/azuremonitor] Add parameter to overwrite azure's max record size
Description: When having lots of different records of one dimension in the metric, azure by default only returns 10 of them. This setting adds the possibility to overwrite the default and specify a custom number in the config of the receiver.
Link to tracking Issue: #32165
Testing: Tested fetching metrics with different configs. Do we need a unit test for this?
Documentation: Added parameter to README
Adding flexibility to support more use cases generally makes sense to me. Looking forward to see this touched up to pass CI. Let me know if I can help with anything.
Adding flexibility to support more use cases generally makes sense to me. Looking forward to see this touched up to pass CI. Let me know if I can help with anything.
Have rebased the branch, now CI passed
This PR was marked stale due to lack of activity. It will be closed in 14 days.
@codeboten Can I get a review for this feature please?
This PR was marked stale due to lack of activity. It will be closed in 14 days.
@nslaughter @codeboten Anyone here to get this merged?
Could we add some tests here to ensure this config option is applied in the expected way? (To show that it does properly limit the max?)
Sure, I can add tests but I'm not sure if the tests verify what you are asking for here. The top
parameter is applied to the Azure API and the API limits the max (so outside the receiver itself) and since we are not having tests with explicit Azure API calls here, it is hard to verify that the parameter is actually limiting the returned results. What I can test though is if the parameter is applied to the API call correctly. Would that be okay for you?
Could we add some tests here to ensure this config option is applied in the expected way? (To show that it does properly limit the max?)
Sure, I can add tests but I'm not sure if the tests verify what you are asking for here. The
top
parameter is applied to the Azure API and the API limits the max (so outside the receiver itself) and since we are not having tests with explicit Azure API calls here, it is hard to verify that the parameter is actually limiting the returned results. What I can test though is if the parameter is applied to the API call correctly. Would that be okay for you?
That's a fair point. If you're able to verify the argument is being passed to the API properly that's enough for me. It's a pretty simple change here passing in the value, so I'm not too worried about it.
This PR was marked stale due to lack of activity. It will be closed in 14 days.