Skip to content

xds: fix NACK logging after slog migration #41171

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 15, 2025
Merged

xds: fix NACK logging after slog migration #41171

merged 2 commits into from
Aug 15, 2025

Conversation

mhofstetter
Copy link
Member 

xds: fix NACK logging after slog migration
This commit fixes a log message that still contains substitutions in the
log message after the log migration.

xds: remove duplicated log variables when logging xds NACK
Currently the log message that reports an xDS NACK from envoy contains
duplicates for the version & nonce.

Therefore, this commit removes the extra log fields `responseNonce` & `version`
when logging the message because they are already part of the scoped `requestLog`.

In addition the variable `xdsAckedVersion` gets renamed to `version` because having
the word `acked` in the field for the NACK log message would be confusing.

@mhofstetter
xds: fix NACK logging after slog migration
2a70aa7 
This commit fixes a log message that still contains substitutions in the
log message after the log migration.

```
2025-08-14T18:19:16.429606429Z time=2025-08-14T18:19:16.427860732Z level=warn source=/go/src/github.com/cilium/cilium/pkg/envoy/xds/server.go:397 msg="NACK received for versions after %s and up to %s; waiting for a version update before sending again" module=enterprise-agent.agent.controlplane.envoy-proxy [...] version=19 responseNonce=20
```

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter
xds: remove duplicated log variables when logging xds NACK
11812ce 
Currently the log message that reports an xDS NACK from envoy contains
duplicates for the version & nonce.

```
2025-08-14T18:19:16.429606429Z time=2025-08-14T18:19:16.427860732Z level=warn source=/go/src/github.com/cilium/cilium/pkg/envoy/xds/server.go:397 msg="NACK received for versions after %s and up to %s; waiting for a version update before sending again" module=enterprise-agent.agent.controlplane.envoy-proxy [...] xdsAckedVersion=19 xdsNonce=20 version=19 responseNonce=20
```

Therefore, this commit removes the extra log fields `responseNonce` & `version`
when logging the message because they are already part of the scoped `requestLog`.

In addition the variable `xdsAckedVersion` gets renamed to `version` because having
the word `acked` in the field for the NACK log message would be confusing.

Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
@mhofstetter mhofstetter added the kind/bug This is a bug in the Cilium logic. label Aug 15, 2025
@mhofstetter mhofstetter requested a review from a team as a code owner August 15, 2025 08:08
@mhofstetter mhofstetter added area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/misc This PR makes changes that have no direct user impact. area/servicemesh GH issues or PRs regarding servicemesh labels Aug 15, 2025
@mhofstetter mhofstetter requested a review from sayboras August 15, 2025 08:08
@mhofstetter mhofstetter added the needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch label Aug 15, 2025
@mhofstetter
Copy link
Member Author

/test

@aanm aanm enabled auto-merge August 15, 2025 08:36
@aanm aanm added this pull request to the merge queue Aug 15, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 15, 2025
Merged via the queue into cilium:main with commit 08874ab Aug 15, 2025
74 checks passed
@joamaki joamaki mentioned this pull request Aug 19, 2025
Merged
19 tasks
@joamaki joamaki added backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. and removed needs-backport/1.18 This PR / issue needs backporting to the v1.18 branch labels Aug 19, 2025
@github-actions github-actions bot added backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. and removed backport-pending/1.18 The backport for Cilium 1.18.x for this PR is in progress. labels Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aanm aanm aanm approved these changes

@sayboras sayboras sayboras approved these changes

Assignees
No one assigned
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. area/servicemesh GH issues or PRs regarding servicemesh backport-done/1.18 The backport for Cilium 1.18.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mhofstetter @aanm @sayboras @joamaki