Skip to content

fix: fix zap.Any logging issues causing unsupported value type errors#1783

Merged
agrawaliti merged 4 commits into
microsoft:mainfrom
agrawaliti:copilot/fix-1680
Jul 30, 2025
Merged

fix: fix zap.Any logging issues causing unsupported value type errors#1783
agrawaliti merged 4 commits into
microsoft:mainfrom
agrawaliti:copilot/fix-1680

Conversation

@agrawaliti

Copy link
Copy Markdown
Contributor

Problem

The zap.Any logger was being used with complex objects in the latency metrics module, resulting in uninformative "unsupported value type" error messages that made debugging difficult:

ts=2025-06-12T14:49:33.339Z level=debug caller=metrics/latency.go:126 msg="Evicted item" item= itemError="unsupported value type"
ts=2025-06-12T14:43:38.295Z level=debug caller=metrics/latency.go:129 msg="Incremented no response metric" metric= metricError="unsupported value type"

Solution

Replaced zap.Any calls with appropriate structured logging using specific zap field types:

Before (problematic):

lm.l.Debug("Evicted item", zap.Any("item", item))
lm.l.Debug("Incremented no response metric", zap.Any("metric", lm.noResponseMetric))
lm.l.Debug("Add apiserver ips", zap.Any("ips", apiServerIPs))

After (fixed):

k := item.Key()
v := item.Value()
lm.l.Debug("Evicted item", 
    zap.String("srcIP", k.srcIP),
    zap.String("dstIP", k.dstIP),
    zap.Uint32("srcPort", k.srcP),
    zap.Uint32("dstPort", k.dstP),
    zap.Uint64("id", k.id),
    zap.Int32("timestamp", v.t))

lm.l.Debug("Incremented no response metric", zap.String("metric", "adv_node_apiserver_no_response"))

ipStrings := make([]string, len(apiServerIPs))
for i, ip := range apiServerIPs {
    ipStrings[i] = ip.String()
}
lm.l.Debug("Add apiserver ips", zap.Strings("ips", ipStrings))

Logs before and after:

ts=2025-06-12T14:49:33.339Z level=debug caller=metrics/latency.go:126 msg="Evicted item" item= itemError="unsupported value type"
ts=2025-06-12T14:43:38.295Z level=debug caller=metrics/latency.go:129 msg="Incremented no response metric" metric= metricError="unsupported value type"
ts=2025-07-23T16:20:50.047Z level=debug caller=metrics/latency.go:128 msg="Evicted item" srcIP=10.224.0.4 dstIP=20.13.226.96 srcPort=56272 dstPort=443 id=614403966 timestamp=543825424
ts=2025-07-23T16:20:50.047Z level=debug caller=metrics/latency.go:137 msg="Incremented no response metric" metric=adv_node_apiserver_no_response

Benefits

  • Informative logging: Debug messages now show actual values instead of "unsupported value type"
  • Better debugging: Network connection details (IPs, ports, timestamps) are clearly visible
  • Structured data: Proper field names make log parsing and analysis easier
  • No breaking changes: Only affects debug log output format

Testing

  • All existing tests pass (23/23)
  • No "unsupported value type" errors from latency.go in test output
  • Verified structured logging produces readable output with meaningful field names

Fixes #1680.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

This commit:
- Replaces zap.Any calls with appropriate zap type-specific methods
- Optimizes IP processing for better efficiency
- Uses constant for metric name instead of hardcoded string
- Applies gofumpt formatting to latency.go

Co-authored-by: rbtr <2940321+rbtr@users.noreply.github.com>
Co-authored-by: nddq <28567936+nddq@users.noreply.github.com>

Fixes microsoft#1680
@agrawaliti

Copy link
Copy Markdown
Contributor Author

Signed PR of: #1704

@ritwikranjan ritwikranjan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm other than the hardcoded metric name.

Comment thread pkg/module/metrics/latency.go Outdated

@ritwikranjan ritwikranjan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

@agrawaliti agrawaliti enabled auto-merge July 30, 2025 12:30
@agrawaliti agrawaliti disabled auto-merge July 30, 2025 12:33
@agrawaliti

Copy link
Copy Markdown
Contributor Author

Verified logs with the final changes

ts=2025-07-30T12:50:25.048Z level=debug caller=metrics/latency.go:128 msg="Evicted item" srcIP=10.224.0.4 dstIP=20.116.148.139 srcPort=56124 dstPort=443 id=559864698 timestamp=425380656
ts=2025-07-30T12:50:25.048Z level=debug caller=metrics/latency.go:137 msg="Incremented no response metric" metric=adv_node_apiserver_no_response
ts=2025-07-30T12:50:25.082Z level=debug caller=packetparser/packetparser_linux.go:564 msg="Received record" cpu=3 lost_samples=0 bytes_remaining=136 worker_id=0
--

@agrawaliti agrawaliti added this pull request to the merge queue Jul 30, 2025
Merged via the queue into microsoft:main with commit ba674a3 Jul 30, 2025
31 checks passed
@agrawaliti agrawaliti deleted the copilot/fix-1680 branch July 30, 2025 13:27
mereta pushed a commit that referenced this pull request Dec 2, 2025
…#1783)

## Problem

The `zap.Any` logger was being used with complex objects in the latency
metrics module, resulting in uninformative "unsupported value type"
error messages that made debugging difficult:

```
ts=2025-06-12T14:49:33.339Z level=debug caller=metrics/latency.go:126 msg="Evicted item" item= itemError="unsupported value type"
ts=2025-06-12T14:43:38.295Z level=debug caller=metrics/latency.go:129 msg="Incremented no response metric" metric= metricError="unsupported value type"
```

## Solution

Replaced `zap.Any` calls with appropriate structured logging using
specific zap field types:

### Before (problematic):
```go
lm.l.Debug("Evicted item", zap.Any("item", item))
lm.l.Debug("Incremented no response metric", zap.Any("metric", lm.noResponseMetric))
lm.l.Debug("Add apiserver ips", zap.Any("ips", apiServerIPs))
```

### After (fixed):
```go
k := item.Key()
v := item.Value()
lm.l.Debug("Evicted item", 
    zap.String("srcIP", k.srcIP),
    zap.String("dstIP", k.dstIP),
    zap.Uint32("srcPort", k.srcP),
    zap.Uint32("dstPort", k.dstP),
    zap.Uint64("id", k.id),
    zap.Int32("timestamp", v.t))

lm.l.Debug("Incremented no response metric", zap.String("metric", "adv_node_apiserver_no_response"))

ipStrings := make([]string, len(apiServerIPs))
for i, ip := range apiServerIPs {
    ipStrings[i] = ip.String()
}
lm.l.Debug("Add apiserver ips", zap.Strings("ips", ipStrings))
```

### Logs before and after: 
```` 
ts=2025-06-12T14:49:33.339Z level=debug caller=metrics/latency.go:126 msg="Evicted item" item= itemError="unsupported value type"
ts=2025-06-12T14:43:38.295Z level=debug caller=metrics/latency.go:129 msg="Incremented no response metric" metric= metricError="unsupported value type"
````
```
ts=2025-07-23T16:20:50.047Z level=debug caller=metrics/latency.go:128 msg="Evicted item" srcIP=10.224.0.4 dstIP=20.13.226.96 srcPort=56272 dstPort=443 id=614403966 timestamp=543825424
ts=2025-07-23T16:20:50.047Z level=debug caller=metrics/latency.go:137 msg="Incremented no response metric" metric=adv_node_apiserver_no_response
```

## Benefits

- **Informative logging**: Debug messages now show actual values instead
of "unsupported value type"
- **Better debugging**: Network connection details (IPs, ports,
timestamps) are clearly visible
- **Structured data**: Proper field names make log parsing and analysis
easier
- **No breaking changes**: Only affects debug log output format

## Testing

- All existing tests pass (23/23)
- No "unsupported value type" errors from latency.go in test output
- Verified structured logging produces readable output with meaningful
field names

Fixes #1680.

<!-- START COPILOT CODING AGENT TIPS -->
---

💡 You can make Copilot smarter by setting up custom instructions,
customizing its development environment and configuring Model Context
Protocol (MCP) servers. Learn more [Copilot coding agent
tips](https://gh.io/copilot-coding-agent-tips) in the docs.

---------

Signed-off-by: Iti Agrawal <agrawaliti0107@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

zap.any always logs "unsupported value type", which is not informative.

2 participants