Skip to content

Add source to span details#4703

Merged
JamesNK merged 2 commits intomainfrom
jamesnk/span-source
Jul 4, 2024
Merged

Add source to span details#4703
JamesNK merged 2 commits intomainfrom
jamesnk/span-source

Conversation

@JamesNK
Copy link
Copy Markdown
Member

@JamesNK JamesNK commented Jun 27, 2024

Fixes #3711

While making other repository changes I noticed that the source was on the trace instead of span. Moved it, and added the UI to display a context section to span details. The new section includes the span's source (in .NET land, this is the activity source name such as Microsoft.AspNetCore or System.Net.Http), parent id and trace id.

image

Microsoft Reviewers: Open in CodeFlow

@JamesNK JamesNK added area-dashboard area-telemetry Telemetry from the Aspire app to the dashboard labels Jun 27, 2024
@JamesNK
Copy link
Copy Markdown
Member Author

JamesNK commented Jun 28, 2024

/azp run

@JamesNK
Copy link
Copy Markdown
Member Author

JamesNK commented Jun 28, 2024

Please review 🙏

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@JamesNK JamesNK force-pushed the jamesnk/span-source branch from c0d7128 to 2938f06 Compare July 3, 2024 05:14
Comment thread tests/Aspire.Dashboard.Tests/TelemetryRepositoryTests/TraceTests.cs
Copy link
Copy Markdown
Member

@radical radical left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really familiar with the code. But diff seems to do what the PR description says, and makes sense to me - whatever I could understand!

{
_contextAttributes =
[
new KeyValuePair<string, string>("Source", ViewModel.Span.Scope.ScopeName)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason these are hardcoded?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you asking why the names aren't in resource files for translation? None of the "known" property names in the details screens are translated because none of the properties that come from OTEL are translated.

Another example: https://github.com/dotnet/aspire/blob/7d5a03251a9e7e3099e87ee34c9499dde3095288/src/Aspire.Dashboard/Components/Controls/StructuredLogDetails.razor.cs#L53-L64

@JamesNK JamesNK merged commit 269ed33 into main Jul 4, 2024
@JamesNK JamesNK deleted the jamesnk/span-source branch July 4, 2024 00:28
@radical
Copy link
Copy Markdown
Member

radical commented Jul 4, 2024

main rolling build broke:

D:\a\_work\1\s\tests\Aspire.Dashboard.Tests\TelemetryRepositoryTests\TraceTests.cs(147,13): error CS0117: 'GetTracesRequest' does not contain a definition for 'ApplicationServiceId' [D:\a\_work\1\s\tests\Aspire.Dashboard.Tests\Aspire.Dashboard.Tests.csproj]
##[error]tests\Aspire.Dashboard.Tests\TelemetryRepositoryTests\TraceTests.cs(147,13): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'GetTracesRequest' does not contain a definition for 'ApplicationServiceId'
D:\a\_work\1\s\tests\Aspire.Dashboard.Tests\TelemetryRepositoryTests\TraceTests.cs(145,47): error CS9035: Required member 'GetTracesRequest.ApplicationKey' must be set in the object initializer or attribute constructor. [D:\a\_work\1\s\tests\Aspire.Dashboard.Tests\Aspire.Dashboard.Tests.csproj]
##[error]tests\Aspire.Dashboard.Tests\TelemetryRepositoryTests\TraceTests.cs(145,47): error CS9035: (NETCORE_ENGINEERING_TELEMETRY=Build) Required member 'GetTracesRequest.ApplicationKey' must be set in the object initializer or attribute constructor.

@JamesNK
Copy link
Copy Markdown
Member Author

JamesNK commented Jul 4, 2024

I included a fix in another PR: 5acbf8f (#4764)

@radical
Copy link
Copy Markdown
Member

radical commented Jul 4, 2024

I included a fix in another PR: 5acbf8f (#4764)

Cool, assuming that it is ready to merge once CI is green. Thanks!

@github-actions github-actions Bot locked and limited conversation to collaborators Aug 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-dashboard area-telemetry Telemetry from the Aspire app to the dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard doesn't show the name of the component firing telemetry

3 participants