Skip to content

cleanup(sidecar,logging): share the level encoder and use named verbosity constants - #2474

Merged
ahg-g merged 2 commits into
llm-d:mainfrom
LukeAVanDrie:epp-logging-tooling-cleanup
Aug 21, 2026
Merged

cleanup(sidecar,logging): share the level encoder and use named verbosity constants#2474
ahg-g merged 2 commits into
llm-d:mainfrom
LukeAVanDrie:epp-logging-tooling-cleanup

Conversation

@LukeAVanDrie

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Consistency fixes in shared infrastructure:

  • The custom zap level encoder existed verbatim in both pkg/common/observability/logging and pkg/sidecar/proxy/options.go, so a change to level naming had to be made twice. It is now exported once as logging.LevelEncoder and the sidecar copy is deleted.
  • The sidecar used bare V(4)/V(5) integers at 50 call sites where the logging convention requires the named logging.DEBUG/logging.TRACE constants; much of the sidecar already follows the convention. Levels are unchanged. V(1) sites are left alone since no named constant maps to them.
  • The sidecar's full-request-body TRACE logs stringified multi-KB bodies per request even with TRACE disabled. They are now guarded with Enabled().
  • MaxActiveModelsKey and UpdateTimeKey in the metrics extractor had no references anywhere and are deleted.
  • Dockerfile.builder pinned envtest at release-0.19 / k8s 1.31.0 (EOL) while kubectl is v1.35.3. The pin is now release-0.23 / 1.35.0, matching controller-runtime v0.23.3 and the k8s v0.35 module set, so hermetic integration tests run against a current control plane.

Which issue(s) this PR fixes:

Part of #2470

Release note (write NONE if no user-facing change):

NONE

Test plan

  • Existing unit tests for pkg/common/..., pkg/sidecar/..., and the metrics extractor
  • make test-integration-hermetic with the new envtest pin (validates binary availability and control-plane compatibility; the epp integration suite passes)
  • make presubmit

@LukeAVanDrie
LukeAVanDrie requested review from a team and roytman as code owners August 19, 2026 23:39
@github-actions github-actions Bot added area/sidecar area/epp area/telemetry size/L Denotes a PR that changes 100-499 lines, ignoring generated files. area/datalayer kind/cleanup labels Aug 19, 2026
roytman
roytman previously approved these changes Aug 20, 2026
Comment thread pkg/sidecar/proxy/allowlist.go Outdated
Comment thread pkg/sidecar/proxy/connector_mooncake.go Outdated
Comment thread pkg/sidecar/proxy/connector_nixlv2.go Outdated
Comment thread pkg/sidecar/proxy/connector_sglang.go Outdated
Comment thread pkg/sidecar/proxy/connector_shared_storage.go Outdated
Comment thread pkg/sidecar/proxy/data_parallel.go Outdated
Comment thread pkg/sidecar/proxy/decode.go Outdated
@roytman
roytman self-requested a review August 20, 2026 06:53
@roytman
roytman dismissed their stale review August 20, 2026 06:56

There are redundant imports.

…sity constants

The zap level encoder existed verbatim in both the common logging
package and the sidecar; export it once as logging.LevelEncoder and
delete the copy. Convert the sidecar's bare V(4)/V(5) calls to the
logging.DEBUG/logging.TRACE constants the logging convention requires,
and guard the full-request-body TRACE logs, which stringified multi-KB
bodies per request even when TRACE is off. V(1) sites are left as is;
no named constant maps to them.

Also drop the unreferenced MaxActiveModelsKey/UpdateTimeKey exports
from the metrics extractor, and bump the envtest pin (release-0.19 /
k8s 1.31, now EOL) to release-0.23 / k8s 1.35.0, matching
controller-runtime v0.23 and the k8s 1.35 module set.

Signed-off-by: Luke Van Drie <lukevandrie@google.com>
Signed-off-by: Luke Van Drie <lukevandrie@google.com>
@LukeAVanDrie
LukeAVanDrie force-pushed the epp-logging-tooling-cleanup branch from 214bba9 to af01af9 Compare August 20, 2026 18:17
@ahg-g
ahg-g merged commit be47271 into llm-d:main Aug 21, 2026
40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/datalayer area/epp area/sidecar area/telemetry kind/cleanup size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants