Skip to content

Commit

Permalink
Fix span names
Browse files Browse the repository at this point in the history
  • Loading branch information
@Kielek
Kielek committed Mar 15, 2024
1 parent 7feba09 commit 87bf97b
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 26 deletions.
3 changes: 2 additions & 1 deletion src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@
* **Breaking Change**: `server.address` and `server.port` no longer added
for `http.server.request.duration` metric.
([#1606](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1606))
* **Breaking change** Spans attributes based on [HTTP semantic convention v1.24.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md).
* **Breaking change** Spans names and attributes
based on [HTTP semantic convention v1.24.0](https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md).
([#1607](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/1607))

## 1.7.0-beta.2
Expand Down
12 changes: 6 additions & 6 deletions src/OpenTelemetry.Instrumentation.AspNet/Implementation/HttpInListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ private void OnStartActivity(Activity activity, HttpContext context)
var requestValues = request.Unvalidated;

// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.24.0/docs/http/http-spans.md
var path = requestValues.Path;
activity.DisplayName = path;
var originalHttpMethod = request.HttpMethod;
var normalizedHttpMethod = this.requestDataHelper.GetNormalizedHttpMethod(originalHttpMethod);
activity.DisplayName = normalizedHttpMethod == "_OTHER" ? "HTTP" : normalizedHttpMethod;

var url = request.Url;
activity.SetTag(SemanticConventions.AttributeServerAddress, url.Host);
activity.SetTag(SemanticConventions.AttributeServerPort, url.Port);
activity.SetTag(SemanticConventions.AttributeUrlScheme, url.Scheme);

var originalHttpMethod = request.HttpMethod;
this.requestDataHelper.SetHttpMethodTag(activity, originalHttpMethod);

var protocolVersion = RequestDataHelper.GetHttpProtocolVersion(request);
Expand All @@ -85,7 +85,7 @@ private void OnStartActivity(Activity activity, HttpContext context)
activity.SetTag(SemanticConventions.AttributeNetworkProtocolVersion, protocolVersion);
}

activity.SetTag(SemanticConventions.AttributeUrlPath, path);
activity.SetTag(SemanticConventions.AttributeUrlPath, requestValues.Path);

// TODO url.query should be sanitized
var query = url.Query;
Expand Down Expand Up @@ -131,8 +131,8 @@ private void OnStopActivity(Activity activity, HttpContext context)

if (!string.IsNullOrEmpty(template))
{
// Override the name that was previously set to the path part of URL.
activity.DisplayName = template!;
// Override the name that was previously set to the normalized HTTP method/HTTP
activity.DisplayName = $"{activity.DisplayName} {template!}";
activity.SetTag(SemanticConventions.AttributeHttpRoute, template);
}

Expand Down
35 changes: 18 additions & 17 deletions test/OpenTelemetry.Instrumentation.AspNet.Tests/HttpInListenerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,21 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests;
public class HttpInListenerTests
{
[Theory]
[InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null)]
[InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", "localhost", 80, "POST", "POST", null, 0, null, true)]
[InlineData("https://localhost/", "https", "/", null, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null)]
[InlineData("https://user:pass@localhost/", "https", "/", null, "localhost", 443, "GeT", "GET", "GeT", 0, null)] // Test URL sanitization
[InlineData("http://localhost:443/", "http", "/", null, "localhost", 443, "GET", "GET", null, 0, null)] // Test http over 443
[InlineData("https://localhost:80/", "https", "/", null, "localhost", 80, "GET", "GET", null, 0, null)] // Test https over 80
[InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null)] // Test complex URL
[InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null)] // Test complex URL sanitization
[InlineData("http://localhost:80/Index", "http", "/Index", null, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}")]
[InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, "localhost", 443, "GET", "GET", null, 2, "about_attr_route/{customerId}")]
[InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}")]
[InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}")]
[InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, false, "/api/value")] // Request will be filtered
[InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, false, "{ThrowException}")] // Filter user code will throw an exception
[InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, false, null, true, "System.InvalidOperationException")] // Test RecordException option
[InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET")]
[InlineData("http://localhost/?foo=bar&baz=test", "http", "/", "foo=bar&baz=test", "localhost", 80, "POST", "POST", null, 0, null, "POST", true)]
[InlineData("https://localhost/", "https", "/", null, "localhost", 443, "NonStandard", "_OTHER", "NonStandard", 0, null, "HTTP")]
[InlineData("https://user:pass@localhost/", "https", "/", null, "localhost", 443, "GeT", "GET", "GeT", 0, null, "GET")] // Test URL sanitization
[InlineData("http://localhost:443/", "http", "/", null, "localhost", 443, "GET", "GET", null, 0, null, "GET")] // Test http over 443
[InlineData("https://localhost:80/", "https", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test https over 80
[InlineData("https://localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL
[InlineData("https://user:password@localhost:80/Home/Index.htm?q1=v1&q2=v2#FragmentName", "https", "/Home/Index.htm", "q1=v1&q2=v2", "localhost", 80, "GET", "GET", null, 0, null, "GET")] // Test complex URL sanitization
[InlineData("http://localhost:80/Index", "http", "/Index", null, "localhost", 80, "GET", "GET", null, 1, "{controller}/{action}/{id}", "GET {controller}/{action}/{id}")]
[InlineData("https://localhost:443/about_attr_route/10", "https", "/about_attr_route/10", null, "localhost", 443, "HEAD", "HEAD", null, 2, "about_attr_route/{customerId}", "HEAD about_attr_route/{customerId}")]
[InlineData("http://localhost:1880/api/weatherforecast", "http", "/api/weatherforecast", null, "localhost", 1880, "GET", "GET", null, 3, "api/{controller}/{id}", "GET api/{controller}/{id}")]
[InlineData("https://localhost:1843/subroute/10", "https", "/subroute/10", null, "localhost", 1843, "GET", "GET", null, 4, "subroute/{customerId}", "GET subroute/{customerId}")]
[InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "/api/value")] // Request will be filtered
[InlineData("http://localhost/api/value", "http", "/api/value", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, "{ThrowException}")] // Filter user code will throw an exception
[InlineData("http://localhost/", "http", "/", null, "localhost", 80, "GET", "GET", null, 0, null, "GET", false, null, true, "System.InvalidOperationException")] // Test RecordException option
public void AspNetRequestsAreCollectedSuccessfully(
string url,
string expectedUrlScheme,
Expand All @@ -46,7 +46,8 @@ public void AspNetRequestsAreCollectedSuccessfully(
string expectedRequestMethod,
string? expectedOriginalRequestMethod,
int routeType,
string routeTemplate,
string? routeTemplate,
string expectedName,
bool setStatusToErrorInEnrich = false,
string? filter = null,
bool recordException = false,
Expand Down Expand Up @@ -117,7 +118,7 @@ public void AspNetRequestsAreCollectedSuccessfully(
Assert.Equal(TelemetryHttpModule.AspNetActivityName, span.OperationName);
Assert.NotEqual(TimeSpan.Zero, span.Duration);

Assert.Equal(routeTemplate ?? HttpContext.Current.Request.Path, span.DisplayName);
Assert.Equal(expectedName, span.DisplayName);
Assert.Equal(ActivityKind.Server, span.Kind);
Assert.True(span.Duration != TimeSpan.Zero);

Expand Down
4 changes: 2 additions & 2 deletions test/OpenTelemetry.Instrumentation.AspNet.Tests/RouteTestHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace OpenTelemetry.Instrumentation.AspNet.Tests;

internal static class RouteTestHelper
{
public static HttpContext BuildHttpContext(string url, int routeType, string routeTemplate, string requestMethod)
public static HttpContext BuildHttpContext(string url, int routeType, string? routeTemplate, string requestMethod)
{
RouteData routeData;
switch (routeType)
Expand All @@ -21,7 +21,7 @@ public static HttpContext BuildHttpContext(string url, int routeType, string rou
case 1: // Traditional MVC.
case 2: // Attribute routing MVC.
case 3: // Traditional WebAPI.
routeData = new RouteData()
routeData = new RouteData
{
Route = new Route(routeTemplate, null),
};
Expand Down

0 comments on commit 87bf97b

Please sign in to comment.