Skip to content

Fix RabbitMQ log forwarder EventId names in structured logs#17355

Open
alirezafzali wants to merge 1 commit into
microsoft:mainfrom
alirezafzali:fix/rabbitmq-logging-event-id-name
Open

Fix RabbitMQ log forwarder EventId names in structured logs#17355
alirezafzali wants to merge 1 commit into
microsoft:mainfrom
alirezafzali:fix/rabbitmq-logging-event-id-name

Conversation

@alirezafzali
Copy link
Copy Markdown
Contributor

Description

RabbitMQ's EventSource uses event names "Info", "Warn", and "Error" that mirror log level. RabbitMQEventSourceLogForwarder was passing those into ILogger as EventId.Name, so the Aspire dashboard structured log UI showed misleading event names (e.g. "Error") instead of leaving the event name unset.

This change forwards only the numeric RabbitMQ event id (1 / 2 / 3) and omits the level-like name. Log level continues to come from LogLevel as before.

Duplicate log message text in structured logs was already addressed in #6827; this completes the remaining part of #2967.

Changes:

  • RabbitMQEventSourceLogForwarder.cs: use new EventId(eventData.EventId) instead of including eventData.EventName
  • AspireRabbitMQLoggingTests.cs: add ForwardsEventIdWithoutLevelName regression test

Dependencies: None

Fixes #2967

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

Copilot AI review requested due to automatic review settings May 21, 2026 15:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adjusts RabbitMQ EventSource log forwarding to avoid populating EventId.Name with the EventSource level name, and adds a regression test to validate that behavior.

Changes:

  • Stop passing eventData.EventName when constructing EventId during EventSource forwarding.
  • Extend the test logger to record EventId values.
  • Add a new unit test asserting EventId.Id is forwarded while EventId.Name remains empty.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/Aspire.RabbitMQ.Client.Tests/AspireRabbitMQLoggingTests.cs Adds a test for EventId name suppression; updates test logger to capture EventId.
src/Components/Aspire.RabbitMQ.Client/RabbitMQEventSourceLogForwarder.cs Changes EventId construction to omit Name, preventing level name being forwarded.

LogInfo("info");
LogWarn("warn");
LogError("error", new InvalidOperationException("test"));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The other unit tests in this file don’t wait after Log*. EventSource forwarding is synchronous once the listener is started. Happy to restructure if maintainers prefer a different pattern.

@github-actions
Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17355

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17355"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RabbitMQ logging issues

2 participants