Skip to content

log: Add "%j" to print message as JSON escaped string#15226

Merged
mattklein123 merged 6 commits intoenvoyproxy:mainfrom
dio:fix-15020
Mar 2, 2021
Merged

log: Add "%j" to print message as JSON escaped string#15226
mattklein123 merged 6 commits intoenvoyproxy:mainfrom
dio:fix-15020

Conversation

@dio
Copy link
Copy Markdown
Member

@dio dio commented Feb 27, 2021

Commit Message: This patch adds a new custom flag %j to the log pattern to print the actual message to log as JSON escaped string.

Additional description: This produces JSON escaped string for the msg.payload part only, not the whole logline. Further description of the use case: #13636.
Risk Level: Low
Testing: Unit
Docs Changes: Updated
Release Notes: Added
Fixes #15020

Signed-off-by: Dhi Aurrahman dio@rockybars.com

dio added 4 commits February 28, 2021 08:49
This patch adds a new custom flag `%j` to the log pattern to printthe
actual message to log, but as escaped JSON string.

Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Copy link
Copy Markdown
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

I think the implication is that UTF-8 encoded strings are passed through without modification. Should we make that explicit in the comments? Otherwise just some style nits.

position += 2;
break;
default:
if (character >= 0x00 and character <= 0x1f) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: use &&

dio added 2 commits March 1, 2021 21:10
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
Signed-off-by: Dhi Aurrahman <dio@rockybars.com>
@dio
Copy link
Copy Markdown
Member Author

dio commented Mar 1, 2021

Thanks, @zuercher for the review, I have updated the comment and code per your review comments.

@mattklein123 mattklein123 merged commit 08d01f1 into envoyproxy:main Mar 2, 2021
// Calculate the extra space to build a JSON escaped string.
// @param input input string.
// @return uint64_t the number of extra characters required to to build a JSON escaped string.
static uint64_t extraSpace(absl::string_view input) {
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.

How can we ensure that extraSpace and escapeString remain in sync? Seems to me that we have some risk for writes past the end of the string. Granted, it's somewhat likely that we'll catch it if tests are written for all escape sequences.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@dio is there a way for us not to copy+paste from the JSON dep? It would be pretty scary if they fix a bug and we don't here. I'm hoping we can just do some minimal Bazel BUILD dep pointed at the relevant header file.

howardjohn added a commit to howardjohn/istio that referenced this pull request Mar 5, 2021
Fixes istio#31137
Depends on envoyproxy/envoy#15226 - we can
backport IFF we backport that
istio-testing pushed a commit to istio/istio that referenced this pull request Mar 8, 2021
Fixes #31137
Depends on envoyproxy/envoy#15226 - we can
backport IFF we backport that
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.

Allow json output for logging (not access logs)

5 participants