Skip to content

Fix displaying status message in span details#7075

Merged
JamesNK merged 2 commits intomainfrom
jamesnk/status-message-fix
Jan 14, 2025
Merged

Fix displaying status message in span details#7075
JamesNK merged 2 commits intomainfrom
jamesnk/status-message-fix

Conversation

@JamesNK
Copy link
Copy Markdown
Member

@JamesNK JamesNK commented Jan 13, 2025

Description

Fixes #7061

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

if (!string.IsNullOrEmpty(StatusMessage))
{
props.Add(new OtlpDisplayField { DisplayName = "StatusMessage", Key = KnownTraceFields.StatusField, Value = Status.ToString() });
props.Add(new OtlpDisplayField { DisplayName = "StatusMessage", Key = KnownTraceFields.StatusMessageField, Value = StatusMessage });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this was the bug?

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.

Yes. status and status_message had the same key.

Apparently very few frameworks include a status message with spans. It looks like Azure Functions does which is why the user hit this problem.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could still explode right or is this unique?

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.

It's unique. Key is trace.status_message. Even if there is a custom tag named trace.status_message the custom tag is prefixed with unknown-. So it ends up as unknown-trace.status_message.

(these keys are all internal)

@JamesNK JamesNK merged commit d7b7807 into main Jan 14, 2025
@JamesNK JamesNK deleted the jamesnk/status-message-fix branch January 14, 2025 10:21
@mu88
Copy link
Copy Markdown
Contributor

mu88 commented Jan 15, 2025

@JamesNK / @davidfowl: do you provide nightly Docker images of the Aspire Dashboard so that I can test whether this PR fixes my issue?

I have a pretty basic ASP.NET Core app with an underlying PostgreSQL DB, accessed via EF Core (everything on .NET 9). When using the OpenTelemetry instrumentation of EF Core and executing an invalid DB query (e.g. await dbContext.Database.ExecuteSqlRawAsync("SELECT * FROM \"NotExistingTable\"");), this corresponding span is correctly marked with Span Status = Error (see here). However, when trying to open the span's details within the Aspire Dashboard, it crashes with the error message of #7061

image

@davidfowl
Copy link
Copy Markdown
Contributor

@joperezr it's still a manual process right?

@joperezr
Copy link
Copy Markdown
Member

Correct. If it's helpful, we could add some instructions on how to build this image yourself, but today it is still a manual process that happens before a release.

@mu88
Copy link
Copy Markdown
Contributor

mu88 commented Jan 16, 2025

Thx for the insights, yes, I'd indeed appreciate knowing how I can build this image by myself 🙂

And when/in which version will this fix be available in a "normal way", one officially published Docker image? Will it be part of the next .NET 9 patch?

@JamesNK
Copy link
Copy Markdown
Member Author

JamesNK commented Jan 16, 2025

We plan to have an Aspire 9.1 release and this fix will be included. We don't have an exact date but I think that will be at least a month away. If you want to use the dashboard with this fix in the short term then you'll need to use your own build.

@joperezr In previous Aspire releases there was a delay in publishing the dashboard image. Do you think that will still be the case?

@mu88
Copy link
Copy Markdown
Contributor

mu88 commented Jan 16, 2025

okay, so I'd be happy if you could provide some information about how to build the corresponding Docker image containing the fix on my own 🙂

@davidfowl
Copy link
Copy Markdown
Contributor

This is the docker file https://github.com/dotnet/dotnet-docker/blob/main/src/aspire-dashboard/9.0/cbl-mariner-distroless/amd64/Dockerfile

I don't know how you discover the version number? @joperezr

@joperezr
Copy link
Copy Markdown
Member

@mu88
Copy link
Copy Markdown
Contributor

mu88 commented Jan 20, 2025

Okay, at least I can build the Docker image 👍🏻 but I'm wondering which version I have to use here so that this PR is pulled into the image. Can you please elaborate on how one can determine the correct version to use for a specific commit?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Duplicate key 'trace.status' results in Blazor error/crash

4 participants