[AspNetCore] [Http] remove Activity Status Description and update unit tests (#5025)

This commit is contained in:
Timothy Mothra 2023-11-08 17:38:55 -08:00 committed by GitHub
parent 4a3c8d36b3
commit f25ff3a4f4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 34 additions and 83 deletions

View File

@ -2,6 +2,12 @@
## Unreleased
* Removed the Activity Status Description that was being set during
exceptions. Activity Status will continue to be reported as `Error`.
This is a **breaking change**. `EnrichWithException` can be leveraged
to restore this behavior.
([#5025](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5025))
* Updated `http.request.method` to match specification guidelines.
* For activity, if the method does not belong to one of the [known
values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values)

View File

@ -435,7 +435,7 @@ internal class HttpInListener : ListenerHandler
activity.RecordException(exc);
}
activity.SetStatus(ActivityStatusCode.Error, exc.Message);
activity.SetStatus(ActivityStatusCode.Error);
try
{

View File

@ -2,6 +2,12 @@
## Unreleased
* Removed the Activity Status Description that was being set during
exceptions. Activity Status will continue to be reported as `Error`.
This is a **breaking change**. `EnrichWithException` can be leveraged
to restore this behavior.
([#5025](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5025))
* Updated `http.request.method` to match specification guidelines.
* For activity, if the method does not belong to one of the [known
values](https://github.com/open-telemetry/semantic-conventions/blob/v1.22.0/docs/http/http-spans.md#:~:text=http.request.method%20has%20the%20following%20list%20of%20well%2Dknown%20values)

View File

@ -353,7 +353,7 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler
if (exc is HttpRequestException)
{
activity.SetStatus(ActivityStatusCode.Error, exc.Message);
activity.SetStatus(ActivityStatusCode.Error);
}
try

View File

@ -230,57 +230,30 @@ internal static class HttpWebRequestActivitySource
}
ActivityStatusCode status;
string exceptionMessage = null;
if (exception is WebException wexc)
if (exception is WebException wexc && wexc.Response is HttpWebResponse response)
{
if (wexc.Response is HttpWebResponse response)
statusCode = response.StatusCode;
if (tracingEmitOldAttributes)
{
statusCode = response.StatusCode;
if (tracingEmitOldAttributes)
{
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode);
}
if (tracingEmitNewAttributes)
{
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode);
}
status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode);
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)statusCode);
}
else
if (tracingEmitNewAttributes)
{
switch (wexc.Status)
{
case WebExceptionStatus.Timeout:
case WebExceptionStatus.RequestCanceled:
status = ActivityStatusCode.Error;
break;
case WebExceptionStatus.SendFailure:
case WebExceptionStatus.ConnectFailure:
case WebExceptionStatus.SecureChannelFailure:
case WebExceptionStatus.TrustFailure:
case WebExceptionStatus.ServerProtocolViolation:
case WebExceptionStatus.MessageLengthLimitExceeded:
status = ActivityStatusCode.Error;
exceptionMessage = exception.Message;
break;
default:
status = ActivityStatusCode.Error;
exceptionMessage = exception.Message;
break;
}
activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, (int)statusCode);
}
status = SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, (int)statusCode);
}
else
{
status = ActivityStatusCode.Error;
exceptionMessage = exception.Message;
}
activity.SetStatus(status, exceptionMessage);
activity.SetStatus(status);
if (TracingOptions.RecordException)
{
activity.RecordException(exception);

View File

@ -131,14 +131,7 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests
// Instrumentation is not expected to set status description
// as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode
if (!urlPath.EndsWith("exception"))
{
Assert.True(string.IsNullOrEmpty(activity.StatusDescription));
}
else
{
Assert.Equal("exception description", activity.StatusDescription);
}
Assert.Null(activity.StatusDescription);
if (recordException)
{

View File

@ -138,14 +138,7 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_Dupe
// Instrumentation is not expected to set status description
// as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode
if (!urlPath.EndsWith("exception"))
{
Assert.True(string.IsNullOrEmpty(activity.StatusDescription));
}
else
{
Assert.Equal("exception description", activity.StatusDescription);
}
Assert.Null(activity.StatusDescription);
if (recordException)
{

View File

@ -132,14 +132,7 @@ public class IncomingRequestsCollectionsIsAccordingToTheSpecTests_New
// Instrumentation is not expected to set status description
// as the reason can be inferred from SemanticConventions.AttributeHttpStatusCode
if (!urlPath.EndsWith("exception"))
{
Assert.True(string.IsNullOrEmpty(activity.StatusDescription));
}
else
{
Assert.Equal("exception description", activity.StatusDescription);
}
Assert.Null(activity.StatusDescription);
if (recordException)
{

View File

@ -337,12 +337,7 @@ public partial class HttpClientTests
// Assert.Equal(tc.SpanStatus, d[span.Status.CanonicalCode]);
Assert.Equal(tc.SpanStatus, activity.Status.ToString());
if (tc.SpanStatusHasDescription.HasValue)
{
var desc = activity.StatusDescription;
Assert.Equal(tc.SpanStatusHasDescription.Value, !string.IsNullOrEmpty(desc));
}
Assert.Null(activity.StatusDescription);
var normalizedAttributes = activity.TagObjects.Where(kv => !kv.Key.StartsWith("otel.")).ToDictionary(x => x.Key, x => x.Value.ToString());

View File

@ -72,8 +72,6 @@ public static class HttpTestData
public string SpanStatus { get; set; }
public bool? SpanStatusHasDescription { get; set; }
public Dictionary<string, string> SpanAttributes { get; set; }
}
}

View File

@ -549,7 +549,7 @@ public class HttpWebRequestActivitySourceTests : IDisposable
Assert.Equal("Stop", exceptionEvent.Key);
Assert.True(activity.Status != ActivityStatusCode.Unset);
Assert.NotNull(activity.StatusDescription);
Assert.Null(activity.StatusDescription);
}
/// <summary>
@ -627,7 +627,7 @@ public class HttpWebRequestActivitySourceTests : IDisposable
Assert.Equal("Stop", exceptionEvent.Key);
Assert.True(exceptionEvent.Value.Status != ActivityStatusCode.Unset);
Assert.NotNull(exceptionEvent.Value.StatusDescription);
Assert.Null(exceptionEvent.Value.StatusDescription);
}
/// <summary>
@ -669,7 +669,7 @@ public class HttpWebRequestActivitySourceTests : IDisposable
Assert.Equal("Stop", exceptionEvent.Key);
Assert.True(exceptionEvent.Value.Status != ActivityStatusCode.Unset);
Assert.NotNull(exceptionEvent.Value.StatusDescription);
Assert.Null(exceptionEvent.Value.StatusDescription);
}
[Fact]

View File

@ -123,11 +123,7 @@ public partial class HttpWebRequestTests
if (tag.Key == SpanAttributeConstants.StatusDescriptionKey)
{
if (tc.SpanStatusHasDescription.HasValue)
{
Assert.Equal(tc.SpanStatusHasDescription.Value, !string.IsNullOrEmpty(tagValue));
}
Assert.Null(tagValue);
continue;
}

View File

@ -93,7 +93,6 @@
"url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/",
"spanName": "HTTP GET",
"spanStatus": "Error",
"spanStatusHasDescription": true,
"responseExpected": false,
"recordException": false,
"spanAttributes": {
@ -111,7 +110,6 @@
"url": "http://sdlfaldfjalkdfjlkajdflkajlsdjf:{port}/",
"spanName": "HTTP GET",
"spanStatus": "Error",
"spanStatusHasDescription": true,
"responseExpected": false,
"recordException": true,
"spanAttributes": {