Examples: Fix ParentProvider not being set on MyFilteringProcessor

[CodeBlanch]

Fixes #3241

Changes

Uses CompositeProcessor<Activity> so that ParentProvider is correctly set on the inner processor for the filtering example.

Requires #3368 (comment) to fully work.

[codecov]

Codecov Report

Merging #3370 (5954cfa) into main (a58c7a3) will increase coverage by 0.16%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #3370      +/-   ##
==========================================
+ Coverage   85.87%   86.04%   +0.16%     
==========================================
  Files         268      268              
  Lines        9460     9460              
==========================================
+ Hits         8124     8140      +16     
+ Misses       1336     1320      -16     

Impacted Files	Coverage Δ
...emetry.Api/Internal/OpenTelemetryApiEventSource.cs	73.52% <0.00%> (-5.89%)	⬇️
...Telemetry/Internal/SelfDiagnosticsEventListener.cs	96.87% <0.00%> (-0.79%)	⬇️
...ter.ZPages/Implementation/ZPagesActivityTracker.cs	100.00% <0.00%> (+2.85%)	⬆️
...metryProtocol/Implementation/ActivityExtensions.cs	95.60% <0.00%> (+4.39%)	⬆️
...tation/OpenTelemetryProtocolExporterEventSource.cs	85.00% <0.00%> (+10.00%)	⬆️
...xporter.OpenTelemetryProtocol/OtlpTraceExporter.cs	77.27% <0.00%> (+18.18%)	⬆️
...entation/ExportClient/OtlpGrpcTraceExportClient.cs	78.57% <0.00%> (+28.57%)	⬆️

[reyang]

Purely from the reader's perspective - I guess folks would question "why would I need to derive from CompositeProcessor?".

[alanwest]

🤔 this is a good question, and I can't think of simple explanation to give users. The alternative to making SetParentProvider public is problematic for similar reasons. I'd struggle to come up with a concise way to explain to users under what circumstances they need to be sure to call SetParentProvider.

In either case the explanation is basically: if you're newing up a processor and you're not directly passing it to providerBuilder.AddProcessor() (i.e., it's nested in another processor or something) then you gotta make sure that the parent provider gets set - which in turn ensures the processor has access to the configured resource... which may or may not be important depending on the processor

which then raises questions of why the parent provider needs to be set, why does it need access to a resource, and when is all of this important

[CodeBlanch]

I don't have a good solution for this! Agree with @alanwest exposing SetParentProvider feels wrong. I just added some XML comments explaining (or hinting at) what is going on in the example. That might help?

We could introduce a class to make it more self-explanatory like...

public class DelegatingProcessor<T> : CompositeProcessor<T>
{
   public DelegatingProcessor(BaseProcessor<T> innerProcessor)
      : base(new[] { innerProcessor ?? throw new ArgumentNullException(nameof(innerProcessor) })
   {
   }
}

...or...

    public class DelegatingProcessor<T> : BaseProcessor<T>
    {
        public DelegatingProcessor(BaseProcessor<T> innerProcessor)
        {
            Guard.ThrowIfNull(innerProcessor);

            this.InnerProcessor = innerProcessor;
        }

        protected BaseProcessor<T> InnerProcessor { get; }

        public override void OnStart(T data)
        {
            this.InnerProcessor.OnStart(data);
        }

        public override void OnEnd(T data)
        {
            this.InnerProcessor.OnEnd(data);
        }

        internal override void SetParentProvider(BaseProvider parentProvider)
        {
            this.InnerProcessor.SetParentProvider(parentProvider);
        }

        protected override bool OnForceFlush(int timeoutMilliseconds)
        {
            return this.InnerProcessor.ForceFlush(timeoutMilliseconds);
        }

        protected override bool OnShutdown(int timeoutMilliseconds)
        {
            return this.InnerProcessor.Shutdown(timeoutMilliseconds);
        }

        protected override void Dispose(bool disposing)
        {
            if (disposing)
            {
                this.InnerProcessor.Dispose();
            }

            base.Dispose(disposing);
        }
    }

I doubt anyone will like that either, but it's an option 🤣

[reyang]

Giving this is an example, I guess I can just close my eyes on it and click the approve button. 🤣

I guess there might be a solution to retrieve the "parent" processor's ParentProvider - based on callstack or context (should be reasonably performant since we only have to fetch it once when the ParentProvider is not yet set), but maybe it's just an overkill 🤷‍♂️

[alanwest]

I doubt anyone will like that either, but it's an option 🤣

Yea not yet excited about this idea. I do agree the naming offers a slight improvement, but I think it's worth sitting on a bit. Definitely think it's worth continuing this discussion in hopes of finding a better solution.

[alanwest]

Something about this problem kinda feels similar to middleware branching. Like the filter processor effectively introduces a branch in the pipeline. Something like

if (filterFunc(span)) {
    // invoke the next process in the chain
} else {
    // terminate the pipeline - maybe call a noop processor or something
}

[reyang]

Looks okay to me.

[alanwest]

👍 looks good. At least the example works as intended now! Can't argue with progress.