(copy-pasted from meeting agenda, not formatted correctly. Will edit later.)
Filtering of dependencies from httpclient. 4 approaches to compare and discuss.
1 - Use Activity processor - OT spec approach, not so performant as allocations are still done.
2 - Use Sampler (special one wrapping existing samplers) - OT spec supports this, and this is better than above, as some allocations are saved.
3. Custom solution for HttpClientInstrumentation - can use the filter to DiagnosticSource.IsEnabled itself, ensuring the unwanted activity is blocked from ever being created. This will be a custom .NET solution, not derived from OT spec.
This already exists, but not exposed as public. #741 is proposing to make this public.
[From Mike B on options API: If we decide to modify the options, I would like to also expose a callback action for changing the DisplayName of the Http Activity. We used to have some of the Url on there, and then we changed it to just HttpMethod. The spec says we should use method but may allow for a hook to change that (ref). Sergey brought up on PR 741 possibly adding a callback to allow users to customize the tags. I combined those two things into one callback. Proposal:
/// <summary>
/// Gets or sets an optional callback method for filtering HttpClient requests that are sent through the instrumentation.
/// </summary>
public Func<HttpRequestMessage, bool> FilterFunc { get; set; }
/// <summary>
/// Gets or sets an optional callback method that will be invoked after HttpClient Activity objects are started.
/// Useful for adding non-standard tags for collection or changing the <see cref="Activity.DisplayName"/> to include the request url.
/// </summary>
public Action<HttpRequestMessage, Activity> StartAction { get; set; }]
4. Terminal Activity concept - this should solve the issue for a broader scope, not just http. For eg: If an exporter is simply writing to SQLServer, we don’t want to capture that as telemetry. Solution tied to httpclient won’t fix this.
[Note from Mike B on the above:
#4 is a slightly different solve than what is on the PR. Terminal activity (SpanKind.Terminal, ActivityKind.Terminal, etc) would be set by an exporter to flag its activity as internal to the execution of the OT mission, meaning, not-for-collection. For example, Zipkin exporter should be able to flag its Http transmission of span data as ignored from collection. Today these calls are filtered out in the logic found in HttpClientInstrumentationOptions (same logic being modified on the PR). This will be an on-going issue until we have a proper solve. For example, if you are using Stackdriver exporter, it transmits over gRPC using Google’s library. No issues today. But if someone comes along and instruments that gRPC library, then the transmit of spans creates a new span, which is transmitted, putting us in an infinite (potentially expensive) loop. You could of course build an ActivityProcessor to filter them, but the idea is to have some built-in way to handle this case automatically. Ideas have been to use SpanKind/ActivityKind or some kind of ambient context (like an AsyncLocal) running over the exporter activity that can be checked for exclusion by the instrumentation.
What the PR is seeking to do is give users the ability to filter out anything they don’t want captured, not just internal stuff. It’s a value-add, more or less. In my case, it is some appliance IT runs on our servers which emits Http calls to its agent out of our process. ActivityProcessor & ActivitySampler would work as solutions to this, but with the perf hit of creating an Activity you know you are going to drop/filter. Since there is already a filter executing in HttpClientInstrumentationOptions, it seemed like a good spot (easy to implement, non-perf impacting, perf-improving for filter case) to allow users to participate in the decision. ActivityProcessor or ActivitySampler would also work, but more effort to build than simply setting some options and you are hit with the perf of creating the Activity you don’t want.]
(copy-pasted from meeting agenda, not formatted correctly. Will edit later.)
Filtering of dependencies from httpclient. 4 approaches to compare and discuss.
1 - Use Activity processor - OT spec approach, not so performant as allocations are still done.
2 - Use Sampler (special one wrapping existing samplers) - OT spec supports this, and this is better than above, as some allocations are saved.
3. Custom solution for HttpClientInstrumentation - can use the filter to DiagnosticSource.IsEnabled itself, ensuring the unwanted activity is blocked from ever being created. This will be a custom .NET solution, not derived from OT spec.
This already exists, but not exposed as public. #741 is proposing to make this public.
[From Mike B on options API: If we decide to modify the options, I would like to also expose a callback action for changing the DisplayName of the Http Activity. We used to have some of the Url on there, and then we changed it to just HttpMethod. The spec says we should use method but may allow for a hook to change that (ref). Sergey brought up on PR 741 possibly adding a callback to allow users to customize the tags. I combined those two things into one callback. Proposal:
]
4. Terminal Activity concept - this should solve the issue for a broader scope, not just http. For eg: If an exporter is simply writing to SQLServer, we don’t want to capture that as telemetry. Solution tied to httpclient won’t fix this.
[Note from Mike B on the above:
#4 is a slightly different solve than what is on the PR. Terminal activity (SpanKind.Terminal, ActivityKind.Terminal, etc) would be set by an exporter to flag its activity as internal to the execution of the OT mission, meaning, not-for-collection. For example, Zipkin exporter should be able to flag its Http transmission of span data as ignored from collection. Today these calls are filtered out in the logic found in HttpClientInstrumentationOptions (same logic being modified on the PR). This will be an on-going issue until we have a proper solve. For example, if you are using Stackdriver exporter, it transmits over gRPC using Google’s library. No issues today. But if someone comes along and instruments that gRPC library, then the transmit of spans creates a new span, which is transmitted, putting us in an infinite (potentially expensive) loop. You could of course build an ActivityProcessor to filter them, but the idea is to have some built-in way to handle this case automatically. Ideas have been to use SpanKind/ActivityKind or some kind of ambient context (like an AsyncLocal) running over the exporter activity that can be checked for exclusion by the instrumentation.
What the PR is seeking to do is give users the ability to filter out anything they don’t want captured, not just internal stuff. It’s a value-add, more or less. In my case, it is some appliance IT runs on our servers which emits Http calls to its agent out of our process. ActivityProcessor & ActivitySampler would work as solutions to this, but with the perf hit of creating an Activity you know you are going to drop/filter. Since there is already a filter executing in HttpClientInstrumentationOptions, it seemed like a good spot (easy to implement, non-perf impacting, perf-improving for filter case) to allow users to participate in the decision. ActivityProcessor or ActivitySampler would also work, but more effort to build than simply setting some options and you are hit with the perf of creating the Activity you don’t want.]