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

Adding only HttpClient instrumentation to ASP.NET Core app does not work #1779

Closed
alanwest opened this issue Sep 1, 2020 · 11 comments
Closed
Assignees
Labels
comp:extensions.enrichment.aspnetcore Things related to OpenTelemetry.Extensions.Enrichment.AspNetCore comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http documentation Improvements or additions to documentation

Comments

@alanwest
Copy link
Member

alanwest commented Sep 1, 2020

Bug Report

If you only add HttpClient instrumentation to an ASP.NET Core app, no spans from HttpClient are collected.

The issue is not specific to HttpClient instrumentation. I have also reproduced this when adding only Grpc.Net.Client instrumentation.

Reproduce

dotnet new webapi
dotnet add package --version 0.5.0-beta.2 OpenTelemetry.Instrumentation.Http
dotnet add package --version 0.5.0-beta.2 OpenTelemetry.Exporter.Console
dotnet add package --version 0.5.0-beta.2 OpenTelemetry.Instrumentation.AspNetCore

# Replace contents of Startup.cs with code below

dotnet run

# Navigate to https://localhost:5001/WeatherForecast
# This example uses the Console exporter. Observe no spans generated.
# Follow the instructions in the code comments to resolve the issue.
using System.Net.Http;
using Microsoft.AspNetCore.Builder;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using OpenTelemetry.Trace;

public class Startup
{
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddOpenTelemetryTracerProvider(builder => {
            builder
                // Uncomment the following line enabling AspNetCore instrumentation
                // This will result in seeing a span for both AspNetCore requests and the HttpClient invocation
                // .AddAspNetCoreInstrumentation()
                .AddHttpClientInstrumentation()
                .AddConsoleExporter();
        });

        services.AddControllers();
    }

    public void Configure(IApplicationBuilder app)
    {
        app.Use(async (context, next) =>
        {
            await next.Invoke();
            using var client = new HttpClient();
            await client.GetStringAsync("https://opentelemetry.io");
        });

        app.UseRouting();

        app.UseEndpoints(endpoints =>
        {
            endpoints.MapControllers();
        });
    }
}

Update

See my analysis below for the reason why this occurs. Some thoughts come to my mind on how best to address this issue.

  1. We could document this behavior and note that ASP.NET Core instrumentation is required.
  2. We could also document any potential workarounds. For instance, using a different sampler rather than the default ParentBasedSampler.
  3. Though, if we want to pursue making this work as is, then I think we need to devise a way to ignore the Activity created by ASP.NET Core in sampling decisions when OpenTelemetry has not been configured to use ASP.NET Core instrumentation.
@alanwest
Copy link
Member Author

alanwest commented Sep 4, 2020

Did some investigation, the reason the HttpClient span does not show up without adding the ASP.NET Core instrumentation is because the ParentBasedSampler always makes a NotRecord decision.

  1. It does have a parent (the ASP.NET Core HTTP IN activity), so parentContext.SpanId != default here
    https://github.com/open-telemetry/opentelemetry-dotnet/blob/32fb7c12f1269147bea1080f04ea4ba489aa0f63/src/OpenTelemetry/Trace/ParentBasedSampler.cs#L43-L49

  2. The parent's TraceFlags do not indicate Recorded here
    https://github.com/open-telemetry/opentelemetry-dotnet/blob/32fb7c12f1269147bea1080f04ea4ba489aa0f63/src/OpenTelemetry/Trace/ParentBasedSampler.cs#L51-L55

  3. So the sampling decision always falls through to here
    https://github.com/open-telemetry/opentelemetry-dotnet/blob/32fb7c12f1269147bea1080f04ea4ba489aa0f63/src/OpenTelemetry/Trace/ParentBasedSampler.cs#L69-L70


I guess this behavior is expected, but it does seem a bit unintuitive. That is, as a user I explicitly decide I only want HttpClient instrumentation, but that does not prevent ASP.NET Core from creating activities which will often be the root activity in a trace and influence downstream sampling decisions.

I suppose a work around would be to not use the ParentBasedSampler...

I also can see an argument that it is an unlikely scenario that someone wants to use OpenTelemetry with their ASP.NET Core and not enable ASP.NET Core instrumentation.

@alanwest alanwest changed the title TracerProvider does not always get initialized when expected Adding only HttpClient instrumentation to ASP.NET Core app does not work Sep 4, 2020
@cijothomas
Copy link
Member

Same issue reported differently:
As reported in gitter channel this is an issue if you try to start own activity inside an AspNetCore application without enabling the AspNetCoreInstrumentation.

@cijothomas
Copy link
Member

@alanwest The root of the issue is some libraries (eg: Asp.Net Core) is creating activity irrespective of whether Otel asked is to do so. This is a "by design" behaviour for Asp.Net Core. Will try to see if Asp.Net Core has plans to disable this feature by default.

@cijothomas cijothomas self-assigned this Oct 22, 2020
@cijothomas
Copy link
Member

The best we can do is to document this behaviour. Tagging this as a documentation issue.

@cijothomas cijothomas added documentation Improvements or additions to documentation and removed bug Something isn't working labels Nov 6, 2020
@mitoihs
Copy link

mitoihs commented Jan 14, 2021

I've just hit that issue. I just need to trace my custom Activities (effectively I have AddSource in place of AddHttpClientInstrumentation of the example) and don't care about AspNetCoreInstrumentation but still, I can't turn it off or nothing is logged. I've tried to turn it on and filter everything out (.AddAspNetCoreInstrumentation(c => c.Filter = ctx => false)) but it's not working in that case also. That's sad. Is there a way to "turn on" tracing in ASP .NET Core without listening to "built-in" activities?

@cijothomas
Copy link
Member

@mitoihs You can use a diff. sampler other than the default ParentBased sampler. This should give you the required solution for you.

@CodeBlanch
Copy link
Member

@cartersocha and I just ran into this too.

In a demo we have something like...

builder.Services.AddOpenTelemetryTracing(options =>
{
    options
        .SetResourceBuilder(resourceBuilder)
        .AddSource("MyCustomSource")  // Collect all traces from "MyCustomSource"
        //.AddAspNetCoreInstrumentation() // Not enabling AspNetCore instrumentation
        .AddConsoleExporter();
});

        public Task<SupportCase?> GetAsync(string id)
        {
            using Activity? activity = MyCustomSource.StartActivity(nameof(GetAsync), ActivityKind.Client); // <- Returns null Activity

            if (activity?.IsAllDataRequested == true)
            {
                activity.SetTag("entityId", id);
            }

            // Some logic here.
        }

The Activity from MyCustomSource is always null because the parent Activity created by AspNetCore has Recorded = false.

Just documenting this, not sure if we should/could do anything about it.

@cijothomas
Copy link
Member

The demos could default to AlwaysOnSampler ?

@CodeBlanch
Copy link
Member

In the actual demo AddAspNetCoreInstrumentation is added so it actually works fine. We were just messing with it to capture some screenshots and disabled AddAspNetCoreInstrumentation temporarily thinking that would cause just the custom stuff to show in the console when in reality it caused everything to disappear 😆

@martinjt
Copy link
Member

I don't think we're going to do anything with this @alanwest @CodeBlanch ? if not, we should close this as not planned rather than keeping it open.

@vishweshbankwar vishweshbankwar transferred this issue from open-telemetry/opentelemetry-dotnet May 14, 2024
@Kielek Kielek added comp:extensions.enrichment.aspnetcore Things related to OpenTelemetry.Extensions.Enrichment.AspNetCore comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http labels May 17, 2024
@martinjt martinjt closed this as not planned Won't fix, can't repro, duplicate, stale Jun 2, 2024
@cijothomas
Copy link
Member

I don't think we're going to do anything with this @alanwest @CodeBlanch ? if not, we should close this as not planned rather than keeping it open.

We haven't done the documentation to call this out : #1779 (comment)

Maybe worth keeping it until someone helps add the documentation. (If we go native instrumentation route, then also this could be an issue?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:extensions.enrichment.aspnetcore Things related to OpenTelemetry.Extensions.Enrichment.AspNetCore comp:instrumentation.http Things related to OpenTelemetry.Instrumentation.Http documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

6 participants