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 JSON output for cgroup_path #2793

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

viktormalik
Copy link
Contributor

@viktormalik viktormalik commented Oct 6, 2023

As #2792 noted, for some reason, cgroup path is not surrounded by quotes in the output which poses problems for JSON parsers as it may contain JSON-reserved characters (:). This is strange, because the same doesn't happen with other builtins, e.g. inet, which also have : in the output.

Anyways, add surrounding quotes manually and write a test for this. The test passes the produced JSON to Python's JSON parser to see if it parses correctly. Prior to this change, the test fails.

Fixes #2792.

Checklist
  • Language changes are updated in man/adoc/bpftrace.adoc and if needed in docs/reference_guide.md
  • User-visible and non-trivial changes updated in CHANGELOG.md
  • The new behaviour is covered by tests

@jordalgo
Copy link
Contributor

jordalgo commented Oct 6, 2023

@viktormalik Wow! Thanks for the very fast fix!

Copy link
Member

@ajor ajor 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 is_quoted_type() function would be the right place to put this fix:
https://github.com/iovisor/bpftrace/blob/281a67a71d06b917d107cd0e85eac7b23ace2b42/src/output.cpp#L10-L15

This would explain how inet works already.

Add cgroup_path to quoted types so that it's surrounded by quotes in
JSON output, otherwise it poses problems for JSON parsers as it may
contain JSON-reserved characters (':').

Also add a test for this. The test passes the produced JSON to Python's
JSON parser to see if it parses correctly. Prior to this change, the
test fails.
@viktormalik
Copy link
Contributor Author

I think the is_quoted_type() function would be the right place to put this fix:

https://github.com/iovisor/bpftrace/blob/281a67a71d06b917d107cd0e85eac7b23ace2b42/src/output.cpp#L10-L15

This would explain how inet works already.

Ah, of course, I completely missed that.

Many thanks! Fixed.

Copy link
Member

@ajor ajor left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

@ajor ajor merged commit 4be5ffa into bpftrace:master Oct 6, 2023
21 checks passed
@viktormalik viktormalik deleted the cgroup-path-json-fix branch October 9, 2023 07:04
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.

cgroup_path not being quoted in json output
3 participants