Hello Ashish,

I like the implementation.

I don't know how often generally the userLoginId contains PII. If it is 
standard practice perhaps the default case should be HASH if not explicitly set 
instead of PLAIN, to avoid leaking information from missing configuration. 

Otherwise, thank you for taking my concerns into account in your 
implementation. :)


Best regards,

Konstantinos Marinos

On 2026/07/18 13:32:43 Ashish Vijaywargiya wrote:
> Hello Konstantinos,
> 
> I am following up on my previous conversation - here's a summary of the
> change I've built:
> 
> 1) Added log.correlation.userLoginId.mode property (plain/hash/off) in
> debug.properties controlling whether userLoginId is written raw, as a
> PBKDF2 digest, or omitted entirely from the structured JSON log correlation
> context, since userLoginId is often PII such as an email address.
> 
> 2) Added log.correlation.userLoginId.hash.pepper and
> log.correlation.userLoginId.hash.keyLengthBits properties, required only
> when mode=hash, providing the deployment-specific PBKDF2 salt and digest
> length used when hashing.
> 
> 3) Updated CorrelationValve to resolve and validate these properties at
> construction time, failing fast with an IllegalStateException at startup if
> mode=hash is set without a pepper or with a non-positive key length,
> instead of failing later on the first login attempt.
> 
> 4) Implemented hashUserLoginId() in CorrelationValve using
> PBKDF2WithHmacSHA256 with the same iteration count OFBiz already uses for
> password storage (security.properties password.encrypt.pbkdf2.iterations),
> producing a deterministic, non-reversible digest so cross-request
> correlation still works without exposing the raw userLoginId in logs.
> 
> 5) Added CorrelationValveTests covering all three modes (plain/hash/off),
> digest determinism and length, distinct users producing distinct digests,
> invalid mode falling back to plain, and fail-fast construction errors for a
> missing pepper or an invalid key length.
> 
> Please review the changes in this pass below and let me know if you have
> any questions or comments.
> PR: https://github.com/apache/ofbiz-framework/pull/1443
> Exact commit/push:
> https://github.com/apache/ofbiz-framework/pull/1443/changes/1e28d178156053b5734a9acf96f4b1d80528cf63
> 
> --
> Kind Regards,
> Ashish Vijaywargiya
> Vice President of Operations
> *HotWax Systems*
> *Enterprise open source experts*
> http://www.hotwaxsystems.com
> 
> 
> 
> On Sun, Jul 12, 2026 at 10:52 AM Konstantinos Marinos <[email protected]>
> wrote:
> 
> > Hello Ashish,
> >
> > +1, I really like that approach.
> >
> > Thank you for looking into it.
> >
> > Best regards,
> >
> > Konstantinos Marinos
> >
> > On Fri, Jul 10, 2026 at 11:35 AM Ashish Vijaywargiya <
> > [email protected]> wrote:
> >
> > > Hello Konstantinos,
> > >
> > > Thank you for your thoughtful feedback. I'm really glad you brought up
> > this
> > > important security and compliance consideration. 👍
> > >
> > > I think we can enhance the implementation by introducing configurable
> > > options such as -> plain, hash, or off for sensitive fields like
> > > `userLoginId`, allowing production environments to avoid logging
> > > PII(Personally Identifiable Information) while still preserving request
> > > correlation when needed.
> > >
> > > I'll also explore the existing hashing utilities available in OFBiz and
> > > come
> > > back with a proposed implementation plan that aligns with the framework's
> > > current capabilities.
> > >
> > > --
> > > Kind Regards,
> > > Ashish Vijaywargiya
> > > Vice President of Operations
> > > *HotWax Systems*
> > > *Enterprise open source experts*
> > > http://www.hotwaxsystems.com
> > >
> > >
> > >
> > > On Fri, Jul 10, 2026 at 11:11 AM Konstantinos Marinos <
> > > [email protected]>
> > > wrote:
> > >
> > > > Hello Ashish,
> > > >
> > > > Thank you for working on this addition. It looks very interesting.
> > > >
> > > > I just have a quick question. From my experience, the userLoginId can
> > > > contain PII (personally identifiable information)  such as the user's
> > > name
> > > > or email address.
> > > > Would there maybe be a quick way to offer the option to obfuscate or
> > hash
> > > > those values for the logs?
> > > >
> > > > Of course the ability to follow a user's activity through multiple
> > > requests
> > > > is very helpful while debugging and I wouldn't want to lose that
> > option,
> > > > but it would spare the developers, who have to worry about compliance,
> > > one
> > > > extra headache to have to deal with this matter.
> > > >
> > > > I look forward to hearing your thoughts on the matter.
> > > >
> > > > Best regards,
> > > >
> > > > Konstantinos Marinos
> > > >
> > > > On Thu, Jul 9, 2026 at 3:08 PM Ashish Vijaywargiya <
> > > > [email protected]> wrote:
> > > >
> > > > > Hello Lucas(& All),
> > > > >
> > > > > Thank you for sharing your thoughts; they were very helpful. 👍
> > > > >
> > > > > Here is the PR of my work:
> > > > > https://github.com/apache/ofbiz-framework/pull/1443
> > > > >
> > > > > Quick summary of what's done on this branch(ecs-json-logging):
> > > > >
> > > >
> > >
> > https://github.com/ashishvijaywargiya/ofbiz-framework/tree/ecs-json-logging
> > > > >
> > > > > 1. Added an opt-in JSON logging option, on top of the existing log4j
> > > > logs,
> > > > > not replacing them.
> > > > >
> > > > > 2. It's controlled by a flag in debug.properties - turn on
> > > > > json.logs.enabled to switch it on.
> > > > >
> > > > > 3. Once enabled, it writes a separate ofbiz-json.log file inside
> > > > > runtime/logs/, alongside the normal log file.
> > > > >
> > > > > 4. Each log line now carries requestId/visitId/userLoginId so we can
> > > > trace
> > > > > a single request or visit across log lines.
> > > > >
> > > > > 5. The JSON format/template is configurable too, so it can be pointed
> > > at
> > > > > different schemas (e.g. Splunk, CloudWatch) later without code
> > changes.
> > > > >
> > > > > On my laptop, I've also set up Grafana + Elastic + OpenSearch via
> > > Docker,
> > > > > just watching this ofbiz-json.log file and picking up new entries
> > > > > automatically
> > > > > - this is a local setup for trying it out, not part of the PR itself.
> > > > >
> > > > > Lucas, yes, this stays strictly opt-in on top of log4j, nothing is
> > > > > replaced.
> > > > >
> > > > > On querying: there's no built-in query interface in the PR itself;
> > > > querying
> > > > > is handled externally by tools like Grafana/Elastic/OpenSearch
> > watching
> > > > the
> > > > > JSON log file, as I'm testing locally.
> > > > >
> > > > > Here are the screenshots of watching the ofbiz logs generated in
> > > ECS/JSON
> > > > > format using the following stacks (Grafana + Elastic + OpenSearch),
> > > > sharing
> > > > > it again from the previous email, just for everyone's easy reference:
> > > > >
> > > > > Grafana-Loki-Promtail(shared yesterday)
> > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/drive/folders/1PD0BLKa4weAXEf16aegEC2DAfKLcgZhw?usp=sharing
> > > > >
> > > > > Elasticsearch-Kibana-Filebeat
> > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/drive/folders/1OvKrQYZZxN4GPlQA9rR6tzU54IGjv_Mf?usp=sharing
> > > > >
> > > > > OpenSearch-OpenSearchDashboards-FluentBit
> > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/drive/folders/1cIrnEyPS91u7GR38hdOPwt8JixFhHwdP?usp=drive_link
> > > > >
> > > > > Please let me know if you require further details on this work.
> > > > >
> > > > > Very soon, I will prepare a document on OFBiz Wiki for this addition
> > > and
> > > > > how anyone can use this feature.
> > > > > Thank you.
> > > > >
> > > > > --
> > > > > Kind Regards,
> > > > > Ashish Vijaywargiya
> > > > > Vice President of Operations
> > > > > *HotWax Systems*
> > > > > *Enterprise open source experts*
> > > > > http://www.hotwaxsystems.com
> > > > >
> > > > >
> > > > >
> > > > > On Thu, Jul 9, 2026 at 1:20 PM Lukas Finster <
> > [email protected]
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Ashish,
> > > > > >
> > > > > > it looks interesting and i can see this potentially beeing very
> > > > usefull.
> > > > > >
> > > > > > I definitly would stick to keeping it an opt-in feature on top of
> > the
> > > > > > existing log4j instead of a replacement.
> > > > > >
> > > > > > One question I am wondering about is how querying will work.
> > > > > >
> > > > > > Will your PR include some type of interface for querying the logs?
> > > > > >
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Lukas
> > > > > >
> > > > > >
> > > > > >
> > > > > > Am 09.07.26 um 09:36 schrieb Ashish Vijaywargiya:
> > > > > > > Hello Dev Members,
> > > > > > >
> > > > > > > Quick update on the structured logging work I shared yesterday -
> > > the
> > > > > > > implementation is finalized now and I've tested it end to end.
> > > > > > >
> > > > > > > 1. json.logs.enabled and json.logs.template switches added in
> > > > > > > framework/base/config/debug.properties, default off, default is
> > the
> > > > > > bundled
> > > > > > > ECS template.
> > > > > > >
> > > > > > > 2. build.gradle reads both and passes them as JVM system
> > properties
> > > > > > > automatically - plain ./gradlew ofbiz picks them up, no extra
> > flags
> > > > > > needed.
> > > > > > >
> > > > > > > 3. log4j2.xml has one new opt-in JSON appender writing ECS
> > > formatted
> > > > > > lines
> > > > > > > to runtime/logs/ofbiz-json.log alongside the existing text logs,
> > > > > nothing
> > > > > > > else changes.
> > > > > > >
> > > > > > > 4. Correlation fields (requestId, visitId, userLoginId) now come
> > > > from a
> > > > > > new
> > > > > > > CorrelationValve in framework/catalina, running as a Tomcat valve
> > > > ahead
> > > > > > of
> > > > > > > the filter chain - ControlFilter stays untouched.
> > > > > > >
> > > > > > > 5. Added CorrelationFieldProvider, a ServiceLoader based
> > extension
> > > > > point
> > > > > > -
> > > > > > > any component can add extra fields like security group or
> > > permissions
> > > > > > with
> > > > > > > zero framework changes.
> > > > > > >
> > > > > > > 6. All providers get merged; a provider overwriting a core field
> > is
> > > > > > allowed
> > > > > > > but logged as a warning, and a failing provider is caught and
> > > skipped
> > > > > > > without breaking the request.
> > > > > > >
> > > > > > > 7. Tested live on a running OFBiz instance - anonymous request,
> > > > > > > session/visit, and login correlation, plus the provider
> > > > > > > merge/overwrite/failure cases, all confirmed from real log
> > output.
> > > > > > >
> > > > > > > 8. Tested against two separate observability stacks to confirm
> > the
> > > > > output
> > > > > > > is genuinely portable - Elastic (Elasticsearch + Kibana +
> > Filebeat)
> > > > and
> > > > > > > OpenSearch (OpenSearch + OpenSearch Dashboards + Fluent Bit).
> > > > > > >
> > > > > > > 9. Both ingested the same ofbiz-json.log with no changes, and I
> > > got a
> > > > > > > working dashboard (log volume by level plus a live search) with
> > > > > real-time
> > > > > > > data in each.
> > > > > > >
> > > > > > > 10. Also confirmed the template placeholder is genuinely
> > swappable
> > > at
> > > > > > > runtime through debug.properties, tested with a different schema,
> > > not
> > > > > > just
> > > > > > > the default ECS one.
> > > > > > >
> > > > > > > Here are the screenshots of watching the ofbiz logs generated in
> > > > > ECS/JSON
> > > > > > > format using following stacks (Grafana + Elastic + OpenSearch):
> > > > > > >
> > > > > > > Grafana-Loki-Promtail(shared yesterday)
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/drive/folders/1PD0BLKa4weAXEf16aegEC2DAfKLcgZhw?usp=sharing
> > > > > > >
> > > > > > > Elasticsearch-Kibana-Filebeat
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/drive/folders/1OvKrQYZZxN4GPlQA9rR6tzU54IGjv_Mf?usp=sharing
> > > > > > >
> > > > > > > OpenSearch-OpenSearchDashboards-FluentBit
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/drive/folders/1cIrnEyPS91u7GR38hdOPwt8JixFhHwdP?usp=drive_link
> > > > > > >
> > > > > > > I will share the PR for this work soon.
> > > > > > > Thank you.
> > > > > > >
> > > > > > > --
> > > > > > > Kind Regards,
> > > > > > > Ashish Vijaywargiya
> > > > > > > Vice President of Operations
> > > > > > > *HotWax Systems*
> > > > > > > *Enterprise open source experts*
> > > > > > > http://www.hotwaxsystems.com
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Wed, Jul 8, 2026 at 9:01 PM Ashish Vijaywargiya <
> > > > > > > [email protected]> wrote:
> > > > > > >
> > > > > > >> Hello OFBiz Dev Members,
> > > > > > >>
> > > > > > >> Here I am sharing my notes on which I've been working through:
> > > > opt-in
> > > > > > >> structured (ECS - Elastic Common Schema) JSON logging on top of
> > > the
> > > > > > >> existing log4j2.xml.
> > > > > > >>
> > > > > > >> 1. Today's OFBiz logs are plain text, so every downstream
> > consumer
> > > > > needs
> > > > > > >> its own regex/grok parser before the data is even queryable -
> > > that's
> > > > > the
> > > > > > >> core problem this addresses.
> > > > > > >>
> > > > > > >> 2. Added a JSON appender to the existing
> > > > > > >> framework/base/config/log4j2.xml, writing ECS (Elastic Common
> > > > Schema)
> > > > > > >> formatted lines to runtime/logs/ofbiz-json.log, alongside the
> > > > current
> > > > > > >> console/ofbiz.log/error.log output - nothing existing changes
> > > shape
> > > > or
> > > > > > >> content.
> > > > > > >>
> > > > > > >> 3. It's fully opt-in and off by default: a single
> > > > > > json.logs.enabled=false
> > > > > > >> switch in framework/base/config/debug.properties. Flip it to
> > true
> > > > and
> > > > > > the
> > > > > > >> default ./gradlew ofbiz command picks it up on its own - no
> > extra
> > > > flag
> > > > > > to
> > > > > > >> remember or document.
> > > > > > >>
> > > > > > >> 4. ECS was chosen as the default (and only bundled) schema
> > because
> > > > > it's
> > > > > > an
> > > > > > >> industry standard that Elastic, Grafana, and most log shippers
> > > > > > (Promtail,
> > > > > > >> Fluent Bit, Filebeat) already understand out of the box, with no
> > > > > custom
> > > > > > >> mapping needed.
> > > > > > >>
> > > > > > >> 5. log4j2.xml exposes a jsonLogTemplateUri placeholder
> > (defaulting
> > > > to
> > > > > > the
> > > > > > >> bundled ECS template), overridable via
> > > > > > >> -Dofbiz.json.logs.template=classpath:your-template.json - so a
> > > > > > >> Splunk/CloudWatch/anything-else template can live in a separate
> > > > custom
> > > > > > >> component and be swapped in with zero framework changes.
> > > > > > >>
> > > > > > >> 6. Every log line also carries requestId, visitId, and
> > userLoginId
> > > > > > >> (populated in ControlFilter via Log4j2's ThreadContext/MDC), so
> > > all
> > > > > > lines
> > > > > > >> from one HTTP request or one user visit can be pulled with a
> > > single
> > > > > > query -
> > > > > > >> correlation the current plain-text logs don't support at all.
> > > > > > >>
> > > > > > >> 7. JSON-over-file also fits containerized/Kubernetes deployments
> > > > > better
> > > > > > >> than relying solely on local rolling text files, which assume
> > > > > > persistent,
> > > > > > >> non-ephemeral disk.
> > > > > > >>
> > > > > > >> 8. The whole thing lives inside the existing log4j2.xml behind a
> > > > > > >> Select/SystemPropertyArbiter condition.
> > > > > > >>
> > > > > > >> 9. How to test it: Setup a local observability/ stack (Grafana +
> > > > Loki
> > > > > +
> > > > > > >> Promtail via docker-compose) to try this out end-to-end.
> > > > > > >>
> > > > > > >> I completed the first pass of the above notes. Here are the
> > > > > screenshots
> > > > > > of
> > > > > > >> watching the ofbiz logs generated in ECS/JSON format using
> > > Grafana +
> > > > > > Loki
> > > > > > >> + Promtail.
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/file/d/1SJRqFKNT0dtJ5mBP2fsC7YSKY-BP97Wo/view?usp=drive_link
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/file/d/1J6RlrjnQawFFwtu4YO8V2HzlbEJD5hZ-/view?usp=drive_link
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/file/d/1AKsdcGg-N1WL7k2uClUdilQoOu2P3MLL/view?usp=drive_link
> > > > > > >>
> > > > > > >>
> > > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> > https://drive.google.com/file/d/181f5kSNwfCieA_z79Z2IEjw0O7PgIPWX/view?usp=drive_link
> > > > > > >>
> > > > > > >> Here are the reference documents that I considered:
> > > > > > >>
> > > > > > >> - Elastic Common Schema (ECS) spec:
> > > > > > >> https://www.elastic.co/guide/en/ecs/current/index.html
> > > > > > >>
> > > > > > >> - ECS field reference:
> > > > > > >>
> > > > https://www.elastic.co/guide/en/ecs/current/ecs-field-reference.html
> > > > > > >>
> > > > > > >> - ECS GitHub repo: https://github.com/elastic/ecs
> > > > > > >>
> > > > > > >> - Log4j2 JsonTemplateLayout (the mechanism used here, includes
> > the
> > > > > > bundled
> > > > > > >> ECS template):
> > > > > > >>
> > > > https://logging.apache.org/log4j/2.x/manual/json-template-layout.html
> > > > > > >>
> > > > > > >> I will be sharing a PR soon for everyone's reference.
> > > > > > >>
> > > > > > >> Please let me know your thoughts on this feature.
> > > > > > >> Thank you.
> > > > > > >>
> > > > > > >> --
> > > > > > >> Kind Regards,
> > > > > > >> Ashish Vijaywargiya
> > > > > > >> Vice President of Operations
> > > > > > >> *HotWax Systems*
> > > > > > >> *Enterprise open source experts*
> > > > > > >> http://www.hotwaxsystems.com
> > > > > > >>
> > > > > > >>
> > > > > > --
> > > > > > Lukas Finster
> > > > > > Softwareentwickler & Berater
> > > > > >
> > > > > > ecomify GmbH, Stralsunder Straße 63, 33605 Bielefeld
> > > > > > Fon: +49 521 448157-90 | Fax: +49 521 448157-99 | www.ecomify.de
> > > > > > Court Registration: Amtsgericht Bielefeld, HRB 41683 | CEO: Martin
> > > > > Becker,
> > > > > > Michael Brohl
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> 

Reply via email to