Skip to content

Option to suppress controller and view spans - #3865

Merged
trask merged 9 commits into
open-telemetry:mainfrom
trask:add-option-to-suppress-internal-spans
Aug 25, 2021
Merged

Option to suppress controller and view spans#3865
trask merged 9 commits into
open-telemetry:mainfrom
trask:add-option-to-suppress-internal-spans

Conversation

@trask

@trask trask commented Aug 18, 2021

Copy link
Copy Markdown
Member

Introduces two new options:

  • otel.instrumentation.common.experimental.suppress-controller-spans
  • otel.instrumentation.common.experimental.suppress-view-spans

to suppress the controller and view spans.

@trask trask changed the title Option to suppress spring-webmvc internal spans Option to suppress controller and view spans Aug 21, 2021
@trask

trask commented Aug 22, 2021

Copy link
Copy Markdown
Member Author

I explored using the new "instrumentation type" for this suppression, but I think the instrumentation type is nicely scoped to semantic conventions, and so this doesn't quite fit. And since there's not semantic conventions, or semantic convention attribute extractors for controllers and views, there's no way to auto-detect the instrumentation type in this case.

An open question I have is that it might be nicer to have a "noop" Instrumenter that can be used, instead of having setDisabled() on InstrumenterBuilder, but since Instrumenter is not an interface creating a "noop" Instrumenter is a bit awkward.

@trask
trask marked this pull request as ready for review August 22, 2021 17:43
@trask

trask commented Aug 22, 2021

Copy link
Copy Markdown
Member Author

I went ahead and applied to all existing Instrumenters to get a fuller picture


private static final boolean SUPPRESS_CONTROLLER_SPANS =
Config.get()
.getBoolean("otel.instrumentation.common.experimental.suppress-controller-spans", false);

@anuraaga anuraaga Aug 23, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should somehow have this in the javaagent-api artifact rather than copying into every module I guess. Either a method on Config or something like JavaAgentConfig with all of them exposed rather than using the flags directly.

Comment on lines +12 to +14
// lazy initialized, so that javaagent can set it, and library instrumentation can fall back and
// read system properties
@Nullable private static ExperimentalConfig instance = null;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That's not true for this class - I think we can safely use a static final instance. It is Config that needs to have lazy initialization, because it's "real" instance is being set by the agent.

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.

indeed, thx!

@trask
trask merged commit 6f22d90 into open-telemetry:main Aug 25, 2021
@trask
trask deleted the add-option-to-suppress-internal-spans branch August 25, 2021 20:43
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.

4 participants