Skip to content

Commit e90d6bb

Browse files
Fix request success criteria for 4xx errors (Azure#33617)
* fix request success criteria for 4xx errors * fix tests * apply for http type only * remove unnecessary using * fix failing test
1 parent 4293117 commit e90d6bb

File tree

3 files changed

+38
-67
lines changed

3 files changed

+38
-67
lines changed

sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/src/Customizations/Models/RequestData.cs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,9 @@
55

66
using System.Diagnostics;
77
using System.Globalization;
8-
98
using Azure.Core;
109
using Azure.Monitor.OpenTelemetry.Exporter.Internals;
1110

12-
using OpenTelemetry.Trace;
13-
1411
namespace Azure.Monitor.OpenTelemetry.Exporter.Models
1512
{
1613
internal partial class RequestData
@@ -34,12 +31,21 @@ public RequestData(int version, Activity activity, ref TagEnumerationState monit
3431
Duration = activity.Duration < SchemaConstants.RequestData_Duration_LessThanDays
3532
? activity.Duration.ToString("c", CultureInfo.InvariantCulture)
3633
: SchemaConstants.Duration_MaxValue;
37-
Success = activity.Status != ActivityStatusCode.Error;
3834
ResponseCode = AzMonList.GetTagValue(ref monitorTags.MappedTags, SemanticConventions.AttributeHttpStatusCode)
3935
?.ToString().Truncate(SchemaConstants.RequestData_ResponseCode_MaxLength)
4036
?? "0";
41-
Url = url.Truncate(SchemaConstants.RequestData_Url_MaxLength);
4237

38+
if (monitorTags.activityType == OperationType.Http && int.TryParse(ResponseCode, out int statusCode))
39+
{
40+
bool isSuccessStatusCode = statusCode != 0 && statusCode < 400;
41+
Success = activity.Status != ActivityStatusCode.Error && isSuccessStatusCode;
42+
}
43+
else
44+
{
45+
Success = activity.Status != ActivityStatusCode.Error;
46+
}
47+
48+
Url = url.Truncate(SchemaConstants.RequestData_Url_MaxLength);
4349
Properties = new ChangeTrackingDictionary<string, string>();
4450
Measurements = new ChangeTrackingDictionary<string, double>();
4551

sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/RequestDataTests.cs

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public void ValidateHttpRequestData()
6262
Assert.Equal(httpUrl, requestData.Url);
6363
Assert.Equal("0", requestData.ResponseCode);
6464
Assert.Equal(activity.Duration.ToString("c", CultureInfo.InvariantCulture), requestData.Duration);
65-
Assert.Equal(activity.GetStatus() != Status.Error, requestData.Success);
65+
Assert.False(requestData.Success);
6666
Assert.Null(requestData.Source);
6767
Assert.True(requestData.Properties.Count == 0);
6868
Assert.True(requestData.Measurements.Count == 0);
@@ -90,5 +90,31 @@ public void ValidateHttpRequestDataResponseCode(string httpStatusCode)
9090

9191
Assert.Equal(httpResponseCode, requestData.ResponseCode);
9292
}
93+
94+
[Theory]
95+
[InlineData("200", true)]
96+
[InlineData("400", false)]
97+
[InlineData("500", false)]
98+
[InlineData("0", false)]
99+
public void ValidateHttpRequestSuccess(string httpStatusCode, bool isSuccess)
100+
{
101+
using ActivitySource activitySource = new ActivitySource(ActivitySourceName);
102+
using var activity = activitySource.StartActivity(
103+
ActivityName,
104+
ActivityKind.Server,
105+
parentContext: new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.Recorded),
106+
startTime: DateTime.UtcNow);
107+
108+
var httpResponseCode = httpStatusCode ?? "0";
109+
activity.SetTag(SemanticConventions.AttributeHttpUrl, "https://www.foo.bar/search");
110+
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, httpStatusCode);
111+
112+
var monitorTags = TraceHelper.EnumerateActivityTags(activity);
113+
114+
var requestData = new RequestData(2, activity, ref monitorTags);
115+
116+
Assert.Equal(httpResponseCode, requestData.ResponseCode);
117+
Assert.Equal(isSuccess, requestData.Success);
118+
}
93119
}
94120
}

sdk/monitor/Azure.Monitor.OpenTelemetry.Exporter/tests/Azure.Monitor.OpenTelemetry.Exporter.Tests/StatusTests.cs

Lines changed: 0 additions & 61 deletions
This file was deleted.

0 commit comments

Comments
 (0)