Skip to content

Commit e1469a2

Browse files
wps0wps0
authored
Azure.Monitor.Query: invalid date string format in MetricsClient (Azure#45998)
1 parent 984ecd1 commit e1469a2

File tree

10 files changed

+227
-80
lines changed

10 files changed

+227
-80
lines changed

‎sdk/monitor/Azure.Monitor.Query/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
### Breaking Changes
88

99
### Bugs Fixed
10+
- Fix bug in 'MetricsClient' QueryResourceAsync method where the 'QueryTimeRange' parameter was incorrectly set
1011

1112
### Other Changes
1213

‎sdk/monitor/Azure.Monitor.Query/assets.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
"AssetsRepo": "Azure/azure-sdk-assets",
33
"AssetsRepoPrefixPath": "net",
44
"TagPrefix": "net/monitor/Azure.Monitor.Query",
5-
"Tag": "net/monitor/Azure.Monitor.Query_2eaa6ed059"
5+
"Tag": "net/monitor/Azure.Monitor.Query_26f5d5d32f"
66
}

‎sdk/monitor/Azure.Monitor.Query/src/MetricsClient.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,8 +150,8 @@ private async Task<Response<MetricsQueryResourcesResult>> ExecuteBatchAsync(IEnu
150150
{
151151
if (options.TimeRange != null)
152152
{
153-
startTime = options.TimeRange.Value.Start.ToString();
154-
endTime = options.TimeRange.Value.End.ToString();
153+
startTime = options.TimeRange.Value.Start.ToIsoString();
154+
endTime = options.TimeRange.Value.End.ToIsoString();
155155
}
156156
aggregations = MetricsClientExtensions.CommaJoin(options.Aggregations);
157157
top = options.Size;

‎sdk/monitor/Azure.Monitor.Query/src/MetricsClientExtensions.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Globalization;
67
using System.Linq;
78
using System.Text;
89
using Azure.Monitor.Query.Models;
@@ -51,5 +52,20 @@ internal static IList<string> CommaSplit(string value) =>
5152
new List<string>() :
5253
// TODO: #10600 - Verify we don't need to worry about escaping
5354
new List<string>(value.Split(','));
55+
56+
internal static string ToIsoString(this DateTimeOffset value)
57+
{
58+
if (value.Offset == TimeSpan.Zero)
59+
{
60+
// Some Azure service required 0-offset dates to be formatted without the
61+
// -00:00 part
62+
const string roundtripZFormat = "yyyy-MM-ddTHH:mm:ss.fffffffZ";
63+
return value.ToString(roundtripZFormat, CultureInfo.InvariantCulture);
64+
}
65+
66+
return value.ToString("O", CultureInfo.InvariantCulture);
67+
}
68+
69+
internal static string ToIsoString(this DateTimeOffset? value) => value?.ToIsoString();
5470
}
5571
}

‎sdk/monitor/Azure.Monitor.Query/src/MetricsQueryResourcesOptions.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,10 @@ namespace Azure.Monitor.Query
1414
public class MetricsQueryResourcesOptions
1515
{
1616
/// <summary>
17-
/// Gets or sets the timespan over which the metric will be queried.
17+
/// 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.
1818
/// </summary>
1919
[CodeGenMember("TimeSpan")]
20+
// TODO: https://github.com/Azure/azure-sdk-for-net/issues/46454
2021
public QueryTimeRange? TimeRange { get; set; }
2122

2223
/// <summary>

‎sdk/monitor/Azure.Monitor.Query/src/QueryTimeRange.cs‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the MIT License.
33

44
using System;
5-
using System.Globalization;
65
using System.Xml;
76
using Azure.Core;
87

@@ -105,21 +104,8 @@ public override string ToString()
105104

106105
internal string ToIsoString()
107106
{
108-
string ToString(DateTimeOffset value)
109-
{
110-
if (value.Offset == TimeSpan.Zero)
111-
{
112-
// Some Azure service required 0-offset dates to be formatted without the
113-
// -00:00 part
114-
const string roundtripZFormat = "yyyy-MM-ddTHH:mm:ss.fffffffZ";
115-
return value.ToString(roundtripZFormat, CultureInfo.InvariantCulture);
116-
}
117-
118-
return value.ToString("O", CultureInfo.InvariantCulture);
119-
}
120-
121-
var startTime = Start != null ? ToString(Start.Value) : null;
122-
var endTime = End != null ? ToString(End.Value) : null;
107+
var startTime = Start.ToIsoString();
108+
var endTime = End.ToIsoString();
123109
var duration = XmlConvert.ToString(Duration);
124110

125111
switch (startTime, endTime, duration)

‎sdk/monitor/Azure.Monitor.Query/tests/LogsQueryClientLiveTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,7 @@ public async Task CanSetServiceTimeout()
707707
// or a partial failure 200 response
708708
if (exception.Status == 200)
709709
{
710-
StringAssert.Contains("Query cancelled by the user's request", exception.Message);
710+
StringAssert.Contains("PartialError", exception.Message);
711711
}
712712
else
713713
{

‎sdk/monitor/Azure.Monitor.Query/tests/MetricsClientLiveTests.cs‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using System.Threading.Tasks;
7+
using Azure.Core;
8+
using Azure.Core.TestFramework;
9+
using Azure.Monitor.Query.Models;
10+
using NUnit.Framework;
11+
12+
namespace Azure.Monitor.Query.Tests
13+
{
14+
public class MetricsClientLiveTests : RecordedTestBase<MonitorQueryTestEnvironment>
15+
{
16+
private MetricsTestData _testData;
17+
18+
public MetricsClientLiveTests(bool isAsync) : base(isAsync)
19+
{
20+
}
21+
22+
private MetricsClient CreateMetricsClient()
23+
{
24+
return InstrumentClient(new MetricsClient(
25+
new Uri(TestEnvironment.ConstructMetricsClientUri()),
26+
TestEnvironment.Credential,
27+
InstrumentClientOptions(new MetricsClientOptions()
28+
{
29+
Audience = TestEnvironment.GetMetricsClientAudience()
30+
})
31+
));
32+
}
33+
34+
[SetUp]
35+
public void SetUp()
36+
{
37+
_testData = new MetricsTestData(TestEnvironment, Recording.UtcNow);
38+
}
39+
40+
[RecordedTest]
41+
public async Task MetricsQueryResourcesAsync()
42+
{
43+
MetricsClient client = CreateMetricsClient();
44+
45+
var resourceId = TestEnvironment.StorageAccountId;
46+
47+
Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
48+
resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
49+
metricNames: new List<string> { "Ingress" },
50+
metricNamespace: "Microsoft.Storage/storageAccounts").ConfigureAwait(false);
51+
52+
Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
53+
MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
54+
Assert.AreEqual(1, metricsQueryResults.Values.Count);
55+
Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
56+
Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
57+
for (int i = 0; i < metricsQueryResults.Values.Count; i++)
58+
{
59+
foreach (MetricResult value in metricsQueryResults.Values[i].Metrics)
60+
{
61+
for (int j = 0; j < value.TimeSeries.Count; j++)
62+
{
63+
Assert.GreaterOrEqual(value.TimeSeries[j].Values[i].Total, 0);
64+
}
65+
}
66+
}
67+
}
68+
69+
[RecordedTest]
70+
public async Task MetricsQueryResourcesWithStartEndTimeRangeAsync()
71+
{
72+
MetricsClient client = CreateMetricsClient();
73+
74+
var resourceId = TestEnvironment.StorageAccountId;
75+
76+
var timeRange = new QueryTimeRange(
77+
start: Recording.UtcNow.Subtract(TimeSpan.FromHours(4)),
78+
end: Recording.UtcNow
79+
);
80+
81+
Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
82+
resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
83+
metricNames: new List<string> { "Ingress" },
84+
metricNamespace: "Microsoft.Storage/storageAccounts",
85+
options: new MetricsQueryResourcesOptions { TimeRange = timeRange} ).ConfigureAwait(false);
86+
87+
Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
88+
MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
89+
Assert.AreEqual(1, metricsQueryResults.Values.Count);
90+
Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
91+
Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
92+
}
93+
94+
[RecordedTest]
95+
public async Task MetricsQueryResourcesWithStartDurationTimeRangeAsync()
96+
{
97+
MetricsClient client = CreateMetricsClient();
98+
99+
var resourceId = TestEnvironment.StorageAccountId;
100+
101+
var timeRange = new QueryTimeRange(
102+
start: Recording.UtcNow.Subtract(TimeSpan.FromHours(4)),
103+
duration: TimeSpan.FromHours(4)
104+
);
105+
106+
Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
107+
resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
108+
metricNames: new List<string> { "Ingress" },
109+
metricNamespace: "Microsoft.Storage/storageAccounts",
110+
options: new MetricsQueryResourcesOptions { TimeRange = timeRange }).ConfigureAwait(false);
111+
112+
Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
113+
MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
114+
Assert.AreEqual(1, metricsQueryResults.Values.Count);
115+
Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
116+
Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
117+
}
118+
119+
[RecordedTest]
120+
[SyncOnly]
121+
public void MetricsQueryResourcesWithEndDurationTimeRange()
122+
{
123+
MetricsClient client = CreateMetricsClient();
124+
125+
var resourceId = TestEnvironment.StorageAccountId;
126+
127+
var timeRange = new QueryTimeRange(
128+
end: Recording.UtcNow,
129+
duration: TimeSpan.FromHours(4)
130+
);
131+
132+
Assert.Throws<AggregateException>(() =>
133+
client.QueryResources(
134+
resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
135+
metricNames: new List<string> { "Ingress" },
136+
metricNamespace: "Microsoft.Storage/storageAccounts",
137+
options: new MetricsQueryResourcesOptions { TimeRange = timeRange }));
138+
}
139+
140+
[RecordedTest]
141+
public async Task MetricsQueryResourcesWithDurationTimeRangeAsync()
142+
{
143+
MetricsClient client = CreateMetricsClient();
144+
145+
var resourceId = TestEnvironment.StorageAccountId;
146+
147+
var timeRange = new QueryTimeRange(
148+
duration: TimeSpan.FromHours(4)
149+
);
150+
151+
Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
152+
resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
153+
metricNames: new List<string> { "Ingress" },
154+
metricNamespace: "Microsoft.Storage/storageAccounts",
155+
options: new MetricsQueryResourcesOptions { TimeRange = timeRange }).ConfigureAwait(false);
156+
157+
Assert.AreEqual(200, metricsResultsResponse.GetRawResponse().Status);
158+
MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
159+
Assert.AreEqual(1, metricsQueryResults.Values.Count);
160+
Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
161+
Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
162+
}
163+
164+
[Test]
165+
[SyncOnly]
166+
public void MetricsQueryResourcesInvalid()
167+
{
168+
MetricsClient client = CreateMetricsClient();
169+
170+
Assert.Throws<ArgumentException>(() =>
171+
client.QueryResources(
172+
resourceIds: new List<ResourceIdentifier>(),
173+
metricNames: new List<string> { "Ingress" },
174+
metricNamespace: "Microsoft.Storage/storageAccounts"));
175+
}
176+
}
177+
}

‎sdk/monitor/Azure.Monitor.Query/tests/MetricsQueryClientLiveTests.cs‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,10 @@
44
using System;
55
using System.Collections.Generic;
66
using System.Linq;
7-
using System.Security.AccessControl;
87
using System.Threading.Tasks;
9-
using Azure.Core;
108
using Azure.Core.TestFramework;
119
using Azure.Monitor.Query.Models;
1210
using NUnit.Framework;
13-
using NUnit.Framework.Internal.Commands;
1411

1512
namespace Azure.Monitor.Query.Tests
1613
{
@@ -34,18 +31,6 @@ private MetricsQueryClient CreateClient()
3431
));
3532
}
3633

37-
private MetricsClient CreateMetricsClient()
38-
{
39-
return InstrumentClient(new MetricsClient(
40-
new Uri(TestEnvironment.ConstructMetricsClientUri()),
41-
TestEnvironment.Credential,
42-
InstrumentClientOptions(new MetricsClientOptions()
43-
{
44-
Audience = TestEnvironment.GetMetricsClientAudience()
45-
})
46-
));
47-
}
48-
4934
[SetUp]
5035
public void SetUp()
5136
{
@@ -327,46 +312,5 @@ public async Task CanGetMetricByNameInvalid()
327312

328313
Assert.Throws<KeyNotFoundException>(() => { results.Value.GetMetricByName("Guinness"); });
329314
}
330-
331-
[RecordedTest]
332-
public async Task MetricsQueryResourcesAsync()
333-
{
334-
MetricsClient client = CreateMetricsClient();
335-
336-
var resourceId = TestEnvironment.StorageAccountId;
337-
338-
Response<MetricsQueryResourcesResult> metricsResultsResponse = await client.QueryResourcesAsync(
339-
resourceIds: new List<ResourceIdentifier> { new ResourceIdentifier(resourceId) },
340-
metricNames: new List<string> { "Ingress" },
341-
metricNamespace: "Microsoft.Storage/storageAccounts").ConfigureAwait(false);
342-
343-
MetricsQueryResourcesResult metricsQueryResults = metricsResultsResponse.Value;
344-
Assert.AreEqual(1, metricsQueryResults.Values.Count);
345-
Assert.AreEqual(TestEnvironment.StorageAccountId + "/providers/Microsoft.Insights/metrics/Ingress", metricsQueryResults.Values[0].Metrics[0].Id);
346-
Assert.AreEqual("Microsoft.Storage/storageAccounts", metricsQueryResults.Values[0].Namespace);
347-
for (int i = 0; i < metricsQueryResults.Values.Count; i++)
348-
{
349-
foreach (MetricResult value in metricsQueryResults.Values[i].Metrics)
350-
{
351-
for (int j = 0; j < value.TimeSeries.Count; j++)
352-
{
353-
Assert.GreaterOrEqual(value.TimeSeries[j].Values[i].Total, 0);
354-
}
355-
}
356-
}
357-
}
358-
359-
[Test]
360-
[SyncOnly]
361-
public void MetricsQueryResourcesInvalid()
362-
{
363-
MetricsClient client = CreateMetricsClient();
364-
365-
Assert.Throws<ArgumentException>(() =>
366-
client.QueryResources(
367-
resourceIds: new List<ResourceIdentifier>(),
368-
metricNames: new List<string> { "Ingress" },
369-
metricNamespace: "Microsoft.Storage/storageAccounts"));
370-
}
371315
}
372316
}

0 commit comments

Comments
 (0)