chore: add SQL comment logging with MDC context DHIS2-20921#22988
Draft
chore: add SQL comment logging with MDC context DHIS2-20921#22988
Conversation
Remove proxyResultSet() (unused proxy overhead), executeAfterMethod stack-walking (expensive per JDBC call), redundant elapsed time callback, and ANSI escape codes from log formatting (HIGHLIGHT to BASIC). Deprecate METHOD_QUERY_LOGGING_ENABLED and ELAPSED_TIME_QUERY_LOGGING_ENABLED for removal in v44.
…ansformer Prepend SQL comments with controller, action, sessionId, and xRequestID from MDC to every query when method.query.logging.enabled is on. This makes queries visible in pg_stat_activity/pg_stat_statements without needing DHIS2 query logging enabled.
Add javadoc warning that MDC values are embedded verbatim in SQL comments and must be validated before being put into MDC. Enable query logging and comments in test performance dhis.conf.
Propagate MDC context from the HTTP thread to the TRACKER-TE-FETCH thread pool so SQL comments appear on queries executed by TrackedEntityAggregate and EnrollmentAggregate. Rename MDC key "action" to "method" for clarity.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



SQL Comment Logging
Prepends a SQL comment with request context to every SQL statement at the JDBC level using
datasource-proxy's
QueryTransformer. Covers all queries (Hibernate HQL, Spring JDBC, native SQL)on both the main and analytics data sources.
What it does
Every SQL statement gets a comment with up to 4 keys (
controller,method,sessionId,xRequestID). Missing keys are left out.SQL snippet:
DHIS2 application log (
enable.query.logging = on):PostgreSQL server log (
log_min_duration_statement = 0):pg_stat_activitypg_stat_statementsDHIS2 Configuration
All settings in
dhis.conf:logging.query.commentsoffenable.query.loggingofforg.hisp.dhis.datasource.queryslow.query.logging.threshold.time1000enable.query.logging)logging.request_id.enabledonsessionIdMDC key (hashed session ID)Config interactions
We only create the proxy datasource when at least one of
logging.query.commentsorenable.query.loggingison. Otherwise the raw datasource is used with zero overhead.logging.query.commentsenable.query.loggingpg_stat_activityor PostgreSQL server logs without DHIS2 application log noise.org.hisp.dhis.datasource.queryat INFO, slow queries at WARN.dhis.confuses this.MDC keys in the comment
controllerSqlCommentInterceptor(MVC interceptor, always registered)methodSqlCommentInterceptorsessionIdSessionIdFilter(gated bylogging.request_id.enabled, defaulton)logging.request_id.enabled = offto omit. Only for authenticated users.xRequestIDRequestIdFilter(readsX-Request-IDheader, no config gate)X-Request-IDheaderWith default config and
logging.query.comments = on:X-Request-ID:/* controller='...',method='...',sessionId='...' */X-Request-ID: all 4 keys/* sessionId='...' */onlysessionIdlogging.request_id.enabled = off: nosessionIdregardless of authenticationSecurity
xRequestIDis user-supplied input embedded in a SQL comment.RequestIdFiltersanitizes it to[-_a-zA-Z0-9]{1,36}, replacing anything else with(illegal). The other keys are safe:controllerandmethodcome from Java class/method names,sessionIdis a base64-encodedSHA-256 hash.
PostgreSQL considerations
pg_stat_activitypg_stat_activity.querygets truncated attrack_activity_query_size(default 1024 bytes). Thecomment adds ~100-150 bytes. Users with long queries (common in tracker) should increase this:
pg_stat_statementspg_stat_statementsnormalizes queries (replaces literals with$N) but preserves SQL comments.So the same query shape with different comments ends up as separate entries. Since
xRequestIDvaries per request, you effectively get one entry per request instead of one per query shape.
This means:
pg_stat_statements.max(default 5000) fills up fast, causing constant evictionIf you need
pg_stat_statementsaggregation:logging.query.commentsin that environmentregexp_replace(query, '/\*.*?\*/', '')pg_stat_activityfor per-request correlation and keeppg_stat_statementsclean byleaving comments off
PostgreSQL 14+ has
pg_stat_statements.track_utilityandcompute_query_idbut neither stripscomments during normalization.
When to use DHIS2 logging vs PostgreSQL logging
logging.query.comments)log_min_duration_statement)xRequestIDgroups themenable.query.logging+ thresholdlog_min_duration_statement(more reliable)pg_stat_statementspg_stat_statementsworks fineauto_explainlogs planslog_min_duration_statementUse DHIS2 comments when you need to know which endpoint causes which queries (N+1 detection,
attributing load). Use PostgreSQL native logging (
log_min_duration_statement,auto_explain,pg_stat_statements) for production monitoring and aggregation.