Skip to content

Commit

Permalink
Fix missing uninstrumented peer icons in dashboard tracing for HTTP r…
Browse files Browse the repository at this point in the history
…equests (dotnet#2079)
  • Loading branch information
JamesNK authored and radical committed Feb 6, 2024
1 parent f218d0e commit e439fa5
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/Aspire.Dashboard/Components/Pages/TraceDetail.razor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ static SpanWaterfallViewModel CreateViewModel(OtlpSpan span, int depth, bool hid
var labelIsRight = (relativeStart + span.Duration / 2) < (span.Trace.Duration / 2);

// A span may indicate a call to another service but the service isn't instrumented.
var hasPeerService = span.Attributes.Any(a => a.Key == OtlpSpan.PeerServiceAttributeKey);
var hasPeerService = OtlpHelpers.GetPeerAddress(span.Attributes) != null;
var isUninstrumentedPeer = hasPeerService && span.Kind is OtlpSpanKind.Client or OtlpSpanKind.Producer && !span.GetChildSpans().Any();
var uninstrumentedPeer = isUninstrumentedPeer ? ResolveUninstrumentedPeerName(span, state.OutgoingPeerResolvers) : null;

Expand Down Expand Up @@ -144,7 +144,7 @@ static SpanWaterfallViewModel CreateViewModel(OtlpSpan span, int depth, bool hid
}

// Fallback to the peer address.
return OtlpHelpers.GetValue(span.Attributes, OtlpSpan.PeerServiceAttributeKey);
return OtlpHelpers.GetPeerAddress(span.Attributes);
}

protected override void OnParametersSet()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ public bool TryResolvePeerName(KeyValuePair<string, string>[] attributes, [NotNu
// A long term improvement here is to add tags to the BrowserLink client and then detect the
// values in the span's attributes.
const string lastSegment = "getScriptTag";
var url = OtlpHelpers.GetValue(attributes, "http.url");

// url.full replaces http.url but look for both for backwards compatibility.
var url = OtlpHelpers.GetValue(attributes, "url.full") ?? OtlpHelpers.GetValue(attributes, "http.url");

// Quick check of URL with EndsWith before more expensive Uri parsing.
if (url != null && url.EndsWith(lastSegment, StringComparison.OrdinalIgnoreCase))
Expand Down
2 changes: 1 addition & 1 deletion src/Aspire.Dashboard/Model/ResourceOutgoingPeerResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public bool TryResolvePeerName(KeyValuePair<string, string>[] attributes, [NotNu

internal static bool TryResolvePeerNameCore(IDictionary<string, ResourceViewModel> resources, KeyValuePair<string, string>[] attributes, out string? name)
{
var address = OtlpHelpers.GetValue(attributes, OtlpSpan.PeerServiceAttributeKey);
var address = OtlpHelpers.GetPeerAddress(attributes);
if (address != null)
{
// Match exact value.
Expand Down
31 changes: 31 additions & 0 deletions src/Aspire.Dashboard/Otlp/Model/OtlpHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,37 @@ private static void CopyKeyValues(RepeatedField<KeyValue> attributes, KeyValuePa
return null;
}

public static string? GetPeerAddress(this KeyValuePair<string, string>[] values)
{
var address = GetValue(values, OtlpSpan.PeerServiceAttributeKey);
if (address != null)
{
return address;
}

// OTEL HTTP 1.7.0 doesn't return peer.service. Fallback to server.address and server.port.
if (GetValue(values, OtlpSpan.ServerAddressAttributeKey) is { } server)
{
if (GetValue(values, OtlpSpan.ServerPortAttributeKey) is { } serverPort)
{
server += ":" + serverPort;
}
return server;
}

// Fallback to older names of net.peer.name and net.peer.port.
if (GetValue(values, OtlpSpan.NetPeerNameAttributeKey) is { } peer)
{
if (GetValue(values, OtlpSpan.NetPeerPortAttributeKey) is { } peerPort)
{
peer += ":" + peerPort;
}
return peer;
}

return null;
}

public static bool HasKey(this KeyValuePair<string, string>[] values, string name)
{
for (var i = 0; i < values.Length; i++)
Expand Down
5 changes: 5 additions & 0 deletions src/Aspire.Dashboard/Otlp/Model/OtlpSpan.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ namespace Aspire.Dashboard.Otlp.Model;
public class OtlpSpan
{
public const string PeerServiceAttributeKey = "peer.service";
public const string UrlFullAttributeKey = "url.full";
public const string ServerAddressAttributeKey = "server.address";
public const string ServerPortAttributeKey = "server.port";
public const string NetPeerNameAttributeKey = "net.peer.name";
public const string NetPeerPortAttributeKey = "net.peer.port";
public const string SpanKindAttributeKey = "span.kind";

public string TraceId => Trace.TraceId;
Expand Down
14 changes: 14 additions & 0 deletions tests/Aspire.Dashboard.Tests/ResourceOutgoingPeerResolverTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,4 +113,18 @@ public void CommaAddressValueAttribute_Match()
Assert.True(ResourceOutgoingPeerResolver.TryResolvePeerNameCore(resources, [KeyValuePair.Create("peer.service", "127.0.0.1,5000")], out var value));
Assert.Equal("test", value);
}

[Fact]
public void ServerAddressAndPort_Match()
{
// Arrange
var resources = new Dictionary<string, ResourceViewModel>
{
["test"] = CreateResource("test", "localhost", 5000)
};

// Act & Assert
Assert.True(ResourceOutgoingPeerResolver.TryResolvePeerNameCore(resources, [KeyValuePair.Create("server.address", "localhost"), KeyValuePair.Create("server.port", "5000")], out var value));
Assert.Equal("test", value);
}
}

0 comments on commit e439fa5

Please sign in to comment.