Repository navigation
[tvOS] Test failures in System.Diagnostics.Tracing.Tests #56073
Description
Activity
- ghost addeduntriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner
on Jul 21, 2021 Those tests have been added recently in #55625. They are skipped for WASM, I'm not sure if we want to do the same for Apple mobile platforms.
@MaximLipnin Can you check to see what the value is for
<EventSourceSupport>?@steveisok Something close to the mobile targets is https://github.com/dotnet/runtime/blob/main/eng/testing/tests.mobile.targets#L26 but we don't set EAT for the staging lanes so perhaps
EventSourceSupportis not setDo we build with diagnostics tracing component support enabled when running tests on mobile, link or deploy needed components? I guess this tests will end up in ves_icall_System_Diagnostics_Tracing_EventPipeInternal_EventActivityIdControl and if we don't have component support loaded that will be a nop operation so won't set thread activity ID and that will trigger the assert in this test.
I don't think we do. We probably should skip these for the time being.
- addedos-iosApple iOSApple iOSand removeduntriagedNew issue has not been triaged by the area ownerNew issue has not been triaged by the area owner
on Jul 21, 2021 8 remaining items
- added a commit that references this issue
on Dec 5, 2022 I'm trying to make this work on browser now.
And I'm not clear how this could work on Mono. I can't find what's callingActivityTracker.Instance.Enable()on Mono@pavelsavara If EventSource is being initialized, it enables an ActivityTracker instance, so whenever events are being written to EventPipe on Mono, OnStart could be called from WriteEventVarArgs or WriteEventWithRelatedActivityIdCore and that could be the things calling
ActivityTracker.Instance.Enable()enables an ActivityTracker instance
Yes, but that's just instance, but not
Enable()andActivityTracker.OnStartwould enable it only if you haveTplEventSourceenabled, right ?runtime/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ActivityTracker.cs
Lines 47 to 59 in 9c9178f
if (m_current == null) // We are not enabled { // We used to rely on the TPL provider turning us on, but that has the disadvantage that you don't get Start-Stop tracking // until you use Tasks for the first time (which you may never do). Thus we change it to pull rather tan push for whether // we are enabled. if (m_checkedForEnable) return; m_checkedForEnable = true; if (useTplSource && TplEventSource.Log.IsEnabled(EventLevel.Informational, TplEventSource.Keywords.TasksFlowActivityIds)) Enable(); if (m_current == null) return; } Right. I think what's happening is the test has a custom EventSource/EventListener that looks for TplEventSource being created to enable that provider. Then later in these tests
SetCurrentActivityIdBeforeEventFlowsAsyncandSetCurrentActivityIdAfterEventDoesNotFlowAsync, they callSetCurrentThreadActivityIdwhich then instantiatesTplEventSource.Log.Reacted by Pavel SavaraThanks
they call
SetCurrentThreadActivityIdwhich then instantiatesTplEventSource.LogThat solves how it works in the unit test, but it would not make it work in production, unless user code also enables
System.Threading.Tasks.TplEventSourcein theOnEventSourceCreated. That sounds fishy.It seems to me that CoreCLR does it always when
FEATURE_EVENT_TRACEis enabled. That's always true, right ?FireAssemblyLoadStart->ActivityTracker::Start->AssemblyLoadContext.StartAssemblyLoad->ActivityTracker.Instance.Enable()Should we do
ActivityTracker.Instance.Enable()fist time that anyEventListeneris created ?Or do something Mono specific when ? Maybe any time that we link
libmono-component-diagnostics_tracing-static.lib?cc @lewing
Yeah, I think you're right that CoreCLR will enable ActivityTracker by default, but I don't know if Mono also should. If Browser needs ActivityTracking, I think we could try turning it on in 10. @lateralusX, do you happen to know if ActivityTracker was not on by default on Mono for a particular reason?
The trigger in CoreCLR is when its raising FireEtwAssemblyLoadStart/FireEtwAssemblyLoadStop event pairs (and there is a listener registering for those events):
void FireAssemblyLoadStart(const BinderTracing::AssemblyBindOperation::BindRequest &request) That will call into ActivityTracker start/stop that will end up in the managed call that will enable ActivityTracker.Instance.Enable(). The other option is to enable specific keyword on the TPLEventSource. Those are the only two scenarios actively supporting activity tracking and since activity tracking comes with some overhead it only gets enabled when really used.
Mono never ported the specific assembly start/stop events, probably since tools at that point didn't consume them, we just emit FireEtwModuleLoad/FireEtwModuleUnload and FireEtwAssemblyLoad/FireEtwAssemblyUnload. Since Mono don't support the assembly loader events that uses activity tracking, it will only enable it in the scenario that we support, when using TPLEventSource listener with TasksFlowActivityIds keyword.
If we decide to support FireEtwAssemblyLoadStart/FireEtwAssemblyLoadStop then we would enable it in similar way as CoreClr does.
Reacted by Pavel Savara and Mitchell HwangSo the question is when wasm/browser should enable activity tracking.
It seems to me that the "activity" is allocated any time when
EventSourcemethod withStartandStopsuffix is called.The interesting example is
class HttpTelemetrywithRequestStart,RequestStop,RequestHeadersStart,RequestHeadersStop,RequestContentStart,RequestContentStop,ResponseHeadersStart,ResponseHeadersStop,ResponseContentStartandResponseContentStop. There are more events for other OS where HTTP is on top of socket.All of this works when
DiagnosticsHandleris enabled. ViaSystem.Net.Http.EnableActivityPropagationviaHttpActivityPropagationSupportwhich isfalsefor browser.Also maybe
MetricsHandlerwhen is enabled ?
Which at the moment is not guarded, but I think it should be behind<MetricsSupport>&System.Diagnostics.Metrics.Meter.IsSupported. And that isfalseby default on browser.since activity tracking comes with some overhead it only gets enabled when really used.
The
HttpTelemetrynorMetricsHandlernorDiagnosticsHandlerdoesn't call theActivityTracker.Instance.Enable()at the moment.
Should it do that ?What other EventSources should ?
cc @steveisok