-
Notifications
You must be signed in to change notification settings - Fork 508
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
Add JSON Folding Support in Logs #1724
Add JSON Folding Support in Logs #1724
Conversation
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
packages/jaeger-ui/src/components/TracePage/TraceTimelineViewer/index.css
Outdated
Show resolved
Hide resolved
content = ( | ||
<JsonView | ||
data={parsed} | ||
shouldInitiallyExpand={allExpanded} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this be conditional on the size of the output, e.g. if the JSON object has more than 10 keys then don't start in expanded form.
Also, please post a screenshot of how it looks in the collapsed at the root form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The JSON tree will now only expand if it has 10 or less keys, nested objects will remain collapsed otherwise
Signed-off-by: Prathamesh Mutkure <pmutkure009@gmail.com>
f008115
to
b501f0f
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1724 +/- ##
==========================================
- Coverage 96.01% 96.00% -0.02%
==========================================
Files 241 240 -1
Lines 7562 7514 -48
Branches 1987 1974 -13
==========================================
- Hits 7261 7214 -47
+ Misses 301 300 -1
☔ View full report in Codecov by Sentry. |
@@ -79,7 +71,28 @@ function formatValue(key: string, value: any) { | |||
} else if (Array.isArray(parsed) && shouldDisplayAsStringList(key)) { | |||
content = stringListMarkup(parsed); | |||
} else { | |||
content = _jsonMarkup(parsed); | |||
const shouldJsonTreeExpand = Object.keys(parsed).length <= 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good as a first approximation, but the right solution would be estimating the total height to be >10, not just the number of first-level children.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I guess we could work on some algorithm, It could be something like 1 property = 8 px, and then we could just go on calculating the height of the first X elements (recursively) and then decide to expand or collapse the tree
This task could be included in the upcoming LFX term
nullValue: 'json-markup-null', | ||
undefinedValue: 'json-markup-undefined', | ||
expander: 'json-markup-expander', | ||
basicChildStyle: 'json-markup-child', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please elaborate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made #2168
Which problem is this PR solving?
Resolves #1415
Description of the changes
react-json-view-lite
to render json in tree viewjsonMarkup.js
file has been removedHow was this change tested?
Before -
Now -
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test