Skip to content

feat: add app logging - #2616

Open
ppatel9703 wants to merge 16 commits into
devfrom
app-logs
Open

feat: add app logging#2616
ppatel9703 wants to merge 16 commits into
devfrom
app-logs

Conversation

@ppatel9703

@ppatel9703 ppatel9703 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Description

Producer side of app-owner log visibility: tags log lines with the project they belong to, writes them to a per-project file, and exposes a disk-based search over that file (and its rotated history) for a searchable "Logs" tab. Live tail delivery is not in this repo — that's Monolith#398, which streams this same file over the existing insight websocket. The frontend consuming both is semoss-ui#3302.

Changes Made

src/prerna/sablecc2/comm/PixelJobRunner.java
Tags the Log4j2 MDC with projectId for the duration of a pixel job, via insight.getContextProjectId() — the project this insight is currently scoped to (set by SetContext(...)). Originally read insight.getProjectId(), a different field entirely ("the project this insight is saved in"), which stays null for any transient REPL-style insight — Terminal's, Console's — that's only ever scoped via SetContext. That bug meant MDC tagging silently never fired for pixels run from Terminal, while looking like it worked everywhere else by coincidence (some other insight-loading paths populate the base field too).

log4j2.xml
Added [project=%X{projectId}] to the global Console/RollingRandomAccessFile patterns, purely so a human grepping semossLog.log can see which project a line belongs to. Unrelated to the per-project routing below.

src/prerna/logging/AppLogManager.java (already present, one fix in this PR)
Per-project Log4j2 RollingFileAppender factory — writes {projectVersionFolder}/logs/app.log (50MB × 10 rotations), filtered to only that project's MDC-tagged events. This PR's fix: now also attaches to the root logger (LogManager.ROOT_LOGGER_NAME), not just prerna/EngineLogger. Custom app reactors almost never live under prerna.* — a typical app's reactors are in their own package (e.g. reactors.vaapi.*), a sibling namespace to prerna, not a descendant, so they fall through to root instead of inheriting prerna's appenders. Without this, a custom reactor's own logger.error(...) calls never reached the per-project file no matter how much logging an app author added.
Known remaining gap: root's own level threshold (WARN in log4j2.xml) still drops INFO/DEBUG from custom packages before any appender sees them — fixing that means lowering Root's level platform-wide, a bigger, separate call.

src/prerna/logging/GetAppLogsReactor.java (deleted)
Was a polling reactor (offset/nextOffset client-side polling). Superseded entirely — live tail is now a websocket push from Monolith (AppLogStreamer), and historical reads are the new search reactor below. No reactor call is needed for either anymore.

src/prerna/logging/SearchAppLogsReactor.java (new)
Reads app.log plus its rotated siblings (app.log.1....10) directly from disk — no database. Filters by text substring and/or level, paginated (offset/limit, capped at 500), newest-first. Owner-gated, same rule the old reactor used (SecurityProjectUtils.getUserProjectPermission + AccessPermissionEnum.isOwner).
Deliberately not a DB-backed appender (an AuditLogsJDBCAppender-style table was the original plan) — app.log's own rotation already bounds storage per project, so a DB would've solved a retention problem that doesn't exist here, at the cost of a new schema/write path with no retention story of its own (the audit-log precedent doesn't have a purge job either).

How to Test

  1. Open any app you own, run some pixels/reactors against it.
  2. Confirm {projectVersionFolder}/logs/app.log exists and only contains this project's lines.
  3. Add a logger.error(...)/logger.warn(...) call to a custom-package reactor (not under prerna.*) in that app, trigger it, confirm the line shows up in app.log (this is the root-logger fix — would not have worked before this PR).
  4. Call SearchAppLogs(paramValues=[{"projectId": "<id>", "query": "...", "levels": "ERROR,WARN", "offset": "0", "limit": "50"}]); — confirm results are newest-first, correctly filtered, and paginate via hasMore/totalMatches.
  5. Confirm a non-owner calling SearchAppLogs gets rejected.
  6. Run a pixel from a Terminal/Console-style insight (one only ever SetContext'd, never setProjectId'd directly) — confirm its logs now carry [project=...] in the global log too (the PixelJobRunner fix).

Notes

  • How the three repos fit together: this repo tags + writes + searches the per-project file. Monolith (AppLogStreamer) tails that same file and pushes it live over /insightSocket. semoss-ui has two consumers: a live-tail Console panel (talks to Monolith's socket directly) and a searchable Logs page (talks to SearchAppLogsReactor here via runQuery).
  • Pod-locality: app.log is local to whichever pod's disk wrote it — no cross-pod visibility in a multi-replica deployment.
  • Cluster sync risk: the file lives inside the project version folder, which rclone sync pushes to S3 with no exclude list (only fires from explicit actions — build/publish, upload, GitHub sync — never automatically). Two pods each syncing their own copy will clobber each other's history on whichever push lands last. Not fixed in this PR.
  • Full architecture write-up (MDC background, full producer/delivery/consumer chain, known limitations): docs/app-logs/architecture.md in the workspace root (not part of this repo).

Review Updates

Addressed the following from review:

  • Stripped em-dashes/typographic characters from comments and log strings in AppLogManager.java, SearchAppLogsReactor.java, PixelJobRunner.java, Insight.java — this repo's Maven compiler plugin enforces cp1252 source encoding, and non-ASCII punctuation risks build/encoding issues.
  • SearchAppLogsReactor: added a user == null || user.getPrimaryLoginToken() == null guard before dereferencing the token, matching the existing convention elsewhere in the codebase (e.g. AgentRuntimeManager).
  • Insight.java: fixed an unused FileSystems import (was imported but referenced fully-qualified) and corrected import ordering.

@ppatel9703
ppatel9703 requested a review from a team as a code owner June 9, 2026 18:14
@snyk-io

snyk-io Bot commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants