Skip to content

Add alerting system client - #4

Merged
alishazaee merged 1 commit into
masterfrom
Add-client-to-alerting-system
Jul 5, 2026
Merged

Add alerting system client#4
alishazaee merged 1 commit into
masterfrom
Add-client-to-alerting-system

Conversation

@alishazaee

@alishazaee alishazaee commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Summary by CodeRabbit

  • New Features
    • Added support for fetching active alerting rules plus revision tracking, with automatic background caching for faster rule/geometry lookups.
    • Rule creation now responds with 201 Created.
  • Bug Fixes
    • Standardized notification and rule timestamp precision (second-level) during persistence and API mapping.
    • Rule titles now default to “NO NAME” when missing or empty.
  • Tests
    • Expanded integration test coverage for rule creation/cache behavior, notifications, and log search.

@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 13abeb4e-74e9-48ca-b1b4-b3c22857f543

📥 Commits

Reviewing files that changed from the base of the PR and between 7c5c26d and 5d47e65.

📒 Files selected for processing (32)
  • .idea/gradle.xml
  • alerting-system/client/build.gradle
  • alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesCache.java
  • alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesClient.java
  • alerting-system/model/build.gradle
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/Notification.java
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/NotificationFilter.java
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/Rule.java
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/RuleCreateDto.java
  • alerting-system/rest/build.gradle
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/TargetLogKafkaConsumer.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/controller/RuleController.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/entity/LogEntity.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/entity/NotificationEntity.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/entity/RuleEntity.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/mappers/RuleMapper.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/LogControllerTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/NotificationControllerTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/RuleControllerTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/service/TargetLogBatchPersisterTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/util/CommonConfigs.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/util/CommonKafkaConfigs.java
  • alerting-system/rest/src/test/resources/application.yml
  • buildSrc/src/main/groovy/java-conventions.gradle
  • buildSrc/src/main/groovy/junit5-conventions.gradle
  • common-libs/common-exceptions/build.gradle
  • common-libs/common-exceptions/src/main/java/ir/pathlens/client/ApiCallException.java
  • common-libs/location-utils/build.gradle
  • common-libs/test-extensions/build.gradle
  • common-libs/test-extensions/src/main/java/ir/pathlens/extension/postgresql/SpringCommonPostgresConfigs.java
  • gradle/libs.versions.toml
  • settings.gradle

📝 Walkthrough

Walkthrough

This PR adds a new alerting client module and rule cache, updates alerting model and REST persistence types with createdAt handling, adjusts controller and Kafka consumer behavior, adds integration test utilities and controller tests, and refreshes build and dependency catalog configuration.

Changes

Alerting Client, Cache and REST Changes

Layer / File(s) Summary
Build scripts and dependency catalog updates
settings.gradle, .idea/gradle.xml, gradle/libs.versions.toml, alerting-system/rest/build.gradle, buildSrc/src/main/groovy/{java-conventions,junit5-conventions}.gradle, common-libs/location-utils/build.gradle, common-libs/test-extensions/build.gradle
Project/module settings and version-catalog entries are updated, and several build scripts switch to catalog aliases for shared dependencies.
Common exceptions and rules client
common-libs/common-exceptions/build.gradle, common-libs/common-exceptions/src/main/java/ir/pathlens/client/ApiCallException.java, alerting-system/client/build.gradle, alerting-system/client/src/main/java/ir/pathlens/alerting/client/{RulesClient,RulesCache}.java
A new checked exception is added, the client module is wired into the build, RulesClient calls alerting API endpoints, and RulesCache maintains a background-refreshed rules snapshot.
Alerting model record updates
alerting-system/model/build.gradle, alerting-system/model/src/main/java/ir/pathlens/alerting/model/{Notification,Rule,RuleCreateDto,NotificationFilter}.java
Notification and Rule change their time-related record components, RuleCreateDto treats null titles as empty, and NotificationFilter gains Javadoc.
REST entities and rule mapping
alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/entity/{RuleEntity,LogEntity,NotificationEntity}.java, alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/mappers/RuleMapper.java
REST entities add timestamp truncation callbacks and explicit column mappings, and RuleMapper truncates timestamps during DTO/entity conversion.
Rule controller and Kafka consumer config
alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/controller/RuleController.java, alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/TargetLogKafkaConsumer.java
Rule creation now responds with HTTP 201, and the Kafka consumer uses unordered processing with an explicit offset-reset setting.
Test configuration utilities
common-libs/test-extensions/src/main/java/ir/pathlens/extension/postgresql/SpringCommonPostgresConfigs.java, alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/util/{CommonConfigs,CommonKafkaConfigs}.java, alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/service/TargetLogBatchPersisterTest.java, alerting-system/rest/src/test/resources/application.yml
Shared PostgreSQL/Kafka test configuration is refactored into reusable helpers, the batch persister test adopts the new setup, and the target-log batch size is reduced.
Rule controller and cache integration tests
alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/RuleControllerTest.java
New integration tests create rules through REST, exercise cache refresh and identity grouping, check revision numbering, and verify activation toggling.
Log and notification controller tests
alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/{LogControllerTest,NotificationControllerTest}.java
New integration tests cover log search filters and notification retrieval, search, and seen-state updates.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant RulesCache
  participant RulesClient
  participant AlertingAPI

  RulesCache->>RulesClient: getRevisionNumber()
  RulesClient->>AlertingAPI: GET revision endpoint
  AlertingAPI-->>RulesClient: revision number
  RulesClient-->>RulesCache: revision number
  RulesCache->>RulesClient: getAllActiveRules()
  RulesClient->>AlertingAPI: GET active rules endpoint
  AlertingAPI-->>RulesClient: List<Rule>
  RulesClient-->>RulesCache: List<Rule>
  RulesCache->>RulesCache: build RulesCacheSnapshot
Loading

Possibly related PRs

  • alishazaee/PathLens#3: Shares the same alerting REST layer and closely related RuleController/entity/Kafka consumer changes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: it adds a new alerting system client module and supporting client code.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch Add-client-to-alerting-system

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (5)
buildSrc/src/main/groovy/junit5-conventions.gradle (1)

7-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Same catalog/buildSrc duplication concern as java-conventions.gradle.

JUnit Jupiter versions are hardcoded to 5.10.0 here, duplicating the junit version defined in gradle/libs.versions.toml. Currently in sync, but any future catalog bump won't automatically propagate here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@buildSrc/src/main/groovy/junit5-conventions.gradle` around lines 7 - 9, The
JUnit Jupiter dependencies in junit5-conventions.gradle are hardcoded,
duplicating the version already defined in the version catalog. Update the
dependency declarations in the junit5 conventions script to reference the
catalog-managed junit version instead of 5.10.0, matching the approach used
elsewhere such as java-conventions.gradle, so future version bumps propagate
automatically.
buildSrc/src/main/groovy/java-conventions.gradle (1)

15-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Log4j versions hardcoded despite new catalog aliases.

gradle/libs.versions.toml now defines log4j-slf4j2-impl and log4j-layout-template-json aliases with version 2.21.0, but this convention script still hardcodes the version string directly instead of referencing the catalog. Since buildSrc builds separately from the root project, this may be an unavoidable limitation (the version catalog isn't automatically accessible here), but it leaves two sources of truth for the log4j version that can silently drift. Consider wiring the catalog into buildSrc (via settings.gradle catalog inclusion for buildSrc) if not already done, or add a comment noting the manual sync requirement.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@buildSrc/src/main/groovy/java-conventions.gradle` around lines 15 - 16, The
log4j dependency declarations in java-conventions.gradle still hardcode the
version instead of using the new catalog aliases. Update the buildSrc setup to
consume the version catalog for these dependencies if possible, or add an
explicit comment near the dependency declarations noting that the log4j version
must be kept in sync manually with the catalog aliases. Use the existing
dependency entries for log4j-slf4j2-impl and log4j-layout-template-json as the
place to make this change.
common-libs/common-exceptions/src/main/java/ir/pathlens/client/ApiCallException.java (1)

6-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a cause-preserving constructor.

Only a message constructor exists. If future call sites need to wrap lower-level exceptions (e.g. IOException from network calls), a (String message, Throwable cause) constructor would preserve the original stack trace.

♻️ Optional cause-preserving constructor
     public ApiCallException(String message) {
         super(message);
     }
+
+    public ApiCallException(String message, Throwable cause) {
+        super(message, cause);
+    }
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@common-libs/common-exceptions/src/main/java/ir/pathlens/client/ApiCallException.java`
around lines 6 - 12, Add a cause-preserving constructor to ApiCallException so
it can wrap lower-level failures without losing the original stack trace. Update
the ApiCallException class to include a constructor that accepts both a message
and a Throwable cause, and delegate to the superclass the same way the existing
message-only constructor does. Keep the existing message constructor unchanged
so current call sites continue to work.
alerting-system/model/src/main/java/ir/pathlens/alerting/model/RuleCreateDto.java (1)

32-34: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Use isBlank() to also catch whitespace-only titles.

isEmpty() won't catch a title of " ", which would bypass the default-name fallback.

♻️ Proposed fix
-        if (title == null || title.isEmpty()) {
+        if (title == null || title.isBlank()) {
             title = "NO NAME";
         }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@alerting-system/model/src/main/java/ir/pathlens/alerting/model/RuleCreateDto.java`
around lines 32 - 34, The title fallback in RuleCreateDto only checks isEmpty(),
so whitespace-only titles can slip through. Update the title validation in the
RuleCreateDto constructor or setter logic to use isBlank() instead of isEmpty(),
keeping the existing default assignment to "NO NAME" when title has no visible
characters.
alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/mappers/RuleMapper.java (1)

24-24: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Truncation unit mismatch with entity lifecycle callbacks (MICROS here vs SECONDS in RuleEntity).

RuleEntity.truncateTimestampsToSeconds() (@PrePersist/@PreUpdate) truncates createdAt/updatedAt/expiresAt to ChronoUnit.SECONDS on every persist/update. Since that coarser truncation always runs afterward, the MICROS truncation done here is effectively discarded for persisted data — toDto's truncation on an already-persisted entity is a no-op, and fromDto's MICROS truncation on expiresAt gets further truncated to SECONDS right before/at insert. This inconsistency could mislead future maintainers into assuming microsecond precision survives persistence when it doesn't.

Consider aligning both truncation units (e.g., use SECONDS consistently, matching what's actually persisted), or document why MICROS is intentionally used here (e.g., for pre-persist comparison/testing purposes).

Separately, truncateToMicros has no null guard — if ever called with a null LocalDateTime (e.g., on a transient, not-yet-persisted entity) it will NPE.

♻️ Suggested consolidation
-    private static java.time.LocalDateTime truncateToMicros(java.time.LocalDateTime dateTime) {
-        return dateTime.truncatedTo(ChronoUnit.MICROS);
+    private static LocalDateTime truncateToMicros(LocalDateTime dateTime) {
+        return dateTime == null ? null : dateTime.truncatedTo(ChronoUnit.SECONDS);
     }

(also update fromDto's ChronoUnit.MICROS to ChronoUnit.SECONDS, and add the java.time.LocalDateTime import)

Also applies to: 35-46

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/mappers/RuleMapper.java`
at line 24, Align the timestamp truncation in RuleMapper with RuleEntity’s
lifecycle behavior: the MICROS truncation in fromDto/toDto is misleading because
RuleEntity.truncateTimestampsToSeconds() ultimately persists seconds precision.
Update the mapper to use ChronoUnit.SECONDS consistently for expiresAt (and any
related truncation paths), or explicitly document why microseconds are only
temporary. Also make truncateToMicros null-safe by guarding against null
LocalDateTime inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@alerting-system/client/build.gradle`:
- Around line 5-9: The `:alerting-system:client` module is leaking
`ir.pathlens.alerting.model.Rule` through `RulesClient.getAllActiveRules()`, so
`:alerting-system:model` must be exposed on the public API instead of being
internal. Update the dependency declaration in the `dependencies` block so the
`project(':alerting-system:model')` entry is an `api` dependency, keeping
`RulesClient`’s return type available to downstream consumers.

In
`@alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesCache.java`:
- Around line 56-68: The immutable snapshot contract in RulesCache is broken
because RulesCacheSnapshot still exposes mutable Set<UUID> values via
getRulesIdsByIdentity(). Update the snapshot creation in RulesCache and the
RulesCacheSnapshot accessors so the grouped rule-id sets are defensively copied
into unmodifiable sets before being stored, not just wrapped by Map.copyOf().
Keep the fix centered around the updatedRuleCache build path and the
getRulesIdsByIdentity() API so callers cannot mutate internal cache state.
- Around line 94-111: The background loop in runSyncLoop only handles
ApiCallException, so any other RuntimeException from sync() or the initial sleep
can kill the worker thread while running stays true. Update runSyncLoop to catch
unexpected runtime failures as well, log them with context, and ensure the loop
does not die silently; also review the initialDelay calculation in runSyncLoop
and the RulesCache constructor validation so invalid minInitialDelayInMillis
values cannot reach TimeUnit.MILLISECONDS.sleep.

In
`@alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesClient.java`:
- Around line 22-31: `RulesClient` is creating a JAX-RS `Client` without any
lifecycle cleanup, so the underlying resources are leaked. Update the
`RulesClient` class to implement `AutoCloseable` or `Closeable`, add a `close()`
method that delegates to `client.close()`, and make sure any callers such as
`RuleControllerTest` or other instantiations use try-with-resources or otherwise
close the instance after use.

In
`@alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/TargetLogKafkaConsumer.java`:
- Around line 39-41: The TargetLogKafkaConsumer setup leaves autoOffsetReset
implicit when config.autoOffsetReset() is null, which can default to latest and
skip backlog on first start. Update the ConsumerConfig property assignment in
TargetLogKafkaConsumer to always set an explicit default, using earliest unless
the bound config already provides a value, so the consumer behavior is clear and
consistent.

In
`@alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/RuleControllerTest.java`:
- Around line 98-114: `RulesCache` background executor threads are leaked in the
test methods that call `submitBackgroundTask()` without cleanup. Update
`testCacheContainsCreatedRules` and `testIdenticalIdentitiesInCache` to manage
the `RulesCache` lifecycle via try-with-resources (or ensure `close()` is called
in a shared teardown), using the `RulesCache` AutoCloseable contract to stop the
background task after assertions complete.

---

Nitpick comments:
In
`@alerting-system/model/src/main/java/ir/pathlens/alerting/model/RuleCreateDto.java`:
- Around line 32-34: The title fallback in RuleCreateDto only checks isEmpty(),
so whitespace-only titles can slip through. Update the title validation in the
RuleCreateDto constructor or setter logic to use isBlank() instead of isEmpty(),
keeping the existing default assignment to "NO NAME" when title has no visible
characters.

In
`@alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/mappers/RuleMapper.java`:
- Line 24: Align the timestamp truncation in RuleMapper with RuleEntity’s
lifecycle behavior: the MICROS truncation in fromDto/toDto is misleading because
RuleEntity.truncateTimestampsToSeconds() ultimately persists seconds precision.
Update the mapper to use ChronoUnit.SECONDS consistently for expiresAt (and any
related truncation paths), or explicitly document why microseconds are only
temporary. Also make truncateToMicros null-safe by guarding against null
LocalDateTime inputs.

In `@buildSrc/src/main/groovy/java-conventions.gradle`:
- Around line 15-16: The log4j dependency declarations in
java-conventions.gradle still hardcode the version instead of using the new
catalog aliases. Update the buildSrc setup to consume the version catalog for
these dependencies if possible, or add an explicit comment near the dependency
declarations noting that the log4j version must be kept in sync manually with
the catalog aliases. Use the existing dependency entries for log4j-slf4j2-impl
and log4j-layout-template-json as the place to make this change.

In `@buildSrc/src/main/groovy/junit5-conventions.gradle`:
- Around line 7-9: The JUnit Jupiter dependencies in junit5-conventions.gradle
are hardcoded, duplicating the version already defined in the version catalog.
Update the dependency declarations in the junit5 conventions script to reference
the catalog-managed junit version instead of 5.10.0, matching the approach used
elsewhere such as java-conventions.gradle, so future version bumps propagate
automatically.

In
`@common-libs/common-exceptions/src/main/java/ir/pathlens/client/ApiCallException.java`:
- Around line 6-12: Add a cause-preserving constructor to ApiCallException so it
can wrap lower-level failures without losing the original stack trace. Update
the ApiCallException class to include a constructor that accepts both a message
and a Throwable cause, and delegate to the superclass the same way the existing
message-only constructor does. Keep the existing message constructor unchanged
so current call sites continue to work.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2a15d72a-388b-4e71-93d4-0901f177dbed

📥 Commits

Reviewing files that changed from the base of the PR and between 8ac2cc2 and 7c5c26d.

📒 Files selected for processing (32)
  • .idea/gradle.xml
  • alerting-system/client/build.gradle
  • alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesCache.java
  • alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesClient.java
  • alerting-system/model/build.gradle
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/Notification.java
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/NotificationFilter.java
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/Rule.java
  • alerting-system/model/src/main/java/ir/pathlens/alerting/model/RuleCreateDto.java
  • alerting-system/rest/build.gradle
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/TargetLogKafkaConsumer.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/controller/RuleController.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/entity/LogEntity.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/entity/NotificationEntity.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/entity/RuleEntity.java
  • alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/mappers/RuleMapper.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/LogControllerTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/NotificationControllerTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/controllers/RuleControllerTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/service/TargetLogBatchPersisterTest.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/util/CommonConfigs.java
  • alerting-system/rest/src/test/java/ir/pathlens/alerting/rest/util/CommonKafkaConfigs.java
  • alerting-system/rest/src/test/resources/application.yml
  • buildSrc/src/main/groovy/java-conventions.gradle
  • buildSrc/src/main/groovy/junit5-conventions.gradle
  • common-libs/common-exceptions/build.gradle
  • common-libs/common-exceptions/src/main/java/ir/pathlens/client/ApiCallException.java
  • common-libs/location-utils/build.gradle
  • common-libs/test-extensions/build.gradle
  • common-libs/test-extensions/src/main/java/ir/pathlens/extension/postgresql/SpringCommonPostgresConfigs.java
  • gradle/libs.versions.toml
  • settings.gradle
💤 Files with no reviewable changes (1)
  • common-libs/test-extensions/build.gradle

Comment thread alerting-system/client/build.gradle
Comment on lines +94 to +111
private void runSyncLoop() {
while (running) {
try {
long initialDelay = random.nextInt(minInitialDelayInMillis, maxInitialDelayInMillis);
try {
TimeUnit.MILLISECONDS.sleep(initialDelay);
} catch (InterruptedException e) {
logger.error("Interrupted during initial delay", e);
Thread.currentThread().interrupt();

return;
}
sync();
} catch (ApiCallException ex) {
logger.error("Unable to sync rules", ex);
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Background sync loop dies silently on any non-ApiCallException failure.

runSyncLoop only catches ApiCallException. Any other RuntimeException from sync() — including IllegalArgumentException from TimeUnit.MILLISECONDS.sleep(initialDelay) if minInitialDelayInMillis is ever negative (unvalidated in the constructor, only the max > min relation is checked) — will propagate, terminate the single worker thread, yet running remains true. The cache then silently stops refreshing forever with no visible signal to callers.

🔧 Proposed fix
                 sync();
             } catch (ApiCallException ex) {
                 logger.error("Unable to sync rules", ex);
+            } catch (RuntimeException ex) {
+                logger.error("Unexpected error while syncing rules; continuing loop", ex);
             }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private void runSyncLoop() {
while (running) {
try {
long initialDelay = random.nextInt(minInitialDelayInMillis, maxInitialDelayInMillis);
try {
TimeUnit.MILLISECONDS.sleep(initialDelay);
} catch (InterruptedException e) {
logger.error("Interrupted during initial delay", e);
Thread.currentThread().interrupt();
return;
}
sync();
} catch (ApiCallException ex) {
logger.error("Unable to sync rules", ex);
}
}
}
private void runSyncLoop() {
while (running) {
try {
long initialDelay = random.nextInt(minInitialDelayInMillis, maxInitialDelayInMillis);
try {
TimeUnit.MILLISECONDS.sleep(initialDelay);
} catch (InterruptedException e) {
logger.error("Interrupted during initial delay", e);
Thread.currentThread().interrupt();
return;
}
sync();
} catch (ApiCallException ex) {
logger.error("Unable to sync rules", ex);
} catch (RuntimeException ex) {
logger.error("Unexpected error while syncing rules; continuing loop", ex);
}
}
}
🧰 Tools
🪛 PMD (7.25.0)

[Low] 108-108: InvalidLogMessageFormat (Error Prone): Too many arguments, expected 0 argument but found 1

(InvalidLogMessageFormat (Error Prone))

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesCache.java`
around lines 94 - 111, The background loop in runSyncLoop only handles
ApiCallException, so any other RuntimeException from sync() or the initial sleep
can kill the worker thread while running stays true. Update runSyncLoop to catch
unexpected runtime failures as well, log them with context, and ensure the loop
does not die silently; also review the initialDelay calculation in runSyncLoop
and the RulesCache constructor validation so invalid minInitialDelayInMillis
values cannot reach TimeUnit.MILLISECONDS.sleep.

Comment thread alerting-system/client/src/main/java/ir/pathlens/alerting/client/RulesClient.java Outdated
Comment on lines +39 to +41
if (config.autoOffsetReset() != null) {
props.put(ConsumerConfig.AUTO_OFFSET_RESET_CONFIG, config.autoOffsetReset());
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect KafkaConsumerConfig definition and its usages/defaults for autoOffsetReset
ast-grep run --pattern 'record KafkaConsumerConfig($$$) { $$$ }' --lang java
rg -nP 'autoOffsetReset' --type=java -C3
rg -nP 'auto-offset-reset|autoOffsetReset' --type=yaml -C3

Repository: alishazaee/PathLens

Length of output: 218


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the target file and nearby symbols
ast-grep outline alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/TargetLogKafkaConsumer.java --view expanded

# Find the config type and any references/defaults for autoOffsetReset across the repo
rg -n --hidden --glob '!**/target/**' --glob '!**/build/**' 'autoOffsetReset|auto-offset-reset|AUTO_OFFSET_RESET_CONFIG' .

# Locate likely config classes for Kafka settings
fd -a 'KafkaConsumerConfig.java' .
fd -a '*Kafka*Config*.java' .
fd -a '*application*.yml' .
fd -a '*application*.yaml' .

Repository: alishazaee/PathLens

Length of output: 1353


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the Kafka consumer config record and the consumer creation code
sed -n '1,120p' alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/KafkaConsumerConfig.java
printf '\n---\n'
sed -n '1,140p' alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/TargetLogKafkaConsumer.java
printf '\n---\n'

# Check runtime config files for any default auto-offset-reset values
fd -a 'application.yml' alerting-system/rest
fd -a 'application.yaml' alerting-system/rest
printf '\n---\n'
rg -n --hidden --glob '!**/target/**' --glob '!**/build/**' 'auto-offset-reset|autoOffsetReset' alerting-system/rest/src/main/resources alerting-system/rest/src/test/resources

# Check if the config object is annotated/bound in a way that makes the field required or optional
rg -n --hidden --glob '!**/target/**' --glob '!**/build/**' '`@ConfigurationProperties`|`@ConstructorBinding`|`@DefaultValue`|record KafkaConsumerConfig|class KafkaConsumerConfig' alerting-system/rest/src/main/java

Repository: alishazaee/PathLens

Length of output: 3218


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect application config binding and any main-resource defaults
sed -n '1,120p' alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/ApplicationConfig.java
printf '\n---\n'
find alerting-system/rest/src/main/resources -maxdepth 2 -type f \( -name 'application.yml' -o -name 'application.yaml' -o -name 'application.properties' \) -print -exec sh -c 'echo "---"; sed -n "1,220p" "$1"' _ {} \;

printf '\n---\n'
rg -n --hidden --glob '!**/target/**' --glob '!**/build/**' 'auto-offset-reset|autoOffsetReset|earliest|latest' alerting-system/rest/src/main/resources alerting-system/rest/src/main/java

Repository: alishazaee/PathLens

Length of output: 1198


Set an explicit autoOffsetReset default
config.autoOffsetReset() is nullable, so omitting it falls back to Kafka’s latest default and can skip existing messages on first start. If this consumer should read backlog by default, make earliest explicit here or in the bound config.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@alerting-system/rest/src/main/java/ir/pathlens/alerting/rest/configs/TargetLogKafkaConsumer.java`
around lines 39 - 41, The TargetLogKafkaConsumer setup leaves autoOffsetReset
implicit when config.autoOffsetReset() is null, which can default to latest and
skip backlog on first start. Update the ConsumerConfig property assignment in
TargetLogKafkaConsumer to always set an explicit default, using earliest unless
the bound config already provides a value, so the consumer behavior is clear and
consistent.

@alishazaee
alishazaee force-pushed the Add-client-to-alerting-system branch from 7c5c26d to 5d47e65 Compare July 5, 2026 20:39
@alishazaee
alishazaee merged commit 96ef385 into master Jul 5, 2026
1 check was pending
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.

1 participant