Feature for setting activity tags on creation

[rkargMsft]

Feature for adding tags at Activity creation time

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes

Adding a feature so that implementations can specify tags to add at Activity creation. This is to support using those tags when making sampling decisions as only tags provided in the CreateActivity/StartActivity are available at that time.

Description

Implementations of IHttpActivityCreationTagsFeature can determine what tags are desired to be used for sampling. It's important that this is an opt-in where the user specifies the tags to add since this work will get done for every Activity created for an incoming request and not just for the ones that eventually pass the sampling filter.

Example implementation:

public class BasicHttpActivityCreationTagsFeature : IHttpActivityCreationTagsFeature
{
    private readonly HttpContext _context;

    public BasicHttpActivityCreationTagsFeature(HttpContext context)
    {
        ArgumentNullException.ThrowIfNull(context);
        _context = context;
    }

    public IEnumerable<KeyValuePair<string, object?>>? ActivityCreationTags
    {
        get
        {
            if (_context?.Request is null)
            {
                return null;
            }

            var tags = new TagList();
            if (_context.Request.Host.HasValue)
            {
                tags.Add("server.address", _context.Request.Host.Host);
                if (_context.Request.Host.Port is not null)
                {
                    tags.Add("server.port", _context.Request.Host.Port);
                }
            }
            tags.Add("http.request.method", _context.Request.Method);
            tags.Add("url.scheme", _context.Request.Scheme);
            return tags;
        }
    }
}

Fixes #50488

[rkargMsft]

It may make sense to actually have the ActivityCreationTags property be IEnumerable<KeyValuePair<string, object?>>? which is what ActivityCreator takes as a parameter and then let the implementation decide what to return.

aspnetcore/src/Shared/Diagnostics/ActivityCreator.cs

Lines 13 to 22 in 6ab9fa6

	public static Activity? CreateFromRemote(
	ActivitySource activitySource,
	DistributedContextPropagator propagator,
	object distributedContextCarrier,
	DistributedContextPropagator.PropagatorGetterCallback propagatorGetter,
	string activityName,
	ActivityKind kind,
	IEnumerable<KeyValuePair<string, object?>>? tags,
	IEnumerable<ActivityLink>? links,
	bool diagnosticsOrLoggingEnabled)

[JamesNK]

What's the scenario where you want to set activity tags on creation? This seems like kind of niche feature and I'd prefer to see whether the problem can be solved with existing APIs.

For example, there is a feature to get the current request activity, IHttpActivityFeature, and there are many events during the corse of a HTTP request. Could you use one of those existing events, get the activity using IHttpActivityFeature and then set the tags?

[JamesNK]

This is to support using those tags when making sampling decisions as only tags provided in the CreateActivity/StartActivity are available at that time.

Is the scenario that you set a feature with tags earlier in the request. Tags could be different depending on the request.

An activity is created very early in a request so the number of places to set it would be limited. Can you give an example of this being used? Also, what does the activity source do with the tags?

[rkargMsft]

I'm specifically using YARP as a reverse proxy for a large number of services. There are calls for many different host names. I want to make tracing sampling decision for those calls and would like to have separate buckets for each hostname. For example, in a given time window, I want a minimum number of traces to always be sampled and then start doing probabilistic sampling after that.

As it is now, I can only get the base operation name (just HTTP <METHOD> at the time of sampling) and no tags. This means that if I have one very high throughput service and one very low throughput service then they're all in the same bucket for sampling (there's not host name or other data in tags yet to differentiate requests) and probabilistically, the low throughput service basically never gets sampled.

The only way to get tags to be available at the time of making the sampling decision is for them to be passed in to the CreateActivity or StartActivity call and the existing code hard codes that to tags:null.

The flow is:

• Create/Start Activity
  • Sampling logic determines if an Activity is returned or null based on only the parameters passed in on Create/Start
• If non-null (because sampling decided to keep the span) then additional tags or modifications can be made

I don't know of any other way to get information available to the sampler than to pass the tags in on Create/StartActivity.

This is extra work for every request, even those that end up not sampled, which is why it does nothing by default and would require a user to specifically opt in with their own implementation for specifically the data points they need in their particular scenario. For example, the vast majority of our services would not use this functionality; only our YARP usage would define an implementation to just add the host/server.address tag.

[rkargMsft]

The OpenTelemetry spec has this SHOULD requirement for tags available at span creation time:

The following attributes can be important for making sampling decisions and SHOULD be provided at span creation time (if provided at all):

http.request.method
server.address
server.port
url.full

It's not free to add those so making it possible but explicitly opt-in seems like a good change.

[JamesNK]

Right now tracing in ASP.NET Core doesn't follow the OTEL standard. When we do then those tags should be set immediately. It sounds like server address and port are what you want to use when making sampling decisions, right?

The performance cost shouldn't be a concern. We would only call ActivitySource if someone is listening to the activity, so people who care about perf and aren't listening won't be impacted.

[rkargMsft]

For our specific use case we would only use server.address.

Performance impact of this change would be initializing the feature if it's registered and doing the feature lookup if there's a listener:

Listener	Feature Registered	Change
No	No	-
Yes	No	Feature lookup
No	Yes	Feature initialization logic (implementation specific)
Yes	Yes	Feature initialization logic (implementation specific) + Feature lookup