Skip to content
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.

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

Change instrumentation feature gates into normal command-line flags #2582

Closed
8 tasks done
swiatekm opened this issue Jan 30, 2024 · 6 comments · Fixed by #2635
Closed
8 tasks done

Change instrumentation feature gates into normal command-line flags #2582

swiatekm opened this issue Jan 30, 2024 · 6 comments · Fixed by #2635
Labels
area:auto-instrumentation Issues for auto-instrumentation good first issue Good for newcomers help wanted Extra attention is needed

Comments

@swiatekm
Copy link
Contributor

swiatekm commented Jan 30, 2024

Component(s)

operator

Describe the issue you're reporting

We currently use operator feature gates to control which instrumentations are enabled. The intent is to keep this configurable forever, so cluster operators can make this decision for the whole operator deployment. Feature gates are a suboptimal choice for this, however, as they assume a strict stage progression from Alpha to Stable, and eventually are intended to be removed completely.

We should change these into normal command-line flags for the operator while it's still a relatively painless change for our users.

As an aside, we also can't upgrade to newer versions of the otel featuregate package, as some of our gate names contain dashes, which the package now forbids. This is starting to block upgrades of other packages which depend on the featuregate package, for example #2570. In this case, the responsible gates are operator.autoinstrumentation.apache-httpd and operator.autoinstrumentation.multi-instrumentation, so we should start with those.

Flags migrated:

@TylerHelmuth TylerHelmuth added good first issue Good for newcomers help wanted Extra attention is needed area:auto-instrumentation Issues for auto-instrumentation area:operator labels Jan 30, 2024
@changexd
Copy link
Contributor

Hi @swiatekm-sumo I am interested to helping this out, so far I'm planning to add flags to config.Config and put it in instPodMutator, however I wonder if we should separate those into different configs?

@swiatekm
Copy link
Contributor Author

I think it's fine if they're part of the operator's main config struct for now. We can refactor this later if it's unwieldy. Right now we don't have a lot of configuration for any individual instrumentation.

@swiatekm
Copy link
Contributor Author

I've updated the issue with a checklist for the flags that need to be migrated. I'd like to keep this issue open to track the procedure across all the flags.

@jaronoff97
Copy link
Contributor

@swiatekm-sumo i opened up issues for each of these so we don't need to worry about closing the parent issue

@artem-nefedov
Copy link
Contributor

Feature gate operator.autoinstrumentation.multi-instrumentation was removed but an example in README still uses it

@swiatekm
Copy link
Contributor Author

With #2834 merged, we are done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:auto-instrumentation Issues for auto-instrumentation good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants