Skip to content
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.

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

fix(*) implement TextMarshaler for JSON keys #2475

Merged
merged 1 commit into from
Aug 2, 2021
Merged

fix(*) implement TextMarshaler for JSON keys #2475

merged 1 commit into from
Aug 2, 2021

Conversation

jpeach
Copy link
Contributor

@jpeach jpeach commented Aug 2, 2021

Summary

Add TextMarshaler implementations for types that are uses as map keys.
These types are marshaled to JSON for logging, and the map key conversion
in this case requires encoding.TextMarshaler to be implemented. It is
quite confusing for users to see a marshaling error in the Kuma logs,
since it is difficult to distinguish from a real Kuma error.

Full changelog

  • Fix the error message formatting for xDS snapshot generation failure

Issues resolved

Fix #2374.

Documentation

N/A

Testing

  • Unit tests
  • E2E tests
  • Manual testing on Universal
  • Manual testing on Kubernetes

Backwards compatibility

  • Add backport-to-stable label if the code is backwards compatible. Otherwise, list breaking changes.

Add TextMarshaler implementations for types that are uses as map keys.
These types are marshaled to JSON for logging, and the map key conversion
in this case requires encoding.TextMarshaler to be implemented. It is
quite confusing for users to see a marshaling error in the Kuma logs,
since it is difficult to distinguish from a real Kuma error.

This fixes #2374.

Signed-off-by: James Peach <james.peach@konghq.com>
@jpeach jpeach requested a review from a team as a code owner August 2, 2021 05:28
@codecov-commenter
Copy link

Codecov Report

Merging #2475 (0cdc7af) into master (b88f66f) will increase coverage by 0.07%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2475      +/-   ##
==========================================
+ Coverage   52.46%   52.53%   +0.07%     
==========================================
  Files         886      886              
  Lines       48276    48280       +4     
==========================================
+ Hits        25329    25365      +36     
+ Misses      20921    20882      -39     
- Partials     2026     2033       +7     
Impacted Files Coverage Δ
api/mesh/v1alpha1/dataplane_helpers.go 80.24% <0.00%> (-1.35%) ⬇️
pkg/core/resources/manager/manager.go 84.81% <0.00%> (-2.54%) ⬇️
pkg/core/resources/manager/cache.go 81.81% <0.00%> (ø)
api/observability/v1/mads.pb.go 35.56% <0.00%> (+1.03%) ⬆️
pkg/mads/v1/client/client.go 43.75% <0.00%> (+2.50%) ⬆️
pkg/insights/resyncer.go 66.38% <0.00%> (+2.94%) ⬆️
pkg/plugins/resources/memory/store.go 82.06% <0.00%> (+4.34%) ⬆️
pkg/events/eventbus.go 92.59% <0.00%> (+7.40%) ⬆️
pkg/xds/cache/once/cache.go 94.87% <0.00%> (+7.69%) ⬆️
pkg/core/bootstrap/autoconfig.go 54.46% <0.00%> (+8.03%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b88f66f...0cdc7af. Read the comment docs.

@jpeach jpeach merged commit 60d7f97 into kumahq:master Aug 2, 2021
@jpeach jpeach deleted the fix/map-logging-error branch August 2, 2021 22:06
mergify bot pushed a commit that referenced this pull request Aug 2, 2021
Add TextMarshaler implementations for types that are uses as map keys.
These types are marshaled to JSON for logging, and the map key conversion
in this case requires encoding.TextMarshaler to be implemented. It is
quite confusing for users to see a marshaling error in the Kuma logs,
since it is difficult to distinguish from a real Kuma error.

This fixes #2374.

Signed-off-by: James Peach <james.peach@konghq.com>
(cherry picked from commit 60d7f97)
jpeach added a commit that referenced this pull request Aug 2, 2021
Add TextMarshaler implementations for types that are uses as map keys.
These types are marshaled to JSON for logging, and the map key conversion
in this case requires encoding.TextMarshaler to be implemented. It is
quite confusing for users to see a marshaling error in the Kuma logs,
since it is difficult to distinguish from a real Kuma error.

This fixes #2374.

Signed-off-by: James Peach <james.peach@konghq.com>
(cherry picked from commit 60d7f97)

Co-authored-by: James Peach <james.peach@konghq.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.

Failed to convert xds.RouteMap to JSON in log message
4 participants