chipmunk icon indicating copy to clipboard operation
chipmunk copied to clipboard

Allow nested parsers on MessageProducer level (v3)

Open AmmarAbouZor opened this issue 1 year ago • 3 comments

This PR isn't for to be merged. It's a suggestion in the scope of work on #2073 and #2078 It's inspired the suggestion in #2071

  • LogMessage doesn't implement Display anymore. Instead, it has the method try_resolve() which can provide an enum of either a text message if the message can be resolved or a template of resolved and unresolved chunks when a full resolve isn't possible + Resolve hints for the session to know which method should be provided to resovle the rest of the chunks.
  • The session has a ParseRestResolver struct which can try to resolved the rest unresolved chunks in the templates depending on the error hint, by having a SomeIP parser in the case of SomeIP Error hint.
  • If the resolver fails to parse the rest of the message, we can add the message to the faulty messages store ( When should will be implemented as part of #2070 )

Here are basic benchmarks for the three PRs to solve the DLT with SomeIP case:

++++++++++++++  V1 ++++++++++++++ 

Run 1: 4051
Run 2: 4087
Run 3: 3931
Run 4: 4050
Run 5: 3989

Avg: (4051 + 4087 + 3931 + 4050 + 3989) / 5 = 4021.6


++++++++++++++  V2  ++++++++++++++

Run 1: 3852
Run 2: 3832
Run 3: 3805
Run 4: 3731
Run 5: 3788

Avg: (3852 + 3832 + 3805 + 3731 + 3788) / 5 = 3801.6


++++++++++++++  V3  ++++++++++++++

Run 1: 3917
Run 2: 4007
Run 3: 3818
Run 4: 3894
Run 5: 3875

Avg: (3917 + 4007 + 3818 + 3894 + 3875) / 5 = 3902.2

AmmarAbouZor avatar Aug 30 '24 14:08 AmmarAbouZor

I still think this kind of solutions should be implemented only if the standard case of processing the log is with various nested parsers. Otherwise, it would better to nest SomeIP within DLT parser to not change the general architectures to resolve a special case.

AmmarAbouZor avatar Sep 02 '24 11:09 AmmarAbouZor

@AmmarAbouZor thanks much for the possible solution. Here is a short summary by all current variants

Criteria V.1 (current state) V.2 V.3
Owner of nested parser initial parser Producer Producer
Scope of calling nested parser Display() of initial parser Producer Loop Producer loop
Call of nested parser During rendering Before rendering Before rendering
Nested parser error part of row (log message) bound error msg bound error msg
Render row By initial parser By initial parser By Producer with template
Changes in Producer's loop no yes yes
Changes in LogMessage trait no yes yes
  • rendering - converting to string (before write into session file) If I miss something, let me know please

DmitryAstafyev avatar Sep 04 '24 07:09 DmitryAstafyev

@AmmarAbouZor thanks much for the possible solution. Here is a short summary by all current variants

Hi @DmitryAstafyev, thanks for the useful summery for the three approaches. I think I may have a small remark on scope of V3 because this solution actually replaces Display() in the rendering phase with the new try_resolve() method, which makes calling the nested parsers rather in the rendering call than in the producer loop itself, or it sits in between.

AmmarAbouZor avatar Sep 05 '24 07:09 AmmarAbouZor

Solution has been mentioned in the issue #2294 We can close all PR approaches for now and group them in that one issue

AmmarAbouZor avatar Apr 11 '25 12:04 AmmarAbouZor