metron-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [metron] sardell commented on issue #1553: METRON-2306: [UI] Grok parsers' have duplicate timestampField
Date Tue, 05 Nov 2019 22:47:07 GMT
sardell commented on issue #1553: METRON-2306: [UI] Grok parsers' have duplicate timestampField
URL: https://github.com/apache/metron/pull/1553#issuecomment-550058047
 
 
   @mmiklavc 
   > Anyhow, where is/was the other "TIMESTAMP" property being set if not in the "parserConfig"
object?
   
   When you fill in the TIMESTAMP input (ie. the input outside of the parser config object
UI area) and submit to create a new parser, the request looks like this:
   ```
   {
       "parserConfig": {
           "grokPath": "/apps/metron/patterns/test",
           "patternLabel": "TEST"
       },
       "fieldTransformations": [],
       "spoutConfig": {},
       "stormConfig": {},
       "timestampField": "timestamp",
       "parserClassName": "org.apache.metron.parsers.GrokParser",
       "sensorTopic": "yaf"
   }
   ```
   
   That's because the model we have implemented on the front-end expects timestampField to
be a property of the parser, but instead it's a property of parserConfig. This also explains
why we do not see the value of an existing parser in that field. The validator for that input
looks like this:
   
   ```
   group['timestampField'] = new FormControl(
         this.sensorParserConfig.timestampField,
         Validators.required
       );
   ```
   That first parameter in FormControl is what a field's default value should be. It should
be looking at `this.sensorParserConfig.parserConfig.timestampField` instead given what is
currently returned. I 
    haven't looked into whether this is the result of miscommunication at implementation or
a change in what the REST endpoint is returning, but this is why it currently doesn't work.
   
   > Specifically for Grok parsers, (e.g. Yaf, Squid) isn't this just a matter of requiring
the "timestampField" property in the config section?
   
   That's exactly what I implemented in this PR.
   
   > you might also consider making "TIMESTAMP" readonly and simply grab the value from
"timestampField", with the error message reflecting the same
   
   I could do this. Given the way we currently present the parserConfig object in the UI,
I felt it was better to keep the property in the parserConfig. However, I can be swayed to
take this approach if others feel it's better to have a separate input field.
   
   When I made this PR, I made a lot of assumptions based on what is implemented in the UI.
This is why I originally mentioned that the REST API should be updated to have timestampField
as a property on the Grok parser itself. However, if it belongs inside the parserConfig object,
I would rather include it as part of the parserConfig section in the UI. Unless anyone objects,
I'm going to proceed to add validation onto the work I did and update here when I have that
done.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message