search

philterd / phileas

The open source PII and PHI redaction engine
https://www.philterd.ai
Apache License 2.0
24 stars 5 forks source link

Removing metrics service implementation #133

Closed jzonthemtn closed 2 months ago

jzonthemtn commented 2 months ago

Removing metrics service implementation for #122.

robfromboulder commented 2 months ago

The functionality here looks good -- but I think I'm seeing a significant drop in performance with these changes. I ran out of time tonight to gather more details but wanted to give a head's up that this might need more testing before merging.

jzonthemtn commented 2 months ago

Interesting. It's probably the LoggingMetricsService that uses log4j to output those metrics. I will take a look.

jzonthemtn commented 2 months ago

Changed to NoOpMetricsService. The other one will go into Philter. Performance is more important than metrics, by default.

robfromboulder commented 2 months ago

Sounds good!

BTW I think the issue was logFilterTime() was calling LOGGER.debug(...) where the debug string is expensive to calculate. Changing this to if (LOGGER.isDebugEnabled()) LOGGER.debug(...) seemed to resolve the issue.

Agree that NoOpMetricsService is a better default -- but you may wanna add that isDebugEnabled check on the Philter side?

robfromboulder commented 2 months ago

Just tried a few performance tests and it's looking good now, many thanks!