From e7f2c2f67391aabbe656db036bb8becb229110b0 Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Fri, 22 Jul 2022 17:02:55 -0300 Subject: [PATCH 01/10] Fix issue where when an application has an ExceptionFilter, the exception data wouldn't be collected --- .../Implementation/HttpInListener.cs | 58 +++++++++++++------ src/OpenTelemetry/CHANGELOG.md | 4 ++ .../PropertyFetcher.cs | 22 ++++++- 3 files changed, 63 insertions(+), 21 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 1ff80acc8b2..0faee5c509c 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -45,6 +45,7 @@ internal class HttpInListener : ListenerHandler private readonly PropertyFetcher startContextFetcher = new("HttpContext"); private readonly PropertyFetcher stopContextFetcher = new("HttpContext"); private readonly PropertyFetcher stopExceptionFetcher = new("Exception"); + private readonly PropertyFetcher stopExceptionFilterFetcher = new("ExceptionContext.Exception"); private readonly PropertyFetcher beforeActionActionDescriptorFetcher = new("actionDescriptor"); private readonly PropertyFetcher beforeActionAttributeRouteInfoFetcher = new("AttributeRouteInfo"); private readonly PropertyFetcher beforeActionTemplateFetcher = new("Template"); @@ -273,30 +274,49 @@ public override void OnCustom(string name, Activity activity, object payload) public override void OnException(Activity activity, object payload) { - if (activity.IsAllDataRequested) + if (!activity.IsAllDataRequested) { - if (!this.stopExceptionFetcher.TryFetch(payload, out Exception exc) || exc == null) - { - AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnException)); - return; - } + return; + } - if (this.options.RecordException) - { - activity.RecordException(exc); - } + var exc = this.TryFetchException(payload); + if (exc == null) + { + AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnException)); + return; + } - activity.SetStatus(Status.Error.WithDescription(exc.Message)); + if (this.options.RecordException) + { + activity.RecordException(exc); + } - try - { - this.options.Enrich?.Invoke(activity, "OnException", exc); - } - catch (Exception ex) - { - AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex); - } + activity.SetStatus(Status.Error.WithDescription(exc.Message)); + + try + { + this.options.Enrich?.Invoke(activity, "OnException", exc); + } + catch (Exception ex) + { + AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex); + } + } + + private Exception TryFetchException(object payload) + { + Exception exc; + if (this.stopExceptionFetcher.TryFetch(payload, out exc) && exc != null) + { + return exc; } + + if (this.stopExceptionFilterFetcher.TryFetch(payload, out exc) && exc != null) + { + return exc; + } + + return null; } private static string GetUri(HttpRequest request) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index e006a6427cd..b97c9674bb3 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +* Fix issue where when an application has an ExceptionFilter, the exception data + wouldn't be collected. + (TODO) + * `TracerProviderSDK` modified for spans with remote parent. For such spans activity will be created irrespective of SamplingResult, to maintain context propagation. diff --git a/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs b/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs index d451e9b8de9..4d1c8fe8342 100644 --- a/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs +++ b/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs @@ -71,12 +71,30 @@ public bool TryFetch(object obj, out T value, bool skipObjNullCheck = false) return false; } + object cur = obj; if (this.innerFetcher == null) { - this.innerFetcher = PropertyFetch.Create(obj.GetType().GetTypeInfo(), this.propertyName); + if (this.propertyName.Contains(".")) + { + var parts = this.propertyName.Split('.'); + for (var i = 0; i < parts.Length; i++) + { + this.innerFetcher = PropertyFetch.Create(cur.GetType().GetTypeInfo(), parts[i]); + if (i == parts.Length - 1) + { + break; + } + + cur = cur.GetType().GetProperty(parts[i]).GetValue(cur); + } + } + else + { + this.innerFetcher = PropertyFetch.Create(obj.GetType().GetTypeInfo(), this.propertyName); + } } - return this.innerFetcher.TryFetch(obj, out value); + return this.innerFetcher.TryFetch(cur, out value); } // see https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceEventSource.cs From 1eeb73acfea720709876e81c9e208b6c537cee8e Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Fri, 22 Jul 2022 17:05:55 -0300 Subject: [PATCH 02/10] changelog --- src/OpenTelemetry/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index b97c9674bb3..1c3c2cd388e 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -4,7 +4,7 @@ * Fix issue where when an application has an ExceptionFilter, the exception data wouldn't be collected. - (TODO) + ([#3475](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3475)) * `TracerProviderSDK` modified for spans with remote parent. For such spans activity will be created irrespective of SamplingResult, to maintain context From 912f080a0dec12cd36085275cce0f5a6812e0e33 Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Sat, 23 Jul 2022 16:17:33 -0700 Subject: [PATCH 03/10] undo changes --- .../Implementation/HttpInListener.cs | 42 +++++++++---------- .../PropertyFetcher.cs | 22 +--------- 2 files changed, 21 insertions(+), 43 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index 0faee5c509c..c6e6311d480 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -45,7 +45,6 @@ internal class HttpInListener : ListenerHandler private readonly PropertyFetcher startContextFetcher = new("HttpContext"); private readonly PropertyFetcher stopContextFetcher = new("HttpContext"); private readonly PropertyFetcher stopExceptionFetcher = new("Exception"); - private readonly PropertyFetcher stopExceptionFilterFetcher = new("ExceptionContext.Exception"); private readonly PropertyFetcher beforeActionActionDescriptorFetcher = new("actionDescriptor"); private readonly PropertyFetcher beforeActionAttributeRouteInfoFetcher = new("AttributeRouteInfo"); private readonly PropertyFetcher beforeActionTemplateFetcher = new("Template"); @@ -274,32 +273,29 @@ public override void OnCustom(string name, Activity activity, object payload) public override void OnException(Activity activity, object payload) { - if (!activity.IsAllDataRequested) - { - return; - } - - var exc = this.TryFetchException(payload); - if (exc == null) + if (activity.IsAllDataRequested) { - AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnException)); - return; - } + if (!this.stopExceptionFetcher.TryFetch(payload, out Exception exc) || exc == null) + { + AspNetCoreInstrumentationEventSource.Log.NullPayload(nameof(HttpInListener), nameof(this.OnException)); + return; + } - if (this.options.RecordException) - { - activity.RecordException(exc); - } + if (this.options.RecordException) + { + activity.RecordException(exc); + } - activity.SetStatus(Status.Error.WithDescription(exc.Message)); + activity.SetStatus(Status.Error.WithDescription(exc.Message)); - try - { - this.options.Enrich?.Invoke(activity, "OnException", exc); - } - catch (Exception ex) - { - AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex); + try + { + this.options.Enrich?.Invoke(activity, "OnException", exc); + } + catch (Exception ex) + { + AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex); + } } } diff --git a/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs b/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs index 4d1c8fe8342..d451e9b8de9 100644 --- a/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs +++ b/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs @@ -71,30 +71,12 @@ public bool TryFetch(object obj, out T value, bool skipObjNullCheck = false) return false; } - object cur = obj; if (this.innerFetcher == null) { - if (this.propertyName.Contains(".")) - { - var parts = this.propertyName.Split('.'); - for (var i = 0; i < parts.Length; i++) - { - this.innerFetcher = PropertyFetch.Create(cur.GetType().GetTypeInfo(), parts[i]); - if (i == parts.Length - 1) - { - break; - } - - cur = cur.GetType().GetProperty(parts[i]).GetValue(cur); - } - } - else - { - this.innerFetcher = PropertyFetch.Create(obj.GetType().GetTypeInfo(), this.propertyName); - } + this.innerFetcher = PropertyFetch.Create(obj.GetType().GetTypeInfo(), this.propertyName); } - return this.innerFetcher.TryFetch(cur, out value); + return this.innerFetcher.TryFetch(obj, out value); } // see https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/DiagnosticSourceEventSource.cs From d2604902bdab51f96d17039c7958ed911df76bfb Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Sat, 23 Jul 2022 17:38:21 -0700 Subject: [PATCH 04/10] since the first iteration of this change, the understanding of the issue has changed. this commit reflects that --- .../Implementation/HttpInListener.cs | 16 ---------------- .../PropertyFetcher.cs | 16 ++++++++++++++-- .../TestApp.AspNetCore.6.0.csproj | 2 +- 3 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs index c6e6311d480..1ff80acc8b2 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs @@ -299,22 +299,6 @@ public override void OnException(Activity activity, object payload) } } - private Exception TryFetchException(object payload) - { - Exception exc; - if (this.stopExceptionFetcher.TryFetch(payload, out exc) && exc != null) - { - return exc; - } - - if (this.stopExceptionFilterFetcher.TryFetch(payload, out exc) && exc != null) - { - return exc; - } - - return null; - } - private static string GetUri(HttpRequest request) { // this follows the suggestions from https://github.com/dotnet/aspnetcore/issues/28906 diff --git a/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs b/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs index d451e9b8de9..d2ec900b051 100644 --- a/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs +++ b/src/OpenTelemetry/DiagnosticSourceInstrumentation/PropertyFetcher.cs @@ -76,6 +76,12 @@ public bool TryFetch(object obj, out T value, bool skipObjNullCheck = false) this.innerFetcher = PropertyFetch.Create(obj.GetType().GetTypeInfo(), this.propertyName); } + if (this.innerFetcher == null) + { + value = default; + return false; + } + return this.innerFetcher.TryFetch(obj, out value); } @@ -96,8 +102,8 @@ static PropertyFetch CreateFetcherForProperty(PropertyInfo propertyInfo) { if (propertyInfo == null || !typeof(T).IsAssignableFrom(propertyInfo.PropertyType)) { - // returns null on any fetch. - return new PropertyFetch(); + // returns null and wait for a valid payload to arrive. + return null; } var typedPropertyFetcher = typeof(TypedPropertyFetch<,>); @@ -137,6 +143,12 @@ public override bool TryFetch(object obj, out T value) this.innerFetcher ??= Create(obj.GetType().GetTypeInfo(), this.propertyName); + if (this.innerFetcher == null) + { + value = default; + return false; + } + return this.innerFetcher.TryFetch(obj, out value); } } diff --git a/test/TestApp.AspNetCore.6.0/TestApp.AspNetCore.6.0.csproj b/test/TestApp.AspNetCore.6.0/TestApp.AspNetCore.6.0.csproj index e7095cbd18c..2be7b0f7b4e 100644 --- a/test/TestApp.AspNetCore.6.0/TestApp.AspNetCore.6.0.csproj +++ b/test/TestApp.AspNetCore.6.0/TestApp.AspNetCore.6.0.csproj @@ -1,4 +1,4 @@ - + net6.0 From 2edb213fa82c0543bf9989cd1df557cfe81acb6e Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Sat, 23 Jul 2022 17:40:46 -0700 Subject: [PATCH 05/10] add tests --- .../ExceptionFilterTests.cs | 185 ++++++++++++++++++ .../Controllers/ErrorController.cs | 31 +++ .../Filters/ExceptionFilter1.cs | 29 +++ .../Filters/ExceptionFilter2.cs | 28 +++ .../Controllers/ErrorController.cs | 31 +++ .../Filters/ExceptionFilter1.cs | 29 +++ .../Filters/ExceptionFilter2.cs | 28 +++ 7 files changed, 361 insertions(+) create mode 100644 test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs create mode 100644 test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs create mode 100644 test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs create mode 100644 test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs create mode 100644 test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs create mode 100644 test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs create mode 100644 test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs new file mode 100644 index 00000000000..de9ef6c81f3 --- /dev/null +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs @@ -0,0 +1,185 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Newtonsoft.Json; +using OpenTelemetry.Context.Propagation; +using OpenTelemetry.Instrumentation.AspNetCore.Implementation; +using OpenTelemetry.Tests; +using OpenTelemetry.Trace; + +#if NETCOREAPP3_1 +using TestApp.AspNetCore._3._1; +using TestApp.AspNetCore._3._1.Filters; +#endif +#if NET6_0 +using TestApp.AspNetCore._6._0; +using TestApp.AspNetCore._6._0.Filters; +#endif +using Xunit; + +namespace OpenTelemetry.Instrumentation.AspNetCore.Tests +{ + public sealed class ExceptionFilterTests + : IClassFixture>, IDisposable + { + private readonly WebApplicationFactory factory; + private TracerProvider tracerProvider = null; + + public ExceptionFilterTests(WebApplicationFactory factory) + { + this.factory = factory; + } + + [Fact] + public async Task ShouldExportExceptionWithNoExceptionFilters() + { + var exportedItems = new List(); + + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => builder.ConfigureTestServices( + (s) => ConfigureTestServices(s, 0, ref exportedItems))) + .CreateClient()) + { + // Act + var response = await client.GetAsync("/api/error"); + + WaitForActivityExport(exportedItems, 1); + } + + // Assert + AssertException(exportedItems); + } + + [Fact] + public async Task ShouldExportActivityWithOneExceptionFilter() + { + var exportedItems = new List(); + + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => builder.ConfigureTestServices( + (s) => ConfigureTestServices(s, 1, ref exportedItems))) + .CreateClient()) + { + // Act + var response = await client.GetAsync("/api/error"); + + WaitForActivityExport(exportedItems, 1); + } + + // Assert + AssertException(exportedItems); + } + + [Fact] + public async Task ShouldExportSingleActivityEvenWithTwoExceptionFilters() + { + var exportedItems = new List(); + + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => builder.ConfigureTestServices( + (s) => ConfigureTestServices(s, 2, ref exportedItems))) + .CreateClient()) + { + // Act + var response = await client.GetAsync("/api/error"); + + WaitForActivityExport(exportedItems, 1); + } + + // Assert + AssertException(exportedItems); + } + + public void Dispose() + { + this.tracerProvider?.Dispose(); + } + + private static void WaitForActivityExport(List exportedItems, int count) + { + // We need to let End callback execute as it is executed AFTER response was returned. + // In unit tests environment there may be a lot of parallel unit tests executed, so + // giving some breezing room for the End callback to complete + Assert.True(SpinWait.SpinUntil( + () => + { + Thread.Sleep(10); + return exportedItems.Count >= count; + }, + TimeSpan.FromSeconds(1))); + } + + private static void ValidateAspNetCoreActivity(Activity activityToValidate, string expectedHttpPath) + { + Assert.Equal(ActivityKind.Server, activityToValidate.Kind); + Assert.Equal(HttpInListener.ActivitySourceName, activityToValidate.Source.Name); + Assert.Equal(HttpInListener.Version.ToString(), activityToValidate.Source.Version); + Assert.Equal(expectedHttpPath, activityToValidate.GetTagValue(SemanticConventions.AttributeHttpTarget) as string); + } + + private void ConfigureTestServices(IServiceCollection services, int mode, ref List exportedItems) + { + switch (mode) + { + case 1: + services.AddMvc(x => x.Filters.Add()); + break; + case 2: + services.AddMvc(x => x.Filters.Add()); + services.AddMvc(x => x.Filters.Add()); + break; + default: + break; + } + + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddAspNetCoreInstrumentation(x => x.RecordException = true) + .AddInMemoryExporter(exportedItems) + .Build(); + } + + private void AssertException(List exportedItems) + { + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + var exMessage = "something's wrong!"; + Assert.Equal("System.Exception", activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType).Value); + Assert.Equal(exMessage, activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionMessage).Value); + + var status = activity.GetStatus(); + Assert.Equal(status, Status.Error.WithDescription(exMessage)); + + ValidateAspNetCoreActivity(activity, "/api/error"); + } + } +} diff --git a/test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs b/test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs new file mode 100644 index 00000000000..b51ba4e8687 --- /dev/null +++ b/test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs @@ -0,0 +1,31 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +using System; +using Microsoft.AspNetCore.Mvc; + +namespace TestApp.AspNetCore._3._1.Controllers +{ + [Route("api/[controller]")] + public class ErrorController : Controller + { + // GET api/error + [HttpGet] + public string Get() + { + throw new Exception("something's wrong!"); + } + } +} diff --git a/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs b/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs new file mode 100644 index 00000000000..44bf59e1cbe --- /dev/null +++ b/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs @@ -0,0 +1,29 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.AspNetCore.Mvc.Filters; + +namespace TestApp.AspNetCore._3._1.Filters +{ + public class ExceptionFilter1 : IExceptionFilter + { + public void OnException(ExceptionContext context) + { + // test if the changes that were made to fix the behaviour where + // an application with a defined ExceptionFilter wouldn't collect those + } + } +} diff --git a/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs b/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs new file mode 100644 index 00000000000..d87bc993777 --- /dev/null +++ b/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs @@ -0,0 +1,28 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.AspNetCore.Mvc.Filters; + +namespace TestApp.AspNetCore._3._1.Filters +{ + public class ExceptionFilter2 : IExceptionFilter + { + public void OnException(ExceptionContext context) + { + // test the behaviour when an application has two ExceptionFilters defined + } + } +} diff --git a/test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs b/test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs new file mode 100644 index 00000000000..6e00f2455f6 --- /dev/null +++ b/test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs @@ -0,0 +1,31 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +using System; +using Microsoft.AspNetCore.Mvc; + +namespace TestApp.AspNetCore._6._0.Controllers +{ + [Route("api/[controller]")] + public class ErrorController : Controller + { + // GET api/error + [HttpGet] + public string Get() + { + throw new Exception("something's wrong!"); + } + } +} diff --git a/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs b/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs new file mode 100644 index 00000000000..31ebf4debd3 --- /dev/null +++ b/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs @@ -0,0 +1,29 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.AspNetCore.Mvc.Filters; + +namespace TestApp.AspNetCore._6._0.Filters +{ + public class ExceptionFilter1 : IExceptionFilter + { + public void OnException(ExceptionContext context) + { + // test if the changes that were made to fix the behaviour where + // an application with a defined ExceptionFilter wouldn't collect those + } + } +} diff --git a/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs b/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs new file mode 100644 index 00000000000..9d7c5fafb57 --- /dev/null +++ b/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs @@ -0,0 +1,28 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.AspNetCore.Mvc.Filters; + +namespace TestApp.AspNetCore._6._0.Filters +{ + public class ExceptionFilter2 : IExceptionFilter + { + public void OnException(ExceptionContext context) + { + // test the behaviour when an application has two ExceptionFilters defined + } + } +} From d5a95435d25e6308c72364c34d08baaa6aec212b Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Mon, 25 Jul 2022 19:13:51 -0700 Subject: [PATCH 06/10] moving tests to BasicTests, adding a test on PropertyFetcherTest.cs and updating changelog --- .../CHANGELOG.md | 4 +- src/OpenTelemetry/CHANGELOG.md | 4 - .../BasicTests.cs | 81 ++++++++ .../ExceptionFilterTests.cs | 185 ------------------ .../Instrumentation/PropertyFetcherTest.cs | 11 ++ 5 files changed, 95 insertions(+), 190 deletions(-) delete mode 100644 test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 3766ca848aa..73652e6076e 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -1,7 +1,9 @@ # Changelog ## Unreleased - +* Fix issue where when an application has an ExceptionFilter, the exception data + wouldn't be collected. + ([#3475](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3475)) * Metrics instrumentation to correctly populate `http.flavor` tag. (1.1 instead of HTTP/1.1 etc.) ([3379](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3379)) diff --git a/src/OpenTelemetry/CHANGELOG.md b/src/OpenTelemetry/CHANGELOG.md index 1c3c2cd388e..e006a6427cd 100644 --- a/src/OpenTelemetry/CHANGELOG.md +++ b/src/OpenTelemetry/CHANGELOG.md @@ -2,10 +2,6 @@ ## Unreleased -* Fix issue where when an application has an ExceptionFilter, the exception data - wouldn't be collected. - ([#3475](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3475)) - * `TracerProviderSDK` modified for spans with remote parent. For such spans activity will be created irrespective of SamplingResult, to maintain context propagation. diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index 03dc2b9d6e3..c52044d0519 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -34,9 +34,11 @@ #if NETCOREAPP3_1 using TestApp.AspNetCore._3._1; +using TestApp.AspNetCore._3._1.Filters; #endif #if NET6_0 using TestApp.AspNetCore._6._0; +using TestApp.AspNetCore._6._0.Filters; #endif using Xunit; @@ -550,6 +552,48 @@ void ConfigureTestServices(IServiceCollection services) Assert.Equal(shouldEnrichBeCalled, enrichCalled); } + [Fact] + public async Task ShouldExportActivityWithOneExceptionFilter() + { + var exportedItems = new List(); + + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => builder.ConfigureTestServices( + (s) => this.ConfigureExceptionFilters(s, 1, ref exportedItems))) + .CreateClient()) + { + // Act + var response = await client.GetAsync("/api/error"); + + WaitForActivityExport(exportedItems, 1); + } + + // Assert + AssertException(exportedItems); + } + + [Fact] + public async Task ShouldExportSingleActivityEvenWithTwoExceptionFilters() + { + var exportedItems = new List(); + + // Arrange + using (var client = this.factory + .WithWebHostBuilder(builder => builder.ConfigureTestServices( + (s) => this.ConfigureExceptionFilters(s, 2, ref exportedItems))) + .CreateClient()) + { + // Act + var response = await client.GetAsync("/api/error"); + + WaitForActivityExport(exportedItems, 1); + } + + // Assert + AssertException(exportedItems); + } + public void Dispose() { this.tracerProvider?.Dispose(); @@ -597,6 +641,43 @@ private static void ActivityEnrichment(Activity activity, string method, object activity.SetTag("enriched", "yes"); } + private void ConfigureExceptionFilters(IServiceCollection services, int mode, ref List exportedItems) + { + switch (mode) + { + case 1: + services.AddMvc(x => x.Filters.Add()); + break; + case 2: + services.AddMvc(x => x.Filters.Add()); + services.AddMvc(x => x.Filters.Add()); + break; + default: + break; + } + + this.tracerProvider = Sdk.CreateTracerProviderBuilder() + .AddAspNetCoreInstrumentation(x => x.RecordException = true) + .AddInMemoryExporter(exportedItems) + .Build(); + } + + private static void AssertException(List exportedItems) + { + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + var exMessage = "something's wrong!"; + Assert.Single(activity.Events); + Assert.Equal("System.Exception", activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType).Value); + Assert.Equal(exMessage, activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionMessage).Value); + + var status = activity.GetStatus(); + Assert.Equal(status, Status.Error.WithDescription(exMessage)); + + ValidateAspNetCoreActivity(activity, "/api/error"); + } + private class ExtractOnlyPropagator : TextMapPropagator { private readonly ActivityContext activityContext; diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs deleted file mode 100644 index de9ef6c81f3..00000000000 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/ExceptionFilterTests.cs +++ /dev/null @@ -1,185 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc.Testing; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.TestHost; -using Microsoft.Extensions.DependencyInjection; -using Moq; -using Newtonsoft.Json; -using OpenTelemetry.Context.Propagation; -using OpenTelemetry.Instrumentation.AspNetCore.Implementation; -using OpenTelemetry.Tests; -using OpenTelemetry.Trace; - -#if NETCOREAPP3_1 -using TestApp.AspNetCore._3._1; -using TestApp.AspNetCore._3._1.Filters; -#endif -#if NET6_0 -using TestApp.AspNetCore._6._0; -using TestApp.AspNetCore._6._0.Filters; -#endif -using Xunit; - -namespace OpenTelemetry.Instrumentation.AspNetCore.Tests -{ - public sealed class ExceptionFilterTests - : IClassFixture>, IDisposable - { - private readonly WebApplicationFactory factory; - private TracerProvider tracerProvider = null; - - public ExceptionFilterTests(WebApplicationFactory factory) - { - this.factory = factory; - } - - [Fact] - public async Task ShouldExportExceptionWithNoExceptionFilters() - { - var exportedItems = new List(); - - // Arrange - using (var client = this.factory - .WithWebHostBuilder(builder => builder.ConfigureTestServices( - (s) => ConfigureTestServices(s, 0, ref exportedItems))) - .CreateClient()) - { - // Act - var response = await client.GetAsync("/api/error"); - - WaitForActivityExport(exportedItems, 1); - } - - // Assert - AssertException(exportedItems); - } - - [Fact] - public async Task ShouldExportActivityWithOneExceptionFilter() - { - var exportedItems = new List(); - - // Arrange - using (var client = this.factory - .WithWebHostBuilder(builder => builder.ConfigureTestServices( - (s) => ConfigureTestServices(s, 1, ref exportedItems))) - .CreateClient()) - { - // Act - var response = await client.GetAsync("/api/error"); - - WaitForActivityExport(exportedItems, 1); - } - - // Assert - AssertException(exportedItems); - } - - [Fact] - public async Task ShouldExportSingleActivityEvenWithTwoExceptionFilters() - { - var exportedItems = new List(); - - // Arrange - using (var client = this.factory - .WithWebHostBuilder(builder => builder.ConfigureTestServices( - (s) => ConfigureTestServices(s, 2, ref exportedItems))) - .CreateClient()) - { - // Act - var response = await client.GetAsync("/api/error"); - - WaitForActivityExport(exportedItems, 1); - } - - // Assert - AssertException(exportedItems); - } - - public void Dispose() - { - this.tracerProvider?.Dispose(); - } - - private static void WaitForActivityExport(List exportedItems, int count) - { - // We need to let End callback execute as it is executed AFTER response was returned. - // In unit tests environment there may be a lot of parallel unit tests executed, so - // giving some breezing room for the End callback to complete - Assert.True(SpinWait.SpinUntil( - () => - { - Thread.Sleep(10); - return exportedItems.Count >= count; - }, - TimeSpan.FromSeconds(1))); - } - - private static void ValidateAspNetCoreActivity(Activity activityToValidate, string expectedHttpPath) - { - Assert.Equal(ActivityKind.Server, activityToValidate.Kind); - Assert.Equal(HttpInListener.ActivitySourceName, activityToValidate.Source.Name); - Assert.Equal(HttpInListener.Version.ToString(), activityToValidate.Source.Version); - Assert.Equal(expectedHttpPath, activityToValidate.GetTagValue(SemanticConventions.AttributeHttpTarget) as string); - } - - private void ConfigureTestServices(IServiceCollection services, int mode, ref List exportedItems) - { - switch (mode) - { - case 1: - services.AddMvc(x => x.Filters.Add()); - break; - case 2: - services.AddMvc(x => x.Filters.Add()); - services.AddMvc(x => x.Filters.Add()); - break; - default: - break; - } - - this.tracerProvider = Sdk.CreateTracerProviderBuilder() - .AddAspNetCoreInstrumentation(x => x.RecordException = true) - .AddInMemoryExporter(exportedItems) - .Build(); - } - - private void AssertException(List exportedItems) - { - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - var exMessage = "something's wrong!"; - Assert.Equal("System.Exception", activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType).Value); - Assert.Equal(exMessage, activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionMessage).Value); - - var status = activity.GetStatus(); - Assert.Equal(status, Status.Error.WithDescription(exMessage)); - - ValidateAspNetCoreActivity(activity, "/api/error"); - } - } -} diff --git a/test/OpenTelemetry.Tests/Instrumentation/PropertyFetcherTest.cs b/test/OpenTelemetry.Tests/Instrumentation/PropertyFetcherTest.cs index 89714dc46e8..f49d61dfbaa 100644 --- a/test/OpenTelemetry.Tests/Instrumentation/PropertyFetcherTest.cs +++ b/test/OpenTelemetry.Tests/Instrumentation/PropertyFetcherTest.cs @@ -67,6 +67,17 @@ public void FetchPropertyMultiplePayloadTypes() Assert.False(fetch.TryFetch(null, out _)); } + [Fact] + public void FetchPropertyMultiplePayloadTypes_IgnoreTypesWithoutExpectedPropertyName() + { + var fetch = new PropertyFetcher("Property"); + + Assert.False(fetch.TryFetch(new PayloadTypeC(), out _)); + + Assert.True(fetch.TryFetch(new PayloadTypeA(), out string propertyValue)); + Assert.Equal("A", propertyValue); + } + private class PayloadTypeA { public string Property { get; set; } = "A"; From c524c31cdbfe500468afae79b1d1d1309ffe885c Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Mon, 25 Jul 2022 16:43:36 -0700 Subject: [PATCH 07/10] fix changelog (markdownlint) and order of methods --- .../CHANGELOG.md | 1 + .../BasicTests.cs | 32 +++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md index 73652e6076e..ff8032eb784 100644 --- a/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md +++ b/src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## Unreleased + * Fix issue where when an application has an ExceptionFilter, the exception data wouldn't be collected. ([#3475](https://github.com/open-telemetry/opentelemetry-dotnet/pull/3475)) diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index c52044d0519..41557a38fb5 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -641,6 +641,22 @@ private static void ActivityEnrichment(Activity activity, string method, object activity.SetTag("enriched", "yes"); } + private static void AssertException(List exportedItems) + { + Assert.Single(exportedItems); + var activity = exportedItems[0]; + + var exMessage = "something's wrong!"; + Assert.Single(activity.Events); + Assert.Equal("System.Exception", activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType).Value); + Assert.Equal(exMessage, activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionMessage).Value); + + var status = activity.GetStatus(); + Assert.Equal(status, Status.Error.WithDescription(exMessage)); + + ValidateAspNetCoreActivity(activity, "/api/error"); + } + private void ConfigureExceptionFilters(IServiceCollection services, int mode, ref List exportedItems) { switch (mode) @@ -662,22 +678,6 @@ private void ConfigureExceptionFilters(IServiceCollection services, int mode, re .Build(); } - private static void AssertException(List exportedItems) - { - Assert.Single(exportedItems); - var activity = exportedItems[0]; - - var exMessage = "something's wrong!"; - Assert.Single(activity.Events); - Assert.Equal("System.Exception", activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType).Value); - Assert.Equal(exMessage, activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionMessage).Value); - - var status = activity.GetStatus(); - Assert.Equal(status, Status.Error.WithDescription(exMessage)); - - ValidateAspNetCoreActivity(activity, "/api/error"); - } - private class ExtractOnlyPropagator : TextMapPropagator { private readonly ActivityContext activityContext; From f2c7f2a7b59c6f202373f4e42a1cbbb1de6b9f99 Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Tue, 23 Aug 2022 17:50:43 -0300 Subject: [PATCH 08/10] add ExceptionFilters on testapps --- .../Filters/ExceptionFilter1.cs | 28 +++++++++++++++++++ .../Filters/ExceptionFilter2.cs | 28 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 test/TestApp.AspNetCore/Filters/ExceptionFilter1.cs create mode 100644 test/TestApp.AspNetCore/Filters/ExceptionFilter2.cs diff --git a/test/TestApp.AspNetCore/Filters/ExceptionFilter1.cs b/test/TestApp.AspNetCore/Filters/ExceptionFilter1.cs new file mode 100644 index 00000000000..74a597a06ad --- /dev/null +++ b/test/TestApp.AspNetCore/Filters/ExceptionFilter1.cs @@ -0,0 +1,28 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.AspNetCore.Mvc.Filters; + +namespace TestApp.AspNetCore.Filters +{ + public class ExceptionFilter1 : IExceptionFilter + { + public void OnException(ExceptionContext context) + { + // test the behaviour when an application has two ExceptionFilters defined + } + } +} diff --git a/test/TestApp.AspNetCore/Filters/ExceptionFilter2.cs b/test/TestApp.AspNetCore/Filters/ExceptionFilter2.cs new file mode 100644 index 00000000000..96934bcb551 --- /dev/null +++ b/test/TestApp.AspNetCore/Filters/ExceptionFilter2.cs @@ -0,0 +1,28 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +using Microsoft.AspNetCore.Mvc.Filters; + +namespace TestApp.AspNetCore.Filters +{ + public class ExceptionFilter2 : IExceptionFilter + { + public void OnException(ExceptionContext context) + { + // test the behaviour when an application has two ExceptionFilters defined + } + } +} From 372da8289667819266e80f5704f7dfb12840c2db Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Tue, 23 Aug 2022 18:07:55 -0300 Subject: [PATCH 09/10] adding ErrorController and fixing tests --- .../BasicTests.cs | 4 +-- .../Controllers/ErrorController.cs | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) create mode 100644 test/TestApp.AspNetCore/Controllers/ErrorController.cs diff --git a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs index f168dbc4fd4..dfa1ba09ff4 100644 --- a/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs +++ b/test/OpenTelemetry.Instrumentation.AspNetCore.Tests/BasicTests.cs @@ -32,6 +32,7 @@ using OpenTelemetry.Tests; using OpenTelemetry.Trace; using TestApp.AspNetCore; +using TestApp.AspNetCore.Filters; using Xunit; namespace OpenTelemetry.Instrumentation.AspNetCore.Tests @@ -755,9 +756,6 @@ private static void AssertException(List exportedItems) Assert.Equal("System.Exception", activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionType).Value); Assert.Equal(exMessage, activity.Events.First().Tags.FirstOrDefault(t => t.Key == SemanticConventions.AttributeExceptionMessage).Value); - var status = activity.GetStatus(); - Assert.Equal(status, Status.Error.WithDescription(exMessage)); - ValidateAspNetCoreActivity(activity, "/api/error"); } diff --git a/test/TestApp.AspNetCore/Controllers/ErrorController.cs b/test/TestApp.AspNetCore/Controllers/ErrorController.cs new file mode 100644 index 00000000000..174888fec05 --- /dev/null +++ b/test/TestApp.AspNetCore/Controllers/ErrorController.cs @@ -0,0 +1,30 @@ +// +// Copyright The OpenTelemetry Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +using Microsoft.AspNetCore.Mvc; + +namespace TestApp.AspNetCore.Controllers +{ + [Route("api/[controller]")] + public class ErrorController : Controller + { + // GET api/error + [HttpGet] + public string Get() + { + throw new Exception("something's wrong!"); + } + } +} From dfd3202700d474442631e1fee435dbdabaa47c87 Mon Sep 17 00:00:00 2001 From: Alexandre Machado Date: Tue, 23 Aug 2022 20:21:57 -0300 Subject: [PATCH 10/10] removing unused directories --- .../Controllers/ErrorController.cs | 31 ------------------- .../Filters/ExceptionFilter1.cs | 29 ----------------- .../Filters/ExceptionFilter2.cs | 28 ----------------- .../Controllers/ErrorController.cs | 31 ------------------- .../Filters/ExceptionFilter1.cs | 29 ----------------- .../Filters/ExceptionFilter2.cs | 28 ----------------- 6 files changed, 176 deletions(-) delete mode 100644 test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs delete mode 100644 test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs delete mode 100644 test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs delete mode 100644 test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs delete mode 100644 test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs delete mode 100644 test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs diff --git a/test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs b/test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs deleted file mode 100644 index b51ba4e8687..00000000000 --- a/test/TestApp.AspNetCore.3.1/Controllers/ErrorController.cs +++ /dev/null @@ -1,31 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -using System; -using Microsoft.AspNetCore.Mvc; - -namespace TestApp.AspNetCore._3._1.Controllers -{ - [Route("api/[controller]")] - public class ErrorController : Controller - { - // GET api/error - [HttpGet] - public string Get() - { - throw new Exception("something's wrong!"); - } - } -} diff --git a/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs b/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs deleted file mode 100644 index 44bf59e1cbe..00000000000 --- a/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter1.cs +++ /dev/null @@ -1,29 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using Microsoft.AspNetCore.Mvc.Filters; - -namespace TestApp.AspNetCore._3._1.Filters -{ - public class ExceptionFilter1 : IExceptionFilter - { - public void OnException(ExceptionContext context) - { - // test if the changes that were made to fix the behaviour where - // an application with a defined ExceptionFilter wouldn't collect those - } - } -} diff --git a/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs b/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs deleted file mode 100644 index d87bc993777..00000000000 --- a/test/TestApp.AspNetCore.3.1/Filters/ExceptionFilter2.cs +++ /dev/null @@ -1,28 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using Microsoft.AspNetCore.Mvc.Filters; - -namespace TestApp.AspNetCore._3._1.Filters -{ - public class ExceptionFilter2 : IExceptionFilter - { - public void OnException(ExceptionContext context) - { - // test the behaviour when an application has two ExceptionFilters defined - } - } -} diff --git a/test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs b/test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs deleted file mode 100644 index 6e00f2455f6..00000000000 --- a/test/TestApp.AspNetCore.6.0/Controllers/ErrorController.cs +++ /dev/null @@ -1,31 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -using System; -using Microsoft.AspNetCore.Mvc; - -namespace TestApp.AspNetCore._6._0.Controllers -{ - [Route("api/[controller]")] - public class ErrorController : Controller - { - // GET api/error - [HttpGet] - public string Get() - { - throw new Exception("something's wrong!"); - } - } -} diff --git a/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs b/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs deleted file mode 100644 index 31ebf4debd3..00000000000 --- a/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter1.cs +++ /dev/null @@ -1,29 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using Microsoft.AspNetCore.Mvc.Filters; - -namespace TestApp.AspNetCore._6._0.Filters -{ - public class ExceptionFilter1 : IExceptionFilter - { - public void OnException(ExceptionContext context) - { - // test if the changes that were made to fix the behaviour where - // an application with a defined ExceptionFilter wouldn't collect those - } - } -} diff --git a/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs b/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs deleted file mode 100644 index 9d7c5fafb57..00000000000 --- a/test/TestApp.AspNetCore.6.0/Filters/ExceptionFilter2.cs +++ /dev/null @@ -1,28 +0,0 @@ -// -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// - -using Microsoft.AspNetCore.Mvc.Filters; - -namespace TestApp.AspNetCore._6._0.Filters -{ - public class ExceptionFilter2 : IExceptionFilter - { - public void OnException(ExceptionContext context) - { - // test the behaviour when an application has two ExceptionFilters defined - } - } -}