[pkg/instrumentation] Add feature gate for dotnet instrumentation #1629
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jaronoff97
approved these changes
Apr 4, 2023
jaronoff97
approved these changes
Apr 4, 2023
|
@TylerHelmuth could you please paste here the output of operator's help command? |
|
pavolloffay
reviewed
Apr 5, 2023
5 tasks
TylerHelmuth
force-pushed
the
add-featuregate-for-dotnet
branch
from
efd06ab to
f6e01ab
Compare
|
@pavolloffay @jaronoff97 added a section to the README detailing the use of the feature gate. |
pavolloffay
reviewed
Apr 13, 2023
TylerHelmuth
force-pushed
the
add-featuregate-for-dotnet
branch
from
4dc5811 to
f43d5ea
Compare
|
@pavolloffay @jaronoff97 ready for another review. I'd like to get this one merge in before #1555 |
jaronoff97
approved these changes
Apr 17, 2023
pavolloffay
reviewed
Apr 19, 2023
|
|
||
| | Language | Gate | Default Value | | ||
| |----------|---------------------------------------|-----------------| | ||
| | .NET | `operator.autoinstrumentation.dotnet` | enabled | |
There was a problem hiding this comment.
how is it disabled? with disabled value?
There was a problem hiding this comment.
iirc you can do --feature-gates=-operator.autoinstrumentation.dotnet
There was a problem hiding this comment.
@pavolloffay I added the disabling instructions above the table:
Prefix a gate with '-' to disable support for the corresponding language.
Let me know if I need to tweak the paragraph or table to make things more obvious.
| if featuregate.EnableDotnetAutoInstrumentationSupport.IsEnabled() { | ||
| insts.DotNet = inst | ||
| } else { | ||
| pm.Recorder.Event(pod.DeepCopy(), "Warning", "InstrumentationRequestRejected", "support for .NET auto instrumentation is not enabled") |
There was a problem hiding this comment.
could we keep the log as well?
There was a problem hiding this comment.
Sure, I'll add back the error log.
There was a problem hiding this comment.
@pavolloffay added back.
pavolloffay
approved these changes
Apr 25, 2023
ItielOlenick
pushed a commit
to ItielOlenick/opentelemetry-operator
that referenced
this pull request
May 1, 2024
…en-telemetry#1629) * Add feature gate for dotnet instrumentation * changelog * Apply feedback * Move gate into featuregate pkg * Add featuregates to info log * Add README entry for the new gate * Write event if feature gate is disabled * Add back error log
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR puts the dotnet instrumentation behind a feature gate named
operator.autoinstrumentation.dotnet. This feature gate is enabled by default, but operator managers could disable it if they do not want Instrumentation to be able to inject dotnet instrumentation.I'm using dotnet as the guinea pig, but if the pattern is good I'll submit PRs for the other languages as well.