Aias00 commented on code in PR #7261:
URL: https://github.com/apache/shenyu/pull/7261#discussion_r4110269261


##########
shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-elasticsearch/src/main/java/org/apache/shenyu/plugin/logging/elasticsearch/client/ElasticSearchLogCollectClient.java:
##########
@@ -150,7 +150,11 @@ public boolean existsIndex(final String indexName) {
      */
     public void createIndex(final String indexName) {
         try {
-            client.indices().create(c -> c.index(indexName));
+            client.indices().create(c -> c.index(indexName).mappings(mapping 
-> mapping

Review Comment:
   Request changes - this mapping can drop every log line in a supported 
configuration.
   
   I confirmed the format is right: `LoggingServerHttpResponse.java:65` 
declares `DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSS")` and formats 
every value at `:163` and `:257`, exactly matching what is declared here. The 
problem is what happens to that value before it reaches Elasticsearch.
   
   `AbstractLogCollector.java:187` masks every log field through the 
desensitization helper, including this one:
   
   ```java
   
logInfo.setTimeLocal(desensitizeForSingleWord(GenericLoggingConstant.TIME_LOCAL,
 logInfo.getTimeLocal(), keyWordMatch, desensitizedAlg));
   ```
   
   and `DataDesensitizeUtils.java:73-77` replaces the value whenever the 
keyword matches:
   
   ```java
   if (StringUtils.hasLength(source) && desensitized && 
keyWordMatch.matches(keyWord)) {
       return DataDesensitizeFactory.selectDesensitize(source, desensitizeAlg); 
  // masked string, not a timestamp
   }
   ```
   
   Once `timeLocal` is a strict `date`, that masked string no longer parses and 
**Elasticsearch rejects the entire document** with `mapper_parsing_exception` - 
not just the field. Masking applies uniformly, so enabling desensitization 
silently loses the whole day's access log, surfacing only as bulk-response 
errors elsewhere.
   
   Before this patch `timeLocal` was dynamic `text`, so masking it was 
pointless but harmless.
   
   Cheapest fix, and the one I would pick: add `.ignoreMalformed(true)` to the 
four mappings, so a bad value is stored but not indexed instead of killing the 
document. Alternatives: exclude the date/numeric fields from 
`desensitizeShenyuRequestLog`, or add a test that runs the desensitized 
pipeline through a real create-index request.
   
   Adjacent, pre-existing, worth fixing in the same pass since you are giving 
these types: `AbstractLogCollector.java:189-201` masks `responseContentLength` 
/ `status` / `upstreamResponseTime` and immediately re-parses with 
`Integer.valueOf(...)` / `Long.valueOf(...)`, which throws 
`NumberFormatException` today.
   



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to