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]