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

Allow additional Tags for activity creation #50488

Open
1 task done
martinjt opened this issue Sep 2, 2023 · 1 comment
Open
1 task done

Allow additional Tags for activity creation #50488

martinjt opened this issue Sep 2, 2023 · 1 comment
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

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 of 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.

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

@dotnet-issue-labeler dotnet-issue-labeler bot 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
@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
@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

No branches or pull requests

4 participants