Skip to content

Conversation

@paulgmiller
Copy link
Member

Reason for Change:
Request response logging in cns uses +v on structs with raw bytes which produces very hard to read nonsesne. Those bytes are really json.RawMessage which supports serializing to json so do that instead. Here's an example.

024/12/04 19:57:22 [1] [azure-cnsrequestIPConfigsHandler] Code:FailedToAllocateIpConfig, {DesiredIPAddre
sses:[] PodInterfaceID:c34a4c61-eth0 InfraContainerID:c34a4c61b6b2173f7cb62945a3e9f00ea33b99e4aa6b283e714
b095b7875a87b OrchestratorContext:[123 34 80 111 100 78 97 109 101 34 58 34 122 116 117 110 110 101 108 4
5 57 122 54 55 100 34 44 34 80 111 100 78 97 109 101 115 112 97 99 101 34 58 34 105 115 116 105 111 45 11
5 121 115 116 101 109 34 125] Ifname: SecondaryInterfacesExist:false}, &{PodIPInfo:[{PodIPConfig:{IPAddre
ss: PrefixLength:0} NetworkContainerPrimaryIPConfig:{IPSubnet:{IPAddress: PrefixLength:0} DNSServers:[] G
atewayIPAddress:} HostPrimaryIPInfo:{Gateway: PrimaryIP: Subnet:} NICType: InterfaceName: MacAddress: Ski
pDefaultRoutes:false Routes:[]}] Response:{ReturnCode:FailedToAllocateIpConfig Message:AllocateIPConfig f
ailed: not enough IPs available for 6d10e7dd-184d-4595-b598-baeba084761f, waiting on Azure CNS to allocat
e more with NC Status: , IP config request is {[] c34a4c61-eth0 c34a4c61b6b2173f7cb62945a3e9f00ea33b99e4a
a6b283e714b095b7875a87b [123 34 80 111 100 78 97 109 101 34 58 34 122 116 117 110 110 101 108 45 57 122 5
4 55 100 34 44 34 80 111 100 78 97 109 101 115 112 97 99 101 34 58 34 105 115 116 105 111 45 115 121 115
116 101 109 34 125] false}}}.

Issue Fixed:

Requirements:

Notes:

@paulgmiller paulgmiller requested a review from a team as a code owner December 6, 2024 19:43
@paulgmiller paulgmiller requested a review from rbtr December 6, 2024 19:43
@rbtr rbtr requested a review from Copilot December 7, 2024 01:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 suggestions.

Comments skipped due to low confidence (1)

cns/logger/cnslogger.go:200

  • If JSON marshaling fails, the original request object should be included in the log message for debugging purposes.
requestString = fmt.Sprintf("Failed to json marshal %s, %+v", err, request)

if bytes, err := json.Marshal(request); err != nil {
requestString = string(bytes)
} else {
requestString = fmt.Sprintf("Failed to json marshal %s, %+v", err, request)
Copy link

Copilot AI Dec 7, 2024

Choose a reason for hiding this comment

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

If JSON marshaling fails, the original request object should be included in the log message for debugging purposes.

Suggested change
requestString = fmt.Sprintf("Failed to json marshal %s, %+v", err, request)
requestString = fmt.Sprintf("%+v", request)

Copilot uses AI. Check for mistakes.
if bytes, err := json.Marshal(response); err != nil {
responseString = string(bytes)
} else {
responseString = fmt.Sprintf("Failed to json marshal %s, %+v", err, responseString)
Copy link

Copilot AI Dec 7, 2024

Choose a reason for hiding this comment

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

If JSON marshaling fails, the original response object should be included in the log message for debugging purposes.

Suggested change
responseString = fmt.Sprintf("Failed to json marshal %s, %+v", err, responseString)
responseString = fmt.Sprintf("Failed to json marshal %s, %+v", err, response)

Copilot uses AI. Check for mistakes.
if bytes, err := json.Marshal(response); err != nil {
responseString = string(bytes)
} else {
responseString = fmt.Sprintf("Failed to json marshal %s, %+v", err, responseString)
Copy link

Copilot AI Dec 7, 2024

Choose a reason for hiding this comment

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

If JSON marshaling fails, the original response object should be included in the log message for debugging purposes.

Suggested change
responseString = fmt.Sprintf("Failed to json marshal %s, %+v", err, responseString)
responseString = fmt.Sprintf("Failed to json marshal %s, %+v", err, response)

Copilot uses AI. Check for mistakes.
@jpayne3506 jpayne3506 self-requested a review December 10, 2024 18:30
@jpayne3506 jpayne3506 added the cns Related to CNS. label Dec 10, 2024
@github-actions
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Dec 25, 2024
@github-actions
Copy link

github-actions bot commented Jan 2, 2025

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Jan 2, 2025
@rbtr rbtr reopened this Jan 3, 2025
@github-actions github-actions bot removed the stale Stale due to inactivity. label Jan 4, 2025
@github-actions
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Jan 18, 2025
@github-actions
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Jan 26, 2025
@rbtr rbtr reopened this Jan 27, 2025
@github-actions github-actions bot removed the stale Stale due to inactivity. label Jan 28, 2025
@github-actions
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Feb 11, 2025
@rbtr rbtr removed the stale Stale due to inactivity. label Feb 11, 2025
@github-actions
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Feb 26, 2025
@github-actions
Copy link

github-actions bot commented Mar 6, 2025

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Mar 6, 2025
@rbtr rbtr reopened this Mar 6, 2025
@rbtr rbtr removed the stale Stale due to inactivity. label Mar 6, 2025
@rbtr rbtr self-assigned this Mar 6, 2025
@rbtr
Copy link
Collaborator

rbtr commented Mar 6, 2025

I'll take this over, keeping it open so I don't lose it

@github-actions
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Mar 21, 2025
@github-actions
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Mar 29, 2025
@rbtr rbtr reopened this Mar 30, 2025
@rbtr rbtr removed the stale Stale due to inactivity. label Mar 30, 2025
@github-actions
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label Apr 14, 2025
@github-actions
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this Apr 22, 2025
@rbtr rbtr reopened this Apr 22, 2025
@rbtr rbtr removed the stale Stale due to inactivity. label Apr 22, 2025
@github-actions
Copy link

github-actions bot commented May 7, 2025

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label May 7, 2025
@jpayne3506 jpayne3506 added exempt-stale Keep this fresh and removed stale Stale due to inactivity. labels May 9, 2025
@rbtr rbtr mentioned this pull request May 19, 2025
@github-actions
Copy link

This pull request is stale because it has been open for 2 weeks with no activity. Remove stale label or comment or this will be closed in 7 days

@github-actions github-actions bot added the stale Stale due to inactivity. label May 24, 2025
@github-actions
Copy link

Pull request closed due to inactivity.

@github-actions github-actions bot closed this May 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cns Related to CNS. exempt-stale Keep this fresh stale Stale due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants