Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filtering topmost Activities via Processor is not respected by Sentry Exporter #3887

Closed
CezaryKlus opened this issue Jan 13, 2025 · 4 comments · Fixed by #3890
Closed

Filtering topmost Activities via Processor is not respected by Sentry Exporter #3887

CezaryKlus opened this issue Jan 13, 2025 · 4 comments · Fixed by #3890
Labels
Bug Something isn't working

Comments

@CezaryKlus
Copy link

CezaryKlus commented Jan 13, 2025

Package

Sentry

.NET Flavor

.NET

.NET Version

8.0.0

OS

Linux

SDK Version

5.0.1

Self-Hosted Sentry Version

No response

Steps to Reproduce

  1. Add filtering Processor, here an example of filtering out activities that do not belong to an ancestor that is an incoming Request or Message Consumer, or Hangfire Job (most often topmost)
internal sealed class FilteringProcessor : BaseProcessor<Activity>
{
    private static bool HasParent(Activity activity, Func<Activity, bool> condition)
    {
        Activity? current = activity;

        while (current != null)
        {
            if (condition(current))
            {
                return true;
            }
            current = current.Parent;
        }

        return false;
    }

    // Called when an activity ends
    // Filters out activities that are not coming from ASP.NET Core request, MassTransit or Hangfire job
    public override void OnEnd(Activity activity)
    {
        var activityKinds = new ActivityKind[] { ActivityKind.Server, ActivityKind.Consumer };

        // If the activity does not have a parent that matches the specified kinds or tags, mark it as not recorded
        if (!HasParent(activity, a =>
            activityKinds.Any(y => a.Kind == y)
            || a.Tags.Any(y => y.Key == "job.id")))
        {
            Console.WriteLine($"Filtering out activity: {activity.OperationName} {activity.DisplayName}");
            activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded;
            activity.IsAllDataRequested = false;
        }
    }
}
  1. Register filtering Processor
 services.AddOpenTelemetry()
     .ConfigureResource(resource => resource
         .AddService(serviceName: ctx.HostingEnvironment.ApplicationName, serviceVersion: ServiceVersion.FromTypeAssembly<TStartup>())
         .AddHostDetector()
         )
     .WithTracing(tracerProviderBuilder =>
         tracerProviderBuilder
             .AddAspNetCoreInstrumentation(o =>
             {
                 // filter out health checks
                 o.Filter = (httpContext) => !httpContext.Request.Path.ToString().Contains(".probe", StringComparison.InvariantCultureIgnoreCase);
                 o.RecordException = true;
             }) // <-- Adds ASP.NET Core telemetry sources
             .AddHttpClientInstrumentation(
                 // https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/samples/Diagnostics.md#filtering-out-duplicated-http-client-activities
                 o =>
                 {
                     o.FilterHttpRequestMessage = (_) => Activity.Current?.Parent?.Source?.Name != "Azure.Core.Http";
                     o.RecordException = true;
                 }) // <-- Adds HttpClient telemetry sources
             .AddSqlClientInstrumentation(o =>
             {
                 o.SetDbStatementForText = true;
                 o.RecordException = true;
             }) // <-- Adds SQL Client telemetry sources
             .AddHangfireInstrumentation(o =>
             {
                 o.RecordException = true;
             }) // https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/OpenTelemetry.Instrumentation.Hangfire
             .AddSource("Azure.*") // <-- Adds Azure telemetry sources https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/samples/Diagnostics.md#opentelemetry-configuration
             .AddSource(MassTransit.Logging.DiagnosticHeaders.DefaultListenerName) // https://masstransit.io/documentation/configuration/observability#aspnet-core-application


             .AddProcessor(new FilteringProcessor()) // <-- Adds a processor to filter out unwanted activities
             .AddSentry() // <-- Configure OpenTelemetry to send trace information to Sentry
     );
  1. Observe Filtering out activity debug messages in the console
  2. Observe Sentry still reports these topmost Activities

Expected Result

Sentry Exporter respects the ActivityTraceFlags and is not transmitting topmost Activities that are flagged as not Recorded

Actual Result

Sentry still reports these filtered Activities

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 13, 2025
@CezaryKlus CezaryKlus changed the title Filtering Activities via Process is not respected by Sentry Exporter Filtering Activities via Processor is not respected by Sentry Exporter Jan 13, 2025
@CezaryKlus
Copy link
Author

Possibly SentrySpanProcessor is always creating Transaction even though it does so for an Activity marked as not Recorded

@jamescrosswell
Copy link
Collaborator

Hi @CezaryKlus ,

Thanks for the detailed report.

What you're saying makes sense, and yet I'm not able to reproduce this.

I've got a much simpler filter than yours:

internal sealed class FilteringProcessor : BaseProcessor<Activity>
{
    // Filters out some activities
    public override void OnEnd(Activity activity)
    {
        // If the activity does not have a parent that matches the specified kinds or tags, mark it as not recorded
        if (activity.Tags.Any(t => t is { Key: "Answer", Value: "unknown" }))
        {
            Console.WriteLine($"Filtering out activity: {activity.OperationName} {activity.DisplayName}");
            activity.ActivityTraceFlags &= ~ActivityTraceFlags.Recorded;
            activity.IsAllDataRequested = false;
        }
    }
}

And I'm using this in the Otel sample that we provide:

using (var activity = activitySource.StartActivity("Main"))
{
    // This creates a span called "Task 1" within the transaction
    using (var task = activitySource.StartActivity("Task 1"))
    {
        task?.SetTag("Answer", 42);
        Thread.Sleep(100); // simulate some work
        Console.WriteLine("Task 1 completed");
        task?.SetStatus(Status.Ok);
    }

    // This creates a span called "Task 2" within the transaction
    using (var task = activitySource.StartActivity("Task 2"))
    {
        task?.SetTag("Answer", "unknown");
        Thread.Sleep(100); // simulate some work
        Console.WriteLine("Task 2 completed");
        task?.SetStatus(Status.Ok);
    }

    // Since we use `AddHttpClientInstrumentation` when initializing OpenTelemetry, the following Http request will also
    // be captured as a Sentry span
    var httpClient = new HttpClient();
    var html = await httpClient.GetStringAsync("https://example.com/");
    Console.WriteLine(html);
}

The filtered trace is correctly being removed from the traceview in Sentry:

Image

Or are you not using Sentry to view the resulting traces? Are you viewing the traces somewhere else?

@CezaryKlus
Copy link
Author

CezaryKlus commented Jan 14, 2025

@jamescrosswell thanks for the feedback. I should have been more precise. I am trying to filter out the Activities coming from some background ASP.NET Core processes that are not related to e.g. any HTTP request.
So these Activities are topmost. Example from the Traces view:

Image

And these are definitely the ones that I filter.

So maybe in your example, the attempt to reproduce would be to try to filter out the Main activity.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jan 14, 2025
@CezaryKlus CezaryKlus changed the title Filtering Activities via Processor is not respected by Sentry Exporter Filtering topmost Activities via Processor is not respected by Sentry Exporter Jan 14, 2025
@jamescrosswell
Copy link
Collaborator

Hi @CezaryKlus,

OK, that I can reproduce. We can tweak the SentrySpanProcessor to filter these spans out.

As a workaround, in the short term, you could use Sentry's BeforeSendTransaction hook to prevent these transactions from being sent to Sentry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
Status: Done
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants