Skip to content
This repository was archived by the owner on Dec 18, 2023. It is now read-only.

Commit 6fc3830

Browse files
SergeyKanzhelevSergeyKanzhelev
authored
Improved data collection for ASP.NET Core incoming requests (#75)
* no tests, but the proper data is being collected * fix tests by allowing tasks to execute, disposing resources and running tests sequentially * remove null check for request object as private method only called from that class after this check
1 parent fa7d468 commit 6fc3830

File tree

10 files changed

+283
-27
lines changed

10 files changed

+283
-27
lines changed

‎src/OpenCensus.Collector.AspNetCore/Implementation/HttpInListener.cs‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@
1616

1717
namespace OpenCensus.Collector.AspNetCore.Implementation
1818
{
19+
using System;
1920
using System.Diagnostics;
21+
using System.Linq;
22+
using System.Text;
2023
using Microsoft.AspNetCore.Http;
2124
using Microsoft.AspNetCore.Http.Features;
2225
using OpenCensus.Collector.Implementation.Common;
@@ -25,6 +28,7 @@ namespace OpenCensus.Collector.AspNetCore.Implementation
2528

2629
internal class HttpInListener : ListenerHandler
2730
{
31+
private const string UnknownHostName = "UNKNOWN-HOST";
2832
private readonly PropertyFetcher startContextFetcher = new PropertyFetcher("HttpContext");
2933
private readonly PropertyFetcher stopContextFetcher = new PropertyFetcher("HttpContext");
3034
private readonly IPropagationComponent propagationComponent;
@@ -51,16 +55,26 @@ public override void OnStartActivity(Activity activity, object payload)
5155
request,
5256
(r, name) => r.Headers[name]);
5357

54-
this.Tracer.SpanBuilderWithRemoteParent("HttpIn", ctx).SetSampler(this.Sampler).StartScopedSpan();
58+
// see the spec https://github.com/census-instrumentation/opencensus-specs/blob/master/trace/HTTP.md
59+
60+
string path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
61+
62+
this.Tracer.SpanBuilderWithRemoteParent(path, ctx).SetSampler(this.Sampler).StartScopedSpan();
5563

5664
var span = this.Tracer.CurrentSpan;
5765

5866
if (span != null)
5967
{
68+
// Note, route is missing at this stage. It will be available later
69+
6070
span.PutServerSpanKindAttribute();
61-
span.PutHttpMethodAttribute(request.Method);
6271
span.PutHttpHostAttribute(request.Host.Host, request.Host.Port ?? 80);
63-
span.PutHttpPathAttribute(request.Path);
72+
span.PutHttpMethodAttribute(request.Method);
73+
span.PutHttpPathAttribute(path);
74+
75+
var userAgent = request.Headers["User-Agent"].FirstOrDefault();
76+
span.PutHttpUserAgentAttribute(userAgent);
77+
span.PutHttpRawUrlAttribute(GetUri(request));
6478
}
6579
}
6680

@@ -86,5 +100,40 @@ public override void OnStopActivity(Activity activity, object payload)
86100
span.PutHttpStatusCode(response.StatusCode, response.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase);
87101
span.End();
88102
}
103+
104+
private static string GetUri(HttpRequest request)
105+
{
106+
var builder = new StringBuilder();
107+
108+
builder.Append(request.Scheme).Append("://");
109+
110+
if (request.Host.HasValue)
111+
{
112+
builder.Append(request.Host.Value);
113+
}
114+
else
115+
{
116+
// HTTP 1.0 request with NO host header would result in empty Host.
117+
// Use placeholder to avoid incorrect URL like "http:///"
118+
builder.Append(UnknownHostName);
119+
}
120+
121+
if (request.PathBase.HasValue)
122+
{
123+
builder.Append(request.PathBase.Value);
124+
}
125+
126+
if (request.Path.HasValue)
127+
{
128+
builder.Append(request.Path.Value);
129+
}
130+
131+
if (request.QueryString.HasValue)
132+
{
133+
builder.Append(request.QueryString);
134+
}
135+
136+
return builder.ToString();
137+
}
89138
}
90139
}

‎src/OpenCensus.Collector.AspNetCore/OpenCensus.Collector.AspNetCore.csproj‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@
2121
<Compile Include="..\OpenCensus.Collector.Implementation.Common\**" LinkBase="Implementation\Common\" />
2222
</ItemGroup>
2323

24+
<ItemGroup>
25+
<None Remove="xunit.runner.json" />
26+
</ItemGroup>
27+
28+
<ItemGroup>
29+
<Content Include="xunit.runner.json">
30+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
31+
</Content>
32+
</ItemGroup>
33+
2434
<ItemGroup>
2535
<ProjectReference Include="..\OpenCensus.Abstractions\OpenCensus.Abstractions.csproj" />
2636
<PackageReference Include="StyleCop.Analyzers" Version="1.1.0-beta008">

‎src/OpenCensus.Collector.AspNetCore/xunit.runner.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"maxParallelThreads": 1,
3+
"parallelizeTestCollections": false
4+
}

‎src/OpenCensus.Exporter.Prometheus/OpenCensus.Exporter.Prometheus.csproj‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020

2121
<ItemGroup>
2222
<ProjectReference Include="..\OpenCensus.Abstractions\OpenCensus.Abstractions.csproj" />
23-
<PackageReference Include="StyleCop.Analyzers" Version="1.0.2">
23+
<PackageReference Include="StyleCop.Analyzers" Version="1.1.0-beta008">
2424
<PrivateAssets>All</PrivateAssets>
2525
</PackageReference>
2626
</ItemGroup>

‎test/OpenCensus.Collector.AspNetCore.Tests/BasicTests.cs‎

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ namespace OpenCensus.Collector.AspNetCore.Tests
2727
using OpenCensus.Common;
2828
using Moq;
2929
using Microsoft.AspNetCore.TestHost;
30-
using System.Threading;
3130
using System;
3231
using OpenCensus.Trace.Propagation;
3332
using Microsoft.AspNetCore.Http;
@@ -55,20 +54,32 @@ void ConfigureTestServices(IServiceCollection services) =>
5554
services.AddSingleton<ITracer>(tracer);
5655

5756
// Arrange
58-
var client = this.factory
57+
using (var client = this.factory
5958
.WithWebHostBuilder(builder =>
6059
builder.ConfigureTestServices(ConfigureTestServices))
61-
.CreateClient();
60+
.CreateClient())
61+
{
6262

63-
// Act
64-
var response = await client.GetAsync("/api/values");
63+
// Act
64+
var response = await client.GetAsync("/api/values");
6565

66-
// Assert
67-
response.EnsureSuccessStatusCode(); // Status Code 200-299
66+
// Assert
67+
response.EnsureSuccessStatusCode(); // Status Code 200-299
68+
69+
for (int i = 0; i < 10; i++)
70+
{
71+
if (startEndHandler.Invocations.Count == 2)
72+
{
73+
break;
74+
}
75+
76+
// We need to let End callback execute as it is executed AFTER response was returned.
77+
// In unit tests environment there may be a lot of parallel unit tests executed, so
78+
// giving some breezing room for the End callback to complete
79+
await Task.Delay(TimeSpan.FromSeconds(1));
80+
}
81+
}
6882

69-
// TODO: this is needed as span generation happens after response is returned for some reason.
70-
// need to investigate
71-
Thread.Sleep(TimeSpan.FromMilliseconds(1));
7283

7384
Assert.Equal(2, startEndHandler.Invocations.Count); // begin and end was called
7485
var spanData = ((Span)startEndHandler.Invocations[1].Arguments[0]).ToSpanData();
@@ -96,29 +107,41 @@ public async Task SuccesfulTemplateControllerCallUsesRemoteParentContext()
96107

97108
var propagationComponent = new Mock<IPropagationComponent>();
98109
propagationComponent.SetupGet(m => m.TextFormat).Returns(tf.Object);
99-
110+
100111

101112
// Arrange
102-
var client = this.factory
113+
using (var client = this.factory
103114
.WithWebHostBuilder(builder =>
104-
builder.ConfigureTestServices((services) => {
115+
builder.ConfigureTestServices((services) =>
116+
{
105117
services.AddSingleton<ITracer>(tracer);
106118
services.AddSingleton<IPropagationComponent>(propagationComponent.Object);
107119
}))
108-
.CreateClient();
120+
.CreateClient())
121+
{
109122

110-
// Act
111-
var response = await client.GetAsync("/api/values");
123+
// Act
124+
var response = await client.GetAsync("/api/values");
112125

113-
// Assert
114-
response.EnsureSuccessStatusCode(); // Status Code 200-299
126+
// Assert
127+
response.EnsureSuccessStatusCode(); // Status Code 200-299
115128

116-
// TODO: this is needed as span generation happens after response is returned for some reason.
117-
// need to investigate
118-
Thread.Sleep(TimeSpan.FromMilliseconds(1));
129+
for (int i = 0; i < 10; i++)
130+
{
131+
if (startEndHandler.Invocations.Count == 2)
132+
{
133+
break;
134+
}
135+
136+
// We need to let End callback execute as it is executed AFTER response was returned.
137+
// In unit tests environment there may be a lot of parallel unit tests executed, so
138+
// giving some breezing room for the End callback to complete
139+
await Task.Delay(TimeSpan.FromSeconds(1));
140+
}
141+
}
119142

120143
Assert.Equal(2, startEndHandler.Invocations.Count); // begin and end was called
121-
var spanData = ((Span)startEndHandler.Invocations[1].Arguments[0]).ToSpanData();
144+
var spanData = ((Span)startEndHandler.Invocations[0].Arguments[0]).ToSpanData();
122145

123146
Assert.Equal(SpanKind.Server, spanData.Kind);
124147
Assert.Equal(AttributeValue.StringAttributeValue("/api/values"), spanData.Attributes.AttributeMap["http.path"]);

‎test/OpenCensus.Collector.AspNetCore.Tests/IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
// <copyright file="IncomingRequestsCollectionsIsAccordingToTheSpecTests.cs" company="OpenCensus Authors">
2+
// Copyright 2018, OpenCensus Authors
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of theLicense at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
// </copyright>
16+
17+
namespace OpenCensus.Collector.AspNetCore.Tests
18+
{
19+
using Xunit;
20+
using Microsoft.AspNetCore.Mvc.Testing;
21+
using TestApp.AspNetCore._2._0;
22+
using System.Threading.Tasks;
23+
using Microsoft.Extensions.DependencyInjection;
24+
using OpenCensus.Trace;
25+
using OpenCensus.Trace.Config;
26+
using OpenCensus.Trace.Internal;
27+
using OpenCensus.Common;
28+
using Moq;
29+
using Microsoft.AspNetCore.TestHost;
30+
using System;
31+
using Microsoft.AspNetCore.Http;
32+
33+
public class IncomingRequestsCollectionsIsAccordingToTheSpecTests
34+
: IClassFixture<WebApplicationFactory<Startup>>
35+
{
36+
private readonly WebApplicationFactory<Startup> factory;
37+
38+
public IncomingRequestsCollectionsIsAccordingToTheSpecTests(WebApplicationFactory<Startup> factory)
39+
{
40+
this.factory = factory;
41+
42+
}
43+
44+
public class TestCallbackMiddlewareImpl : CallbackMiddleware.CallbackMiddlewareImpl
45+
{
46+
47+
public override async Task<bool> ProcessAsync(HttpContext context)
48+
{
49+
context.Response.StatusCode = 503;
50+
await context.Response.WriteAsync("empty");
51+
return false;
52+
}
53+
}
54+
55+
[Fact]
56+
public async Task SuccesfulTemplateControllerCallGeneratesASpan()
57+
{
58+
var startEndHandler = new Mock<IStartEndHandler>();
59+
var tracer = new Tracer(new RandomGenerator(), startEndHandler.Object, new DateTimeOffsetClock(), new TraceConfig());
60+
61+
// Arrange
62+
using (var client = this.factory
63+
.WithWebHostBuilder(builder =>
64+
builder.ConfigureTestServices((IServiceCollection services) =>
65+
{
66+
services.AddSingleton<CallbackMiddleware.CallbackMiddlewareImpl>(new TestCallbackMiddlewareImpl());
67+
services.AddSingleton<ITracer>(tracer);
68+
}))
69+
.CreateClient())
70+
{
71+
72+
try
73+
{
74+
// Act
75+
var response = await client.GetAsync("/api/values");
76+
}
77+
catch (Exception ex)
78+
{
79+
// ignore errors
80+
}
81+
82+
for (int i = 0; i < 10; i++)
83+
{
84+
if (startEndHandler.Invocations.Count == 2)
85+
{
86+
break;
87+
}
88+
89+
// We need to let End callback execute as it is executed AFTER response was returned.
90+
// In unit tests environment there may be a lot of parallel unit tests executed, so
91+
// giving some breezing room for the End callback to complete
92+
await Task.Delay(TimeSpan.FromSeconds(1));
93+
}
94+
}
95+
96+
Assert.Equal(2, startEndHandler.Invocations.Count); // begin and end was called
97+
var spanData = ((Span)startEndHandler.Invocations[0].Arguments[0]).ToSpanData();
98+
99+
Assert.Equal(SpanKind.Server, spanData.Kind);
100+
Assert.Equal(AttributeValue.StringAttributeValue("/api/values"), spanData.Attributes.AttributeMap["http.path"]);
101+
Assert.Equal(AttributeValue.LongAttributeValue(503), spanData.Attributes.AttributeMap["http.status_code"]);
102+
}
103+
}
104+
}

‎test/OpenCensus.Collector.Dependencies.Tests/OpenCensus.Collector.Dependencies.Tests.csproj‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@
66
<TargetFrameworks>netcoreapp2.0</TargetFrameworks>
77
</PropertyGroup>
88

9+
<ItemGroup>
10+
<None Remove="xunit.runner.json" />
11+
</ItemGroup>
12+
13+
<ItemGroup>
14+
<Content Include="xunit.runner.json">
15+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
16+
</Content>
17+
</ItemGroup>
18+
919
<ItemGroup>
1020
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.8.0" />
1121
<PackageReference Include="Moq" Version="4.9.0" />
@@ -20,7 +30,7 @@
2030

2131
<ItemGroup>
2232
<EmbeddedResource Include="http-out-test-cases.json">
23-
<CopyToOutputDirectory>Never</CopyToOutputDirectory>
33+
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
2434
</EmbeddedResource>
2535
</ItemGroup>
2636

‎test/OpenCensus.Collector.Dependencies.Tests/xunit.runner.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"maxParallelThreads": 1,
3+
"parallelizeTestCollections": false
4+
}

0 commit comments

Comments
 (0)