Preserve nesting for exporting events in JSON format #1257

tobim · 2021-01-04T14:50:14Z

📔 Description

What the title says. Layout types now keep nested records as-is. The JSON writer reconstructs the layers while printing.

📝 Checklist

All user-facing changes have changelog entries.
The changes are reflected on docs.tenzir.com/vast, if necessary.
The PR description contains instructions for the reviewer, if necessary.
Fix taxonomy integration test.

🎯 Review Instructions

By commit, but do a quick first pass over the list before reviewing properly, several issues are fixed in later commits.

mavam

Looks like you forgot to add table_slice.hpp changes? I'm curious what drives the need for the at overload.

libvast/src/format/csv.cpp

mavam · 2021-01-08T07:44:28Z

libvast/src/format/pcap.cpp

    auto& payload = caf::get<view<std::string>>(payload_field);
    // Make PCAP header.
    ::pcap_pkthdr header;
-    auto ns_field = slice.at(row, 0);
+    auto ns_field = slice.at(row, 0, *pcap_packet_type.at("time"));


A bit unrelated: I never liked the manual integer offsets to get to a field. Maybe we can factor this logic to happen dynamically when the writer gets instantiated?

I think that belongs to another PR.

tobim · 2021-01-08T11:38:33Z

@mavam:

Looks like you forgot to add table_slice.hpp changes? I'm curious what drives the need for the at overload.

It's in Cache the type for column based data access

mavam · 2021-01-08T12:01:41Z

It's in Cache the type for column based data access

Ah, got it. The link yields a 404 for me, though,

dominiklohmann · 2021-01-08T12:16:23Z

Nit: Can you add an assertion to the overload that checks whether the given type is correct?

mavam · 2021-01-08T13:54:04Z

Weird CI failure...

dominiklohmann · 2021-01-08T14:03:31Z

Weird CI failure...

Why? This is exactly #1261, this PR just needs to be rebased.

libvast/src/msgpack_table_slice.cpp

dominiklohmann

This PR is great. I've done some performance measurements, and the differences are barely noticeable for Arrow, but show a slight negative impact for MessagePack. We should probably cache the flat layout in there as well as suggested by my previous review.

Additionally, please expand the documentation of the new table_slice::at function.

libvast/vast/table_slice.hpp

libvast/vast/type.hpp

tobim · 2021-01-13T11:04:55Z

Feedback is integrated and PR is rebased to fix the merge conflict.

dominiklohmann

I tested this again locally. Performance is alright, the loss is irrelevant in the grand scheme of things.

Thanks for working on this, it's a big UX improvement.

The only application of it in `tailor` was redundant.

The layout is always available from the outer context. Now it only contains the type of the field that the offset points to.

Currently only `table_slice_row` uses the 2 argument overload.

- Print mismatched outputs at the WARNING level - Demote "Running step X ..." to DEBUG - Use INFO by default

dominiklohmann · 2021-01-13T16:41:22Z

@tobim you may have to do some adjustments on this because of #1230. If you wanna pair on the adjustments, feel free to ping me in Slack or here.

tobim added the feature New functionality label Jan 4, 2021

tobim force-pushed the story/ch20399/nested-layouts branch 3 times, most recently from b91df3f to 769626a Compare January 6, 2021 15:56

tobim marked this pull request as ready for review January 6, 2021 16:10

tobim requested a review from a team January 6, 2021 16:10

mavam reviewed Jan 8, 2021

View reviewed changes

tobim force-pushed the story/ch20399/nested-layouts branch 2 times, most recently from a9a36df to fa78535 Compare January 8, 2021 16:32

dominiklohmann reviewed Jan 12, 2021

View reviewed changes

libvast/src/msgpack_table_slice.cpp Outdated Show resolved Hide resolved

0snap mentioned this pull request Jan 12, 2021

Remove unflatten from pyvast-threatbus tenzir/threatbus#92

Merged

3 tasks

dominiklohmann reviewed Jan 12, 2021

View reviewed changes

libvast/vast/table_slice.hpp Show resolved Hide resolved

mavam reviewed Jan 13, 2021

View reviewed changes

libvast/vast/type.hpp Outdated Show resolved Hide resolved

dominiklohmann reviewed Jan 13, 2021

View reviewed changes

libvast/vast/type.hpp Outdated Show resolved Hide resolved

tobim force-pushed the story/ch20399/nested-layouts branch from f87a753 to a20534b Compare January 13, 2021 10:56

dominiklohmann approved these changes Jan 13, 2021

View reviewed changes

tobim force-pushed the story/ch20399/nested-layouts branch from a20534b to 161e15c Compare January 13, 2021 15:47

tobim added 7 commits January 13, 2021 17:33

Preserve nested records in table slice layouts

d248281

Add support for printing nested records

7ad361d

Fix concept resolution

b9d861f

Fix taxonomies integration test

54ba38c

Adjust explorer integration test reference

16f6761

Avoid flattening the layout on data access

1fb9e15

Cache the type for column based data access

c3b9e29

tobim added 20 commits January 13, 2021 17:33

Add a type function to record_type::range_state

ba3a160

Avoid layout flattening in format implementations

78de7f5

Avoid flattening in the meta index

1e4fd47

Let writer constructors take caf::settings as argument

57bd929

Remove default constructors from writers

a64d0e4

Add a changelog entry

6c11521

Remove suboptimal todo comment

ed8e95e

Remove the type_pruner

75cf2f0

The only application of it in `tailor` was redundant.

Remove the full layout from the data extractor

61ce1a2

The layout is always available from the outer context. Now it only contains the type of the field that the offset points to.

Pass the type to table_slice::at where it makes sense

15834bf

Currently only `table_slice_row` uses the 2 argument overload.

Sanitize integration test logging

c736ae6

- Print mismatched outputs at the WARNING level - Demote "Running step X ..." to DEBUG - Use INFO by default

Fix formatting nested events

f6930a8

Fix the log level parameter for the integration target

84ad158

Add assertions for {arrow,msgpack}_table_slice::at

884c71a

Fixup newly introduced assertion failures

fc1b923

Fix the msgpack table slice builder

a4ffbe2

Avoid excessive calls to flatten()

53e6c86

Document the additional at overload

614b8a5

Fix formatting

0894eaa

Fix python formatting

6dd99a8

Fixup simdjson tests

01eb415

tobim force-pushed the story/ch20399/nested-layouts branch from 774cbe5 to 01eb415 Compare January 13, 2021 20:05

tobim merged commit 1008990 into master Jan 13, 2021

tobim deleted the story/ch20399/nested-layouts branch January 13, 2021 21:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preserve nesting for exporting events in JSON format #1257

Preserve nesting for exporting events in JSON format #1257

tobim commented Jan 4, 2021 •

edited

Loading

mavam left a comment

mavam Jan 8, 2021

tobim Jan 13, 2021

tobim commented Jan 8, 2021

mavam commented Jan 8, 2021

dominiklohmann commented Jan 8, 2021

mavam commented Jan 8, 2021

dominiklohmann commented Jan 8, 2021 •

edited

Loading

dominiklohmann left a comment

tobim commented Jan 13, 2021

dominiklohmann left a comment

dominiklohmann commented Jan 13, 2021

Preserve nesting for exporting events in JSON format #1257

Preserve nesting for exporting events in JSON format #1257

Conversation

tobim commented Jan 4, 2021 • edited Loading

📔 Description

📝 Checklist

🎯 Review Instructions

mavam left a comment

Choose a reason for hiding this comment

mavam Jan 8, 2021

Choose a reason for hiding this comment

tobim Jan 13, 2021

Choose a reason for hiding this comment

tobim commented Jan 8, 2021

mavam commented Jan 8, 2021

dominiklohmann commented Jan 8, 2021

mavam commented Jan 8, 2021

dominiklohmann commented Jan 8, 2021 • edited Loading

dominiklohmann left a comment

Choose a reason for hiding this comment

tobim commented Jan 13, 2021

dominiklohmann left a comment

Choose a reason for hiding this comment

dominiklohmann commented Jan 13, 2021

tobim commented Jan 4, 2021 •

edited

Loading

dominiklohmann commented Jan 8, 2021 •

edited

Loading