Documentation: add documentation for non-experimental environment variables - #9227
Documentation: add documentation for non-experimental environment variables#9227ulascansenturk wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9227 +/- ##
==========================================
- Coverage 83.33% 83.16% -0.18%
==========================================
Files 420 423 +3
Lines 34028 35139 +1111
==========================================
+ Hits 28358 29223 +865
- Misses 4253 4414 +161
- Partials 1417 1502 +85 🚀 New features to boost your workflow:
|
| Variables whose names contain `EXPERIMENTAL` are intentionally not documented | ||
| here: they guard features that are still in development, and they may change | ||
| behavior, change defaults, or be removed entirely in any release without | ||
| notice. |
There was a problem hiding this comment.
Are you intending to say that the documented list here is exhaustive other than EXPERIMENTAL flags?
There was a problem hiding this comment.
Yes, that's the intent, and your question caught that it wasn't actually true. I swept every os.Getenv/os.LookupEnv call across the main module and all submodules and found four non-experimental variables missing: GOOGLE_CLOUD_PROJECT (read by gcp/observability, alongside the two config variables already listed) and CSM_CANONICAL_SERVICE_NAME, CSM_WORKLOAD_NAME, CSM_MESH_ID (read by stats/opentelemetry/csm). All four are now documented.
I also made the exhaustiveness claim explicit rather than implied, and broadened the exclusion note to cover TEST_ONLY names as well as EXPERIMENTAL ones. That accounts for GRPC_TEST_ONLY_GOOGLE_C2P_RESOLVER_TRAFFIC_DIRECTOR_URI, which exists for gRPC's own tests. The 15 variables already in the list all check out against the code, with no stale entries.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a new documentation file, environment_variables.md, which lists and describes the environment variables supported by the grpc-go implementation. The review feedback suggests updating the logging section to include the FATAL severity level in the documentation for GRPC_GO_LOG_SEVERITY_LEVEL to ensure completeness.
| logger. One of `ERROR`, `WARNING` or `INFO` (e.g. `INFO` enables info, | ||
| warning and error logs). Defaults to `ERROR`. Has no effect if the |
There was a problem hiding this comment.
The FATAL log level is also supported by the default logger's environment variable parser (it maps to fatalLog in grpclog). We should include FATAL in the list of supported severity levels to make the documentation complete.
| logger. One of `ERROR`, `WARNING` or `INFO` (e.g. `INFO` enables info, | |
| warning and error logs). Defaults to `ERROR`. Has no effect if the | |
| logger. One of `FATAL`, `ERROR`, `WARNING` or `INFO` (e.g. `INFO` enables info, | |
| warning, error and fatal logs). Defaults to `ERROR`. Has no effect if the |
There was a problem hiding this comment.
Not taking this one, as FATAL isn't a recognized value. The parser at grpclog/loggerv2.go:66 has cases for only "", ERROR, WARNING and INFO (plus lowercase forms) and no default, so FATAL matches nothing and leaves infoW, warningW and errorW at io.Discard. Since grpclog/internal/loggerv2.go:258 derives the fatal writer as fatalW := errorW, setting FATAL produces no output at any severity rather than fatal-only logging.
It does point at a real gap in the doc, though: an unrecognized value silences the default logger entirely instead of falling back to the ERROR default, which is worth calling out. I've added a note to that effect, including that Fatal* calls still exit the process in that case since exit(1) runs unconditionally after the (discarded) print.
State that the list is exhaustive and broaden the exclusion note to cover TEST_ONLY variables alongside EXPERIMENTAL ones, then add the non-experimental variables that were missing: GOOGLE_CLOUD_PROJECT and the three CSM_* labels read by stats/opentelemetry/csm. Also document that an unrecognized GRPC_GO_LOG_SEVERITY_LEVEL silences the default logger entirely rather than falling back to the default.
7848987 to
995de8b
Compare
Adds
environment_variables.mdat the repository top level, documenting the non-experimental environment variables supported by grpc-go, grouped by area (logging, binary logging, name resolution, xDS, load balancing, security, server, GCP observability).Variables containing
EXPERIMENTALin their names are intentionally left out, with a note explaining they may change or be removed without notice.Fixes #9213
RELEASE NOTES: none