Skip to content

Allow additional Tags for activity creation #50488

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.

Sign up for GitHub

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

Jump to bottom
Open
1 task done
martinjt opened this issue Sep 2, 2023 · 3 comments · May be fixed by #62090
Open
1 task done

Allow additional Tags for activity creation #50488

martinjt opened this issue Sep 2, 2023 · 3 comments · May be fixed by #62090
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Comments

@martinjt
Copy link

martinjt commented Sep 2, 2023 •
edited by danroth27
Loading

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

When doing sampling with Activity/Traces, the information that you have to make the decision on whether to Sample or Drop is only that which is passed in as tags to Start/CreateActivity method. Sampling at the Head (root activity) is the most efficient method as it allows child activitys to inherit that decision and therefore reduce processing. You can sample at an individual (child) activity level too.

Right now, there is very little information to go on when trying to apply Head Sampling in a .NET application. This is because of the Activity being created having no tags being used as part of the Create.

aspnetcore/src/Hosting/Hosting/src/Internal/HostingApplicationDiagnostics.cs

Line 389 in 2f7a3f7

activity = _activitySource.CreateActivity(ActivityName, ActivityKind.Server, string.IsNullOrEmpty(requestId) ? null! : requestId);

This means that the samplers can only base their decision on the parent context.

In order to use other contextual information, you need to use something like the IHttpContextAccessor to access the ambient context.

Describe the solution you'd like

What would make this better from a consumer perspective would be an extension point to augment the activity tags at creation time.

I don't know if Features for HTTP are available to get during the pipeline, however, something like the following

interface IHttpActivityEnrichmentFeature
{
   ActivityTagsCollection OnCreate(HttpContext context);
   void PostCreate(Activity activity, HttpContext context);
}

This would allow users to do that enrichment at the right time. It would allow the level of data being used to be configured according to the performance concerns of the user.

Additional context

No response
@ghost ghost added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Sep 2, 2023
@gfoidl gfoidl added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Sep 4, 2023
@martinjt martinjt mentioned this issue Sep 6, 2023
Open
@amcasey amcasey mentioned this issue Dec 1, 2023
Open
1 task
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@roycald245 roycald245 mentioned this issue Aug 12, 2024
Open
1 task
@joegoldman2
Copy link
Contributor

What do you think about renaming the two methods to ActivityCreating and ActivityCreated?

/// <summary>
/// Defines a feature to enrich activities with additional tags.
/// </summary>
public interface IHttpActivityEnrichmentFeature
{
    /// <summary>
    /// Called during the creation of an activity.
    /// </summary>
    /// <param name="context">The current HTTP context.</param>
    ActivityTagsCollection ActivityCreating(HttpContext context);

    /// <summary>
    /// Called after an activity has been created to perform further enrichment.
    /// </summary>
    /// <param name="activity">The created activity.</param>
    /// <param name="context">The current HTTP context.</param>
    void ActivityCreated(Activity activity, HttpContext context);
}

This convention is already used in some places like https://github.com/dotnet/efcore/blob/main/src/EFCore.Relational/Diagnostics/IDbConnectionInterceptor.cs.

@rkargMsft
Copy link

Taking a look at this.
Ideally, if something like the host or method were added at Create time then it wouldn't also be added again later after an Activity passes the sampling check but I don't see a clean way to avoid that duplicate work right now.

Maybe that extra work of setting it twice, only for the sampled Activity case, isn't that big of a deal?

@rkargMsft rkargMsft linked a pull request May 23, 2025 that will close this issue
Open
4 tasks
@rkargMsft
Copy link

Adding feature as a hook for users to be able to specify what tags should be added on creation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Feature for setting activity tags on creation rkargMsft/aspnetcore
5 participants
@martinjt @gfoidl @wtgodbe @joegoldman2 @rkargMsft