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

x/vuln: govulncheck output should be newline delimited json #70824

Closed
MarvinJWendt opened this issue Dec 13, 2024 · 7 comments
Closed

x/vuln: govulncheck output should be newline delimited json #70824

MarvinJWendt opened this issue Dec 13, 2024 · 7 comments
Labels
vulncheck or vulndb Issues for the x/vuln or x/vulndb repo

Comments

@MarvinJWendt
Copy link

MarvinJWendt commented Dec 13, 2024

Proposal

The govulncheck tool currently outputs JSON in a multi-line, prettified format. This makes parsing the output unnecessarily difficult, especially since the -format json flag is intended for machine consumption, and machines do not care about pretty output.

To improve usability and align with standard practices, the govulncheck tool should output newline-delimited JSON (NDJSON) when the -format json flag is specified. This format ensures that each top-level JSON object is written as a single line, simplifying parsing and making it compatible with tools that process NDJSON. This is usually the preferred way of handling JSON streams.

Using NDJSON is not a breaking change, as the JSON structure will remain the same, just outputted in a single line without indentation.

Benefits

  • Ease of Parsing: NDJSON is widely supported by existing tools and libraries, making integration seamless.
  • Improved Performance: Single-line JSON objects reduce overhead in parsing by eliminating the need to handle multi-line structures.
  • Outputting NDJSON is simpler, as the JSON does not need to be prettified beforehand.

Alternatives

As an alternative, a -format ndjson flag could be introduced. While this would ensure that -format json isn't touched, it would output the exact same structures.

Side note

NDJSON is not an alternative to JSON. It's bascially "one JSON object per line".

Reference: https://github.com/ndjson/ndjson-spec

@gopherbot gopherbot added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Dec 13, 2024
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Dec 13, 2024
@seankhliao
Copy link
Member

see #60207 any json parsing tool should already be able to handle arbitrary whitespace between json elements.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Dec 13, 2024
@MarvinJWendt
Copy link
Author

MarvinJWendt commented Dec 13, 2024

@seankhliao this is not related to #60207.
#60207 was about a user that wanted compliant JSON output (one top level element) to create a valid JSON file.

This issue is about making the JSON event stream compliant to popular JSON event streaming specifications (NDJSON, JSONL)

any json parsing tool should already be able to handle arbitrary whitespace between json elements

nushell, a popular shell for data analysis, doesn't support this, but supports NDJSON / JSONL.
I suspect that there are many other tools that don't support this, as it is not compliant to any popular specification for streaming JSON events.

Could you elaborate on why the decision was made to prettify a format that is made for machines? If users want pretty output, they should not use a format that is made for machines.

@MarvinJWendt
Copy link
Author

Doing something with JSON streams usually looks something like this:

(pseudocode)

govulncheck -format json ./... | for each line {send JSON object to tool}

It is hard to find the beginning / end of a JSON object via scripts, if it is prettified.

@MarvinJWendt
Copy link
Author

MarvinJWendt commented Dec 13, 2024

Also, the current behavior is not consistent with some other JSON outputs in the go ecosystem like go test ./... -json, which outputs NDJSON (not prettified) and is compatible with tools like nushell.

In #60207, you compared this to go list -json which is prettified indeed, but as with my points stated above, I think this is a design mistake.

When users really want prettified JSON objects, they could stream the newline delimited JSON to something like jq.

@seankhliao
Copy link
Member

seankhliao commented Dec 13, 2024

https://en.wikipedia.org/wiki/JSON_streaming#Concatenated_JSON
ndjson / jsonl is not anymore authoritative than the other approaches to json streaming, and pretty printing allows for human consumption of the format where more detail may be included that's difficult to represent in a text format.

we could say the same, where users needs ndjson (or some other json representation), they could use a tool like jq.

@MarvinJWendt
Copy link
Author

pretty printing allows for human consumption of the format where more detail may be included that's difficult to represent in a text format

Valid point.

we could say the same, where users needs ndjson (or some other json representation), they could use a tool like jq.

True, but I think a format meant for machine readability should focus on machine readability first, and if someone wants to use the format in another way (which it wasn't made for) they can do the extra step.

I guess both formats have their pros and cons. Thank you for clarifying :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

4 participants