Azure.Monitor.Query: invalid date string format in MetricsClient (#45998

)
Azure · Oct 8, 2024 · e1469a2 · e1469a2
1 parent 984ecd1
commit e1469a2
Show file tree

Hide file tree

Showing 10 changed files with 227 additions and 80 deletions.
diff --git a/sdk/monitor/Azure.Monitor.Query/CHANGELOG.md b/sdk/monitor/Azure.Monitor.Query/CHANGELOG.md
@@ -7,6 +7,7 @@
 ### Breaking Changes
 
 ### Bugs Fixed
+- Fix bug in 'MetricsClient' QueryResourceAsync method where the 'QueryTimeRange' parameter was incorrectly set
 
 ### Other Changes
 

diff --git a/sdk/monitor/Azure.Monitor.Query/assets.json b/sdk/monitor/Azure.Monitor.Query/assets.json
@@ -2,5 +2,5 @@
   "AssetsRepo": "Azure/azure-sdk-assets",
   "AssetsRepoPrefixPath": "net",
   "TagPrefix": "net/monitor/Azure.Monitor.Query",
-  "Tag": "net/monitor/Azure.Monitor.Query_2eaa6ed059"
+  "Tag": "net/monitor/Azure.Monitor.Query_26f5d5d32f"
 }
diff --git a/sdk/monitor/Azure.Monitor.Query/src/MetricsClient.cs b/sdk/monitor/Azure.Monitor.Query/src/MetricsClient.cs
@@ -150,8 +150,8 @@ private async Task<Response<MetricsQueryResourcesResult>> ExecuteBatchAsync(IEnu
             {
                 if (options.TimeRange != null)
                 {
-                    startTime = options.TimeRange.Value.Start.ToString();
-                    endTime = options.TimeRange.Value.End.ToString();
+                    startTime = options.TimeRange.Value.Start.ToIsoString();
+                    endTime = options.TimeRange.Value.End.ToIsoString();
                 }
                 aggregations = MetricsClientExtensions.CommaJoin(options.Aggregations);
                 top = options.Size;

diff --git a/sdk/monitor/Azure.Monitor.Query/src/MetricsClientExtensions.cs b/sdk/monitor/Azure.Monitor.Query/src/MetricsClientExtensions.cs
@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using System.Text;
 using Azure.Monitor.Query.Models;
@@ -51,5 +52,20 @@ internal static IList<string> CommaSplit(string value) =>
                 new List<string>() :
                 // TODO: #10600 - Verify we don't need to worry about escaping
                 new List<string>(value.Split(','));
+
+        internal static string ToIsoString(this DateTimeOffset value)
+        {
+            if (value.Offset == TimeSpan.Zero)
+            {
+                // Some Azure service required 0-offset dates to be formatted without the
+                // -00:00 part
+                const string roundtripZFormat = "yyyy-MM-ddTHH:mm:ss.fffffffZ";
+                return value.ToString(roundtripZFormat, CultureInfo.InvariantCulture);
+            }
+
+            return value.ToString("O", CultureInfo.InvariantCulture);
+        }
+
+        internal static string ToIsoString(this DateTimeOffset? value) => value?.ToIsoString();
     }
 }
diff --git a/sdk/monitor/Azure.Monitor.Query/src/MetricsQueryResourcesOptions.cs b/sdk/monitor/Azure.Monitor.Query/src/MetricsQueryResourcesOptions.cs
@@ -14,9 +14,10 @@ namespace Azure.Monitor.Query
     public class MetricsQueryResourcesOptions
     {
         /// <summary>
-        /// Gets or sets the timespan over which the metric will be queried.
+        /// Gets or sets the timespan over which the metric will be queried. If only the starttime is set, the endtime default becomes the current time. When the endtime is specified, the starttime is necessary as well. Duration is disregarded.
         /// </summary>
         [CodeGenMember("TimeSpan")]
+        // TODO: https://github.com/Azure/azure-sdk-for-net/issues/46454
         public QueryTimeRange? TimeRange { get; set; }
 
         /// <summary>

diff --git a/sdk/monitor/Azure.Monitor.Query/src/QueryTimeRange.cs b/sdk/monitor/Azure.Monitor.Query/src/QueryTimeRange.cs
@@ -2,7 +2,6 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Globalization;
 using System.Xml;
 using Azure.Core;
 
@@ -105,21 +104,8 @@ public override string ToString()
 
         internal string ToIsoString()
         {
-            string ToString(DateTimeOffset value)
-            {
-                if (value.Offset == TimeSpan.Zero)
-                {
-                    // Some Azure service required 0-offset dates to be formatted without the
-                    // -00:00 part
-                    const string roundtripZFormat = "yyyy-MM-ddTHH:mm:ss.fffffffZ";
-                    return value.ToString(roundtripZFormat, CultureInfo.InvariantCulture);
-                }
-
-                return value.ToString("O", CultureInfo.InvariantCulture);
-            }
-
-            var startTime = Start != null ? ToString(Start.Value) : null;
-            var endTime = End != null ? ToString(End.Value) : null;
+            var startTime = Start.ToIsoString();
+            var endTime = End.ToIsoString();
             var duration = XmlConvert.ToString(Duration);
 
             switch (startTime, endTime, duration)

diff --git a/sdk/monitor/Azure.Monitor.Query/tests/LogsQueryClientLiveTests.cs b/sdk/monitor/Azure.Monitor.Query/tests/LogsQueryClientLiveTests.cs
@@ -707,7 +707,7 @@ public async Task CanSetServiceTimeout()
                     // or a partial failure 200 response
                     if (exception.Status == 200)
                     {
-                        StringAssert.Contains("Query cancelled by the user's request", exception.Message);
+                        StringAssert.Contains("PartialError", exception.Message);
                     }
                     else
                     {

diff --git a/sdk/monitor/Azure.Monitor.Query/tests/MetricsClientLiveTests.cs b/sdk/monitor/Azure.Monitor.Query/tests/MetricsClientLiveTests.cs
@@ -0,0 +1,177 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Azure.Core;
+using Azure.Core.TestFramework;
+using Azure.Monitor.Query.Models;
+using NUnit.Framework;
+
+namespace Azure.Monitor.Query.Tests
+{
+    public class MetricsClientLiveTests : RecordedTestBase<MonitorQueryTestEnvironment>
+    {
+        private MetricsTestData _testData;
+
+        public MetricsClientLiveTests(bool isAsync) : base(isAsync)
+        {
+        }
+
+        private MetricsClient CreateMetricsClient()
+        {
+            return InstrumentClient(new MetricsClient(
+                new Uri(TestEnvironment.ConstructMetricsClientUri()),
+                TestEnvironment.Credential,
+                InstrumentClientOptions(new MetricsClientOptions()
+                {
+                    Audience = TestEnvironment.GetMetricsClientAudience()
+                })
+            ));
+        }
+
+        [SetUp]
+        public void SetUp()
+        {
+            _testData = new MetricsTestData(TestEnvironment, Recording.UtcNow);
+        }
+
+        [RecordedTest]
+        public async Task MetricsQueryResourcesAsync()
+        {
+            MetricsClient client = CreateMetricsClient();
+
+            var resourceId = TestEnvironment.StorageAccountId;
+
+            Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
+                resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
+                metricNames: new List<string> { "Ingress" },
+                metricNamespace: "Microsoft.Storage/storageAccounts").ConfigureAwait(false);
+
+            Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
+            MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
+            Assert.AreEqual(1, metricsQueryResults.Values.Count);
+            Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
+            Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
+            for (int i = 0; i < metricsQueryResults.Values.Count; i++)
+            {
+                foreach (MetricResult value in metricsQueryResults.Values[i].Metrics)
+                {
+                    for (int j = 0; j < value.TimeSeries.Count; j++)
+                    {
+                        Assert.GreaterOrEqual(value.TimeSeries[j].Values[i].Total, 0);
+                    }
+                }
+            }
+        }
+
+        [RecordedTest]
+        public async Task MetricsQueryResourcesWithStartEndTimeRangeAsync()
+        {
+            MetricsClient client = CreateMetricsClient();
+
+            var resourceId = TestEnvironment.StorageAccountId;
+
+            var timeRange = new QueryTimeRange(
+                start: Recording.UtcNow.Subtract(TimeSpan.FromHours(4)),
+                end: Recording.UtcNow
+            );
+
+            Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
+                resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
+                metricNames: new List<string> { "Ingress" },
+                metricNamespace: "Microsoft.Storage/storageAccounts",
+                options: new MetricsQueryResourcesOptions { TimeRange = timeRange} ).ConfigureAwait(false);
+
+            Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
+            MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
+            Assert.AreEqual(1, metricsQueryResults.Values.Count);
+            Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
+            Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
+        }
+
+        [RecordedTest]
+        public async Task MetricsQueryResourcesWithStartDurationTimeRangeAsync()
+        {
+            MetricsClient client = CreateMetricsClient();
+
+            var resourceId = TestEnvironment.StorageAccountId;
+
+            var timeRange = new QueryTimeRange(
+                start: Recording.UtcNow.Subtract(TimeSpan.FromHours(4)),
+                duration: TimeSpan.FromHours(4)
+            );
+
+            Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
+                resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
+                metricNames: new List<string> { "Ingress" },
+                metricNamespace: "Microsoft.Storage/storageAccounts",
+                options: new MetricsQueryResourcesOptions { TimeRange = timeRange }).ConfigureAwait(false);
+
+            Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
+            MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
+            Assert.AreEqual(1, metricsQueryResults.Values.Count);
+            Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
+            Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
+        }
+
+        [RecordedTest]
+        [SyncOnly]
+        public void MetricsQueryResourcesWithEndDurationTimeRange()
+        {
+            MetricsClient client = CreateMetricsClient();
+
+            var resourceId = TestEnvironment.StorageAccountId;
+
+            var timeRange = new QueryTimeRange(
+                end: Recording.UtcNow,
+                duration: TimeSpan.FromHours(4)
+            );
+
+            Assert.Throws<AggregateException>(() =>
+                client.QueryResources(
+                resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
+                metricNames: new List<string> { "Ingress" },
+                metricNamespace: "Microsoft.Storage/storageAccounts",
+                options: new MetricsQueryResourcesOptions { TimeRange = timeRange }));
+        }
+
+        [RecordedTest]
+        public async Task MetricsQueryResourcesWithDurationTimeRangeAsync()
+        {
+            MetricsClient client = CreateMetricsClient();
+
+            var resourceId = TestEnvironment.StorageAccountId;
+
+            var timeRange = new QueryTimeRange(
+                duration: TimeSpan.FromHours(4)
+            );
+
+            Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
+                resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
+                metricNames: new List<string> { "Ingress" },
+                metricNamespace: "Microsoft.Storage/storageAccounts",
+                options: new MetricsQueryResourcesOptions { TimeRange = timeRange }).ConfigureAwait(false);
+
+            Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
+            MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
+            Assert.AreEqual(1, metricsQueryResults.Values.Count);
+            Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
+            Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
+        }
+
+        [Test]
+        [SyncOnly]
+        public void MetricsQueryResourcesInvalid()
+        {
+            MetricsClient client = CreateMetricsClient();
+
+            Assert.Throws<ArgumentException>(() =>
+                client.QueryResources(
+                resourceIds: new List<ResourceIdentifier>(),
+                metricNames: new List<string> { "Ingress" },
+                metricNamespace: "Microsoft.Storage/storageAccounts"));
+        }
+    }
+}
diff --git a/sdk/monitor/Azure.Monitor.Query/tests/MetricsQueryClientLiveTests.cs b/sdk/monitor/Azure.Monitor.Query/tests/MetricsQueryClientLiveTests.cs
@@ -4,13 +4,10 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
-using System.Security.AccessControl;
 using System.Threading.Tasks;
-using Azure.Core;
 using Azure.Core.TestFramework;
 using Azure.Monitor.Query.Models;
 using NUnit.Framework;
-using NUnit.Framework.Internal.Commands;
 
 namespace Azure.Monitor.Query.Tests
 {
@@ -34,18 +31,6 @@ private MetricsQueryClient CreateClient()
             ));
         }
 
-        private MetricsClient CreateMetricsClient()
-        {
-            return InstrumentClient(new MetricsClient(
-                new Uri(TestEnvironment.ConstructMetricsClientUri()),
-                TestEnvironment.Credential,
-                InstrumentClientOptions(new MetricsClientOptions()
-                {
-                    Audience = TestEnvironment.GetMetricsClientAudience()
-                })
-            ));
-        }
-
         [SetUp]
         public void SetUp()
         {
@@ -327,46 +312,5 @@ public async Task CanGetMetricByNameInvalid()
 
             Assert.Throws<KeyNotFoundException>(() => { results.Value.GetMetricByName("Guinness"); });
         }
-
-        [RecordedTest]
-        public async Task MetricsQueryResourcesAsync()
-        {
-            MetricsClient client = CreateMetricsClient();
-
-            var resourceId = TestEnvironment.StorageAccountId;
-
-            Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
-                resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
-                metricNames: new List<string> { "Ingress" },
-                metricNamespace: "Microsoft.Storage/storageAccounts").ConfigureAwait(false);
-
-            MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
-            Assert.AreEqual(1, metricsQueryResults.Values.Count);
-            Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
-            Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
-            for (int i = 0; i < metricsQueryResults.Values.Count; i++)
-            {
-                foreach (MetricResult value in metricsQueryResults.Values[i].Metrics)
-                {
-                    for (int j = 0; j < value.TimeSeries.Count; j++)
-                    {
-                        Assert.GreaterOrEqual(value.TimeSeries[j].Values[i].Total, 0);
-                    }
-                }
-            }
-        }
-
-        [Test]
-        [SyncOnly]
-        public void MetricsQueryResourcesInvalid()
-        {
-            MetricsClient client = CreateMetricsClient();
-
-            Assert.Throws<ArgumentException>(() =>
-                client.QueryResources(
-                resourceIds: new List<ResourceIdentifier>(),
-                metricNames: new List<string> { "Ingress" },
-                metricNamespace: "Microsoft.Storage/storageAccounts"));
-        }
     }
 }