Skip to content

Give unique event ID to TRANSACTION_CREATED_OLETX_EVENTID - #109138

Merged
jeffhandley merged 3 commits into
dotnet:mainfrom
Camios:patch-1
May 15, 2025
Merged

Give unique event ID to TRANSACTION_CREATED_OLETX_EVENTID#109138
jeffhandley merged 3 commits into
dotnet:mainfrom
Camios:patch-1

Conversation

@Camios

@Camios Camios commented Oct 23, 2024

Copy link
Copy Markdown
Contributor

TransactionsEtwProvider incorrectly allocated the same event Id (11) to METHOD_ENTER_LTM_EVENTID and TRANSACTION_CREATED_OLETX_EVENTID events. Event Listeners are unable to subscribe to this EventSource when there are events with the same ID.

I suspect it is acceptable for two methods to send events for the same ID, but they'll have different payloads. I don't know if there's any thing that can be done to resolve that for existing versions, but no one would have been able to listen to these events, so perhaps that's less of a problem. I chose to change the second event because it looks like it should have been 51 by the order of other events around it.

TransactionsEtwProvider incorrectly allocated the same event Id (11) to METHOD_ENTER_LTM_EVENTID and TRANSACTION_CREATED_OLETX_EVENTID events. 
Event Listeners are unable to subscribe to this EventSource when there are events with the same ID. 

I suspect it is acceptable for two methods to send events for the same ID, but they'll have different payloads. I don't know if there's any thing that can be done to resolve that for existing versions, but no one would have been able to listen to these events, so perhaps that's less of a problem. 
I chose to change the second event because it looks like it should have been 51 by the order of other events around it.
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 23, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @roji, @ajcvickers
See info in area-owners.md if you want to be subscribed.

@Camios

Camios commented Oct 23, 2024

Copy link
Copy Markdown
Contributor Author

#109101 (fixed link)

@roji

roji commented Oct 23, 2024

Copy link
Copy Markdown
Member

@Camios wouldn't changing this ID break any existing application listening to these events?

Not sure what the relevance of #10910 is here...

@martincostello

Copy link
Copy Markdown
Member

Should be a reference to #109101.

@Camios

Camios commented Oct 23, 2024

Copy link
Copy Markdown
Contributor Author

@roji

@Camios wouldn't changing this ID break any existing application listening to these events?

No, from my observations, when an EventListener calls EnableEvents on this EventSource it fails because the EventSource has two events with the same ID. So there can't be any EventListeners. You can see the failure details in #109101 (sorry I missed the last number when I posted the link earlier)

@KalleOlaviNiemitalo

Copy link
Copy Markdown

Does .NET Framework output these ETW events? If so, there might be software that consumes them, even though .NET Core has not been able to output them.

@Camios

Camios commented Oct 23, 2024

Copy link
Copy Markdown
Contributor Author

Does .NET Framework output these ETW events? If so, there might be software that consumes them, even though .NET Core has not been able to output them.

No. The .NET Framework version of Transactions doesn't use ETW and doesn't use numbers for event IDs. See https://referencesource.microsoft.com/#System.Transactions/System/Transactions/Trace/TraceRecords.cs,6cee693cd59558e9

Can anything technically listen to this event source if it has two events with the same key (we know EventListener-based classes can't)?
If yes, I'd like to know what/how, because that would be a workaround I'd like to explore in the meantime. But there's still the problem of two events with the same ID. The ID is supposed to be a unique key. Having two events the same is breaking a contract, that event listeners shouldn't be expected to deal with.
If not, then nothing can listen to events from this event source until this bug is fixed.

@KalleOlaviNiemitalo

Copy link
Copy Markdown

Can anything technically listen to this event source if it has two events with the same key (we know EventListener-based classes can't)?

ETW and EventPipe cannot listen. Those would go through EventSource.WriteEventWithRelatedActivityIdCore, which needs m_eventData, which would be initialized in CreateManifestAndDescriptors, but the exception "Event MethodEnterTraceLtm has ID 11 which is already in use." is thrown first and causes the initialization to be skipped.

@Camios

Camios commented Nov 12, 2024

Copy link
Copy Markdown
Contributor Author

Hi, how do I get this to progress - it has been sitting ready for review for three weeks?

@ThreePinkApples

Copy link
Copy Markdown

I've been struggling to debug some issues I have with TransactionScopes not completing, and came across this issue preventing me from listening to the transaction events. Is there any chance of this being merged soon?

@joshdean2000

Copy link
Copy Markdown

How do you go about getting this merged in? This is causing problems as it prevents all EventListeners from working if this EventSource is present.

@jeffhandley

Copy link
Copy Markdown
Member

This change is logical and clear. Sorry for the delay getting it merged; thank you for the contribution @Camios.

@jeffhandley
jeffhandley merged commit fa39f1c into dotnet:main May 15, 2025
@Camios
Camios deleted the patch-1 branch May 16, 2025 02:33
@Camios

Camios commented May 16, 2025

Copy link
Copy Markdown
Contributor Author

@jeffhandley thank you for advancing this! It was my first contribution. Where can I track this change to a future dotnet release?

@roji

roji commented May 16, 2025

Copy link
Copy Markdown
Member

@Camios this has been merged for the upcoming .NET 10 release.

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

Labels

area-System.Transactions community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants