Updating ActivityProcessor

[eddynaka]

Fixes #974

Changes

• ActivityProcessor now implements IDisposable
• Moved some of dispose logic to ActivityProcessor, so every processor would use the same code as base.
• Refactor

Please provide a brief description of the changes here. Update the
CHANGELOG.md for non-trivial changes.

For significant contributions please make sure you have completed the following items:

• Design discussion issue #
• Changes in public API reviewed

[codecov]

Codecov Report

Merging #975 into master will increase coverage by 0.05%.
The diff coverage is 94.44%.

@@            Coverage Diff             @@
##           master     #975      +/-   ##
==========================================
+ Coverage   68.14%   68.19%   +0.05%     
==========================================
  Files         218      219       +1     
  Lines        5983     5974       -9     
  Branches      978      978              
==========================================
- Hits         4077     4074       -3     
+ Misses       1632     1626       -6     
  Partials      274      274              

Impacted Files	Coverage Δ
...c/OpenTelemetry.Exporter.ZPages/ZPagesProcessor.cs	85.71% <ø> (ø)
src/OpenTelemetry/Trace/SimpleActivityProcessor.cs	46.15% <0.00%> (+8.65%)	⬆️
...Exporter.Jaeger/Implementation/JaegerUdpBatcher.cs	78.10% <100.00%> (ø)
src/OpenTelemetry/Trace/ActivityProcessor.cs	100.00% <100.00%> (ø)
...c/OpenTelemetry/Trace/BatchingActivityProcessor.cs	80.82% <100.00%> (-0.76%)	⬇️
...metry/Trace/Internal/BroadcastActivityProcessor.cs	92.50% <100.00%> (+5.54%)	⬆️
src/OpenTelemetry/Metrics/CounterMetricSdkBase.cs	80.00% <0.00%> (-13.34%)	⬇️
...nTelemetry/Trace/Internal/NoopActivityProcessor.cs	50.00% <0.00%> (+16.66%)	⬆️

[cijothomas]

Need to highlight that user should override their Dispose method, if they want to achieve custom behavior.

[eddynaka]

Just updated

[reyang]

I guess this block should only be called when disposing == true?

[eddynaka]

in our actual code, it's doing every time. So, I'm not sure.

[CodeBlanch]

I think typically Close/Shutdown call goes outside of the check. Eg:

protected virtual void Dispose(bool disposing)
{
   Close();

   if (disposing)
   {
       // Free managed resources.
   }
}

[eddynaka]

after I moved, i saw some problems from BroadcastActivityProcessor:

protected override void Dispose(bool disposing)
        {
            if (disposing && !this.disposed)
            {
                foreach (var processor in this.processors)
                {
                    try
                    {
                        if (processor is IDisposable disposable)
                        {
                            disposable.Dispose();
                        }
                    }
                    catch (Exception e)
                    {
                        OpenTelemetrySdkEventSource.Log.SpanProcessorException("Dispose", e);
                    }
                }

                this.disposed = true;
            }

            base.Dispose(disposing);
        }

we are disposing the processors and, then, the base.dispose will call the shutdown...

I will revert to the base calling before, and we can think in another way.

[reyang]

LGTM.