Skip to content

Suppress instrumentation upon calling ActivityExporter.ExportAsync #967

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.

Sign up for GitHub

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

Closed
alanwest wants to merge 1 commit into from
Closed

Suppress instrumentation upon calling ActivityExporter.ExportAsync #967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions examples/Console/TestConsoleExporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,22 @@ public MyProcessor(ActivityProcessor next)
this.next = next;
}

public override void OnEnd(Activity activity)
public override Task ShutdownAsync(CancellationToken cancellationToken)
{
return this.next.ShutdownAsync(cancellationToken);
}

public override Task ForceFlushAsync(CancellationToken cancellationToken)
{
return this.next.ForceFlushAsync(cancellationToken);
}

protected override void OnEndInternal(Activity activity)
{
this.next.OnEnd(activity);
}

public override void OnStart(Activity activity)
protected override void OnStartInternal(Activity activity)
{
if (activity.IsAllDataRequested)
{
Expand All @@ -129,16 +139,6 @@ public override void OnStart(Activity activity)

this.next.OnStart(activity);
}

public override Task ShutdownAsync(CancellationToken cancellationToken)
{
return this.next.ShutdownAsync(cancellationToken);
}

public override Task ForceFlushAsync(CancellationToken cancellationToken)
{
return this.next.ForceFlushAsync(cancellationToken);
}
}
}
}
40 changes: 20 additions & 20 deletions src/OpenTelemetry.Exporter.ZPages/ZPagesProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,25 @@ public ZPagesProcessor(ZPagesExporter exporter)
}

/// <inheritdoc />
public override void OnStart(Activity activity)
public override Task ShutdownAsync(CancellationToken cancellationToken)
{
throw new NotImplementedException();
}

/// <inheritdoc />
public override Task ForceFlushAsync(CancellationToken cancellationToken)
{
throw new NotImplementedException();
}

/// <inheritdoc />
public void Dispose()
{
throw new NotImplementedException();
}

/// <inheritdoc />
protected override void OnStartInternal(Activity activity)
{
if (!ZPagesActivityTracker.ProcessingList.ContainsKey(activity.DisplayName))
{
Expand All @@ -65,7 +83,7 @@ public override void OnStart(Activity activity)
}

/// <inheritdoc />
public override void OnEnd(Activity activity)
protected override void OnEndInternal(Activity activity)
{
try
{
Expand Down Expand Up @@ -120,23 +138,5 @@ public override void OnEnd(Activity activity)
Console.Write("OnEnd", ex);
}
}

/// <inheritdoc />
public override Task ShutdownAsync(CancellationToken cancellationToken)
{
throw new NotImplementedException();
}

/// <inheritdoc />
public override Task ForceFlushAsync(CancellationToken cancellationToken)
{
throw new NotImplementedException();
}

/// <inheritdoc />
public void Dispose()
{
throw new NotImplementedException();
}
}
}
6 changes: 0 additions & 6 deletions src/OpenTelemetry.Instrumentation.Http/HttpClientInstrumentationOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ internal static bool IsInternalUrl(Uri requestUri)
{
var originalString = requestUri.OriginalString;

// zipkin
Copy link
Copy Markdown
Member Author

@alanwest alanwest Aug 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a quick manual test with Zipkin, so I removed this filter.

if (originalString.Contains(":9411/api/v2/spans"))
{
return true;
}

// applicationinsights
if (originalString.StartsWith("https://dc.services.visualstudio") ||
originalString.StartsWith("https://rt.services.visualstudio") ||
Expand Down
24 changes: 22 additions & 2 deletions src/OpenTelemetry/Trace/ActivityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,29 @@ public abstract class ActivityProcessor
/// Activity start hook.
/// </summary>
/// <param name="activity">Instance of activity to process.</param>
public abstract void OnStart(Activity activity);
public void OnStart(Activity activity)
{
if (Sdk.SuppressInstrumentation)
{
return;
}
Comment on lines +33 to +36
Copy link
Copy Markdown
Member Author

@alanwest alanwest Aug 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking SuppressInstrumentation in one spot to avoid reentering pipeline.


this.OnStartInternal(activity);
}

/// <summary>
/// Activity end hook.
/// </summary>
/// <param name="activity">Instance of activity to process.</param>
public abstract void OnEnd(Activity activity);
public void OnEnd(Activity activity)
{
if (Sdk.SuppressInstrumentation)
{
return;
}

this.OnEndInternal(activity);
}

/// <summary>
/// Shuts down Activity processor asynchronously.
Expand All @@ -49,5 +65,9 @@ public abstract class ActivityProcessor
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>Returns <see cref="Task"/>.</returns>
public abstract Task ForceFlushAsync(CancellationToken cancellationToken);

protected abstract void OnStartInternal(Activity activity);

protected abstract void OnEndInternal(Activity activity);
}
}
96 changes: 50 additions & 46 deletions src/OpenTelemetry/Trace/BatchingActivityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,52 +136,6 @@ public BatchingActivityProcessor(ActivityExporter exporter, int maxQueueSize, Ti
};
}

/// <inheritdoc/>
public override void OnStart(Activity activity)
{
}

/// <inheritdoc/>
public override void OnEnd(Activity activity)
{
// because of race-condition between checking the size and enqueueing,
// we might end up with a bit more activities than maxQueueSize.
// Let's just tolerate it to avoid extra synchronization.
if (this.currentQueueSize >= this.maxQueueSize)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorQueueIsExhausted();
return;
}

var size = Interlocked.Increment(ref this.currentQueueSize);

this.exportQueue.Enqueue(activity);

if (size >= this.maxExportBatchSize)
{
bool lockTaken = this.flushLock.Wait(0);
if (lockTaken)
{
Task.Run(async () =>
{
try
{
await this.FlushAsyncInternal(drain: false, lockAlreadyHeld: true, CancellationToken.None).ConfigureAwait(false);
}
catch (Exception ex)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.OnEnd), ex);
}
finally
{
this.flushLock.Release();
}
});
return;
}
}
}

/// <inheritdoc/>
/// <exception cref="OperationCanceledException">If the <paramref name="cancellationToken"/> is canceled.</exception>
public override async Task ShutdownAsync(CancellationToken cancellationToken)
Expand Down Expand Up @@ -239,6 +193,52 @@ protected virtual void Dispose(bool isDisposing)
}
}

/// <inheritdoc/>
protected override void OnStartInternal(Activity activity)
{
}

/// <inheritdoc/>
protected override void OnEndInternal(Activity activity)
{
// because of race-condition between checking the size and enqueueing,
// we might end up with a bit more activities than maxQueueSize.
// Let's just tolerate it to avoid extra synchronization.
if (this.currentQueueSize >= this.maxQueueSize)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorQueueIsExhausted();
return;
}

var size = Interlocked.Increment(ref this.currentQueueSize);

this.exportQueue.Enqueue(activity);

if (size >= this.maxExportBatchSize)
{
bool lockTaken = this.flushLock.Wait(0);
if (lockTaken)
{
Task.Run(async () =>
{
try
{
await this.FlushAsyncInternal(drain: false, lockAlreadyHeld: true, CancellationToken.None).ConfigureAwait(false);
}
catch (Exception ex)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorException(nameof(this.OnEnd), ex);
}
finally
{
this.flushLock.Release();
}
});
return;
}
}
}

private async Task FlushAsyncInternal(bool drain, bool lockAlreadyHeld, CancellationToken cancellationToken)
{
if (!lockAlreadyHeld)
Expand Down Expand Up @@ -334,7 +334,11 @@ private async Task<int> ExportBatchAsync(CancellationToken cancellationToken)
this.batch.Add(nextActivity);
}

var previous = Sdk.SuppressInstrumentation;
Sdk.SuppressInstrumentation = true;
var result = await this.exporter.ExportAsync(this.batch, cancellationToken).ConfigureAwait(false);
Sdk.SuppressInstrumentation = previous;

Comment on lines +337 to +341
Copy link
Copy Markdown
Member Author

@alanwest alanwest Aug 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where SuppressInstrumentation is actually set.

if (result != ExportResult.Success)
{
OpenTelemetrySdkEventSource.Log.ExporterErrorResult(result);
Expand Down
65 changes: 35 additions & 30 deletions src/OpenTelemetry/Trace/Internal/BroadcastActivityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,36 +44,6 @@ public BroadcastActivityProcessor(IEnumerable<ActivityProcessor> processors)
this.processors = processors;
}

public override void OnEnd(Activity activity)
{
foreach (var processor in this.processors)
{
try
{
processor.OnEnd(activity);
}
catch (Exception e)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorException("OnEnd", e);
}
}
}

public override void OnStart(Activity activity)
{
foreach (var processor in this.processors)
{
try
{
processor.OnStart(activity);
}
catch (Exception e)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorException("OnStart", e);
}
}
}

public override Task ShutdownAsync(CancellationToken cancellationToken)
{
var tasks = new List<Task>();
Expand Down Expand Up @@ -132,5 +102,40 @@ protected virtual void Dispose(bool isDisposing)
this.isDisposed = true;
}
}

protected override void OnEndInternal(Activity activity)
{
foreach (var processor in this.processors)
{
try
{
processor.OnEnd(activity);
}
catch (Exception e)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorException("OnEnd", e);
}
}
}

protected override void OnStartInternal(Activity activity)
{
if (Sdk.SuppressInstrumentation)
{
return;
}

foreach (var processor in this.processors)
{
try
{
processor.OnStart(activity);
}
catch (Exception e)
{
OpenTelemetrySdkEventSource.Log.SpanProcessorException("OnStart", e);
}
}
}
}
}
16 changes: 8 additions & 8 deletions src/OpenTelemetry/Trace/Internal/NoopActivityProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,6 @@ namespace OpenTelemetry.Trace.Internal
{
internal sealed class NoopActivityProcessor : ActivityProcessor
{
public override void OnStart(Activity activity)
{
}

public override void OnEnd(Activity activity)
{
}

public override Task ShutdownAsync(CancellationToken cancellationToken)
{
#if NET452
Expand All @@ -47,5 +39,13 @@ public override Task ForceFlushAsync(CancellationToken cancellationToken)
return Task.CompletedTask;
#endif
}

protected override void OnStartInternal(Activity activity)
{
}

protected override void OnEndInternal(Activity activity)
{
}
}
}
Loading