[ASP.NET] Update enrich callbacks

[vishweshbankwar]

Fixes #766

Changes

Enrich Before

this.tracerProvider = Sdk.CreateTracerProviderBuilder()
    .AddAspNetInstrumentation((options) => options.Enrich
        = (activity, eventName, rawObject) =>
    {
        if (eventName.Equals("OnStartActivity"))
        {
            if (rawObject is HttpRequest httpRequest)
            {
                activity.SetTag("physicalPath", httpRequest.PhysicalPath);
            }
        }
        else if (eventName.Equals("OnStopActivity"))
        {
            if (rawObject is HttpResponse httpResponse)
            {
                activity.SetTag("responseType", httpResponse.ContentType);
            }
        }
    })

Enrich with this change

this.tracerProvider = Sdk.CreateTracerProviderBuilder()
    .AddAspNetInstrumentation(o =>
        {
            o.EnrichWithHttpRequest = (activity, httpRequest) =>
            {
                activity.SetTag("physicalPath", httpRequest.PhysicalPath);
            };
            o.EnrichWithHttpResponse = (activity, httpResponse) =>
            {
                activity.SetTag("responseType", httpResponse.ContentType);
            };
            o.EnrichWithException = (activity, exception) =>
            {
                activity.SetTag("exceptionType", exception.GetType().ToString());
            };
        })
    .Build();

For significant contributions please make sure you have completed the following items:

• Appropriate CHANGELOG.md updated for non-trivial changes
• Design discussion issue #

[codecov]

Codecov Report

Merging #940 (b5db90d) into main (3ca2b34) will increase coverage by 0.02%.
The diff coverage is 87.50%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #940      +/-   ##
==========================================
+ Coverage   69.40%   69.42%   +0.02%     
==========================================
  Files         202      202              
  Lines        7668     7667       -1     
==========================================
+ Hits         5322     5323       +1     
+ Misses       2346     2344       -2     

Impacted Files	Coverage Δ
...metry.Exporter.Instana/Implementation/Transport.cs	0.00% <0.00%> (ø)
.../OpenTelemetry.Exporter.Instana/InstanaExporter.cs	78.37% <100.00%> (-0.29%)	⬇️
...rumentation.AspNet/AspNetInstrumentationOptions.cs	100.00% <100.00%> (ø)
...umentation.AspNet/Implementation/HttpInListener.cs	88.31% <100.00%> (ø)

[CodeBlanch]

Random idea. Since we are making this change...

Should we consider switching to HttpContext or adding it to the signature?

Action<Activity, HttpContext>
or
Action<Activity, HttpContext, HttpRequest>

I seem to remember users asking for that way back. Off the top of my head the nice thing HttpContext gives you is the items bag so you can easily push stuff into your telemetry.

[vishweshbankwar]

if we pass in the Context, we could get rid of request. And if we do this, do we also change ASP.NET Core to match this?

[CodeBlanch]

if we pass in the Context, we could get rid of request.

Ya if you want to keep a single delegate we could probably get away with just Action<Activity, string, HttpContext, Exception?>. We could also introduce a contract that is expandable like...

public class AspNetInstrumentationOptions
{
   public AspNetEnrichAction Enrich {get; set;}
}

public delegate void AspNetInstrumentationEnrichAction(in AspNetEnrichArguments arguments);

public readonly struct AspNetInstrumentationEnrichArguments 
{
    public string EventName {get;}
    public Activity Activity {get;}
    public HttpContext HttpContext {get;}
    public Exception? Exception {get;}
}

And if we do this, do we also change ASP.NET Core to match this?

We could. Less of an issue there though because the ASP.NET Core request & response objects have a property that link back to the HttpContext. I kind of like that design with a dedicated delegate + arguments struct it feels cleaner? 🤷

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

This PR was marked stale due to lack of activity. It will be closed in 7 days.

[github-actions]

Closed as inactive. Feel free to reopen if this PR is still being worked on.