Skip to content

Rename view and controller suppression config properties - #5747

Merged
trask merged 2 commits into
open-telemetry:mainfrom
mateuszrzeszutek:setEnabled
Apr 5, 2022
Merged

Rename view and controller suppression config properties#5747
trask merged 2 commits into
open-telemetry:mainfrom
mateuszrzeszutek:setEnabled

Conversation

@mateuszrzeszutek

Copy link
Copy Markdown
Member

... so that they use the ...something-telemetry.enabled naming scheme.

Also, I changed the Instrumenter API and added the setEnabled() method so that you don't have to negate the boolean value before passing it to the builder.

@mateuszrzeszutek
mateuszrzeszutek requested a review from a team April 4, 2022 14:02
Comment on lines +162 to +169
/**
* Disables the {@link Instrumenter} - it will not generate any telemetry.
*
* <p>Equivalent to calling {@code .setEnabled(false)}.
*/
public InstrumenterBuilder<REQUEST, RESPONSE> disable() {
return setEnabled(false);
}

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.

this looks like it's only called from a test, do you think it will be more generally useful in the future?

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.

Hmm, I included it because I thought that it could be a good idea to have a method that explicitly disables the instrumenter, but on the second thought, it's not that useful -- you'd probably want to disable the instrumenter based on some configuration property, in which case you'd almost always call .setEnabled().
I'll remove it, we probably don't need it in the first stable release.

@trask
trask merged commit 39e1fcb into open-telemetry:main Apr 5, 2022
RashmiRam pushed a commit to RashmiRam/opentelemetry-auto-instr-java that referenced this pull request May 23, 2022
…try#5747)

* Rename view and controller suppression config properties

* code review comments
@mateuszrzeszutek
mateuszrzeszutek deleted the setEnabled branch November 18, 2022 10:28
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