Skip to content

Suppress instrumentation upon calling ActivityExporter.ExportAsync#967

Closed
alanwest wants to merge 1 commit intoopen-telemetry:reyang/suppress-instrumentationfrom
alanwest:alanwest/suppress-instrumentation-activityprocessor
Closed

Suppress instrumentation upon calling ActivityExporter.ExportAsync#967
alanwest wants to merge 1 commit intoopen-telemetry:reyang/suppress-instrumentationfrom
alanwest:alanwest/suppress-instrumentation-activityprocessor

Conversation

@alanwest
Copy link
Copy Markdown
Member

@alanwest alanwest commented Aug 1, 2020

Draft PR, currently targeting @reyang's currently unmerged PR #960

Sorry, it's a bit of a noisy diff because I made some changes to the abstract ActivityProcessor class trying out the idea of checking the SuppressInstrumentation bit in one spot. I think this will work (??). Though, perf may be the concern here.

This only touches tracing at this point, though will want the same kind of thing for metric export as well.

{
var originalString = requestUri.OriginalString;

// zipkin
Copy link
Copy Markdown
Member Author

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.

Comment on lines +337 to +341
var previous = Sdk.SuppressInstrumentation;
Sdk.SuppressInstrumentation = true;
var result = await this.exporter.ExportAsync(this.batch, cancellationToken).ConfigureAwait(false);
Sdk.SuppressInstrumentation = previous;

Copy link
Copy Markdown
Member Author

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.

Comment on lines +33 to +36
if (Sdk.SuppressInstrumentation)
{
return;
}
Copy link
Copy Markdown
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants