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

Collect Deployment Flag Options #1578

Merged
merged 12 commits into from
Feb 28, 2024

Conversation

bjee19
Copy link
Contributor

@bjee19 bjee19 commented Feb 14, 2024

Problem: We want to collect deployment flag options so we can understand what a typical installation is using.

Solution: Collect the deployment flag options.

Testing: Unit tests and E2E testing.

With the deployment flag keys and values paired, this is what we get printed in the logs with a default installation.

config: user-defined

gateway: default

gateway-ctlr-name: user-defined

gatewayclass: user-defined

health-disable: false

health-port: default

help: false

leader-election-disable: false

leader-election-lock-name: user-defined

metrics-disable: false

metrics-port: default

metrics-secure-serving: false

nginx-plus: false

service: “user-defined”

update-gatewayclass-status: “true”

Closes #1315

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@github-actions github-actions bot added the enhancement New feature or request label Feb 14, 2024
@bjee19
Copy link
Contributor Author

bjee19 commented Feb 14, 2024

E2E test results:

With Option 3 of DeploymentFlagOptions where there is one slice for all the flag keys and one slice for all the flag values, this is what is printed for the default installation through helm.

"DeploymentFlagOptions":{"FlagKeys":["config","gateway","gateway-ctlr-name","gatewayclass","health-disable","health-port","help","leader-election-disable","leader-election-lock-name","metrics-disable","metrics-port","metrics-secure-serving","nginx-plus","service","update-gatewayclass-status"],"FlagValues":["user-defined","default","user-defined","user-defined","false","default","false","false","user-defined","false","default","false","false","user-defined","true"]}

Parsed out into matching keys and values this is the output:

config: user-defined

gateway: default

gateway-ctlr-name: user-defined

gatewayclass: user-defined

health-disable: false

health-port: default

help: false

leader-election-disable: false

leader-election-lock-name: user-defined

metrics-disable: false

metrics-port: default

metrics-secure-serving: false

nginx-plus: false

service: user-defined

update-gatewayclass-status: true

This seems correct to me with our current definition of "user-defined" and "default" values. In the installation I changed the health port and it did correctly change from default -> user-defined.

@bjee19
Copy link
Contributor Author

bjee19 commented Feb 14, 2024

Are we satisfied with these results / do we get much out of this data? Currently there are no "default" values for basically all of the string flags so they will always be "user-defined" if they are set at all (which leads into this next point). Also, if a user does the regular quick start deployment, the values in the helm template are set and will be recorded as "user-defined" even if they are the "default" values for a default deployment.

@mpstefan Do you have any questions for this data?

@bjee19 bjee19 force-pushed the enh/telemetry-deployment-flags branch from f13d01c to 9d1bed9 Compare February 16, 2024 18:15
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Feb 16, 2024
go.mod Show resolved Hide resolved
@bjee19 bjee19 marked this pull request as ready for review February 16, 2024 18:25
@bjee19 bjee19 requested a review from a team as a code owner February 16, 2024 18:25
cmd/gateway/commands.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
cmd/gateway/commands.go Outdated Show resolved Hide resolved
internal/mode/static/config/config.go Outdated Show resolved Hide resolved
cmd/gateway/commands_test.go Outdated Show resolved Hide resolved
cmd/gateway/commands_test.go Outdated Show resolved Hide resolved
internal/mode/static/config/config.go Outdated Show resolved Hide resolved
@bjee19 bjee19 force-pushed the enh/telemetry-deployment-flags branch from 8a728f2 to 5a3d931 Compare February 23, 2024 18:27
cmd/gateway/commands.go Outdated Show resolved Hide resolved
Copy link
Member

@ciarams87 ciarams87 left a comment

Choose a reason for hiding this comment

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

🚀

cmd/gateway/commands.go Outdated Show resolved Hide resolved
@bjee19 bjee19 force-pushed the enh/telemetry-deployment-flags branch from e5d891f to 60ac78b Compare February 28, 2024 17:34
@bjee19
Copy link
Contributor Author

bjee19 commented Feb 28, 2024

E2E log output:

config: user-defined

gateway: default

gateway-api-experimental-features: false

gateway-ctlr-name: user-defined

gatewayclass: user-defined

health-disable: false

health-port: default

help: false

leader-election-disable: false

leader-election-lock-name: user-defined

metrics-disable: false

metrics-port: default 

metrics-secure-serving: false

nginx-plus: false

product-telemetry-disable: false

service: user-defined

update-gatewayclass-status: true

usage-report-cluster-name: default

usage-report-secret: default

usage-report-server-url: default

usage-report-skip-verify: false

@bjee19 bjee19 merged commit bf8a951 into nginxinc:main Feb 28, 2024
34 checks passed
@bjee19 bjee19 deleted the enh/telemetry-deployment-flags branch May 7, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Collect Deployment Flag Options (NGF)
6 participants