Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions src/Sentry.Profiling/SampleProfilerSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ namespace Sentry.Profiling;
internal class SampleProfilerSession : IDisposable
{
private readonly EventPipeSession _session;
private readonly TraceLogEventSource _eventSource;
private readonly SampleProfilerTraceEventParser _sampleEventParser;
private readonly IDiagnosticLogger? _logger;
private readonly SentryStopwatch _stopwatch;
Expand All @@ -22,21 +21,21 @@ private SampleProfilerSession(SentryStopwatch stopwatch, EventPipeSession sessio
{
_session = session;
_logger = logger;
_eventSource = eventSource;
_sampleEventParser = new SampleProfilerTraceEventParser(_eventSource);
EventSource = eventSource;
_sampleEventParser = new SampleProfilerTraceEventParser(EventSource);
_stopwatch = stopwatch;
}

// Exposed only for benchmarks.
internal static EventPipeProvider[] Providers = new[]
{
// Note: all events we need issued by "DotNETRuntime" provider are at "EventLevel.Informational"
// see https://learn.microsoft.com/en-us/dotnet/fundamentals/diagnostics/runtime-events
// TODO replace Keywords.Default with a subset. Currently it is:
// Default = GC | Type | GCHeapSurvivalAndMovement | Binder | Loader | Jit | NGen | SupressNGen
// | StopEnumeration | Security | AppDomainResourceManagement | Exception | Threading | Contention | Stack | JittedMethodILToNativeMap
// | ThreadTransfer | GCHeapAndTypeNames | Codesymbols | Compilation,
new EventPipeProvider(ClrTraceEventParser.ProviderName, EventLevel.Informational, (long) ClrTraceEventParser.Keywords.Default),
new EventPipeProvider(ClrTraceEventParser.ProviderName, EventLevel.Verbose, (long) (
Copy link
Contributor Author

@vaind vaind Feb 6, 2025

Choose a reason for hiding this comment

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

EventLevel.Verbose is the actual fix here. The rest of the change is just an improvement so we don't receive things we don't need.

ClrTraceEventParser.Keywords.Jit
| ClrTraceEventParser.Keywords.NGen
| ClrTraceEventParser.Keywords.Loader
| ClrTraceEventParser.Keywords.Binder
| ClrTraceEventParser.Keywords.JittedMethodILToNativeMap
)),
new EventPipeProvider(SampleProfilerTraceEventParser.ProviderName, EventLevel.Informational),
// new EventPipeProvider(TplEtwProviderTraceEventParser.ProviderName, EventLevel.Informational, (long) TplEtwProviderTraceEventParser.Keywords.Default)
};
Expand All @@ -46,11 +45,14 @@ private SampleProfilerSession(SentryStopwatch stopwatch, EventPipeSession sessio
// need a large buffer if we're connecting righ away. Leaving it too large increases app memory usage.
internal static int CircularBufferMB = 16;

// Exposed for tests
internal TraceLogEventSource EventSource { get; }

public SampleProfilerTraceEventParser SampleEventParser => _sampleEventParser;

public TimeSpan Elapsed => _stopwatch.Elapsed;

public TraceLog TraceLog => _eventSource.TraceLog;
public TraceLog TraceLog => EventSource.TraceLog;

// default is false, set 1 for true.
private static int _throwOnNextStartupForTests = 0;
Expand Down Expand Up @@ -108,15 +110,15 @@ public async Task WaitForFirstEventAsync(CancellationToken cancellationToken = d
{
var tcs = new TaskCompletionSource();
var cb = (TraceEvent _) => { tcs.TrySetResult(); };
_eventSource.AllEvents += cb;
EventSource.AllEvents += cb;
try
{
// Wait for the first event to be processed.
await tcs.Task.WaitAsync(cancellationToken).ConfigureAwait(false);
}
finally
{
_eventSource.AllEvents -= cb;
EventSource.AllEvents -= cb;
}
}

Expand All @@ -128,8 +130,8 @@ public void Stop()
{
_stopped = true;
_session.Stop();
EventSource.Dispose();
_session.Dispose();
_eventSource.Dispose();
}
catch (Exception ex)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.IO.Abstractions.TestingHelpers;
using Microsoft.Diagnostics.Tracing;
using Microsoft.Diagnostics.Tracing.Parsers.Clr;
using Sentry.Internal.Http;

namespace Sentry.Profiling.Tests;
Expand Down Expand Up @@ -180,6 +181,41 @@ public async Task Profiler_AfterTimeout_Stops()
}
}

[SkippableFact]
public async Task EventPipeSession_ReceivesExpectedCLREvents()
{
SampleProfilerSession? session = null;
SkipIfFailsInCI(() => session = SampleProfilerSession.StartNew(_testOutputLogger));
using (session)
{
var eventsReceived = new HashSet<string>();
session!.EventSource.Clr.All += (TraceEvent ev) => eventsReceived.Add(ev.EventName);

var loadedMethods = new HashSet<string>();
session!.EventSource.Clr.MethodLoadVerbose += (MethodLoadUnloadVerboseTraceData ev) => loadedMethods.Add(ev.MethodName);


await session.WaitForFirstEventAsync(CancellationToken.None);
var limitMs = 50;
var sut = new SamplingTransactionProfiler(_testSentryOptions, session, limitMs, CancellationToken.None);
RunForMs(limitMs * 5);
MethodToBeLoaded(100);
RunForMs(limitMs * 5);
sut.Finish();

Assert.Contains("Method/LoadVerbose", eventsReceived);
Assert.Contains("Method/ILToNativeMap", eventsReceived);
Assert.Contains("Method/UnloadVerbose", eventsReceived);

Assert.Contains("MethodToBeLoaded", loadedMethods);
}
}

private static long MethodToBeLoaded(int n)
{
return -n;
}

[SkippableTheory]
[InlineData(true)]
[InlineData(false)]
Expand Down
Loading