plugins/ocp: fix telemetry string log output format

[Karthikmct]

This PR will take care of fixing the output format for "telemetry-string-log" command in ocp plug in

Fixed the code to handle json output crash and proper display of ASCII Table content
Disabling debug logs output while parsing telemetry-string-log data

Signed-off-by: Karthik Balan [email protected],

Reviewed-by: Arunpandian J [email protected],

[igaw]

Generally, if you commit message already contains bullet points which this change does, these bullet point changes should usually be separate patches.

As far I read the ascii output change, it's a output format change, right? Not sure how you want to handle this. @arthurshau what are you thoughts on this?

[Karthikmct]

Generally, if you commit message already contains bullet points which this change does, these bullet point changes should usually be separate patches.
@igaw , patch is about fixing json output format for telemetry sting log, one is handling a crash and aligning the ASCII table as per specification requirement. These two fixes goes hand in hand, so raised as one PR.

As far I read the ascii output change, it's a output format change, right? Not sure how you want to handle this.
@igaw , It's about change in ascii table display in json output format. Not a change to output format itself. fix is to align with specification expected template.

Thanks.

[igaw]

First point, okay. Didn't look too closely. Just thought, it's unrelated.

Second point. If your change doesn't change the field names, then all is good. My argument is that changing the json field names (e.g. renaming) is a regression. The json output should be stable, so that any parser doesn't have to be touched just because we rename something.

[Karthikmct]

Second point. If your change doesn't change the field names, then all is good. My argument is that changing the json field names (e.g. renaming) is a regression. The json output should be stable, so that any parser doesn't have to be touched just because we rename something.

Got it. No changes are there for the field names. Its still noted 'ASCII Table'. The change is instead of printing the complete ascii data as a single dump, organized it in Byte wise ASCII data (as expected by specification)

[Karthikmct]

@igaw , To ease the review and merge. I modified this pull request into 2 part. This part is to fix the json output format crash and disabling the debug prints only.

Will take up the ASCII Table alignment in a separate pull request. please check further :)

[igaw]

Looks good, just the reserved field renaming thing

[Karthikmct]

@igaw , Could you please update further on the patch upstream?
Do let me know on clarification.

[igaw]

Looks good to me. Thanks!