-
Notifications
You must be signed in to change notification settings - Fork 4
feat(metrics): report total queue sizes #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds total queue size metrics and filtered queue size metrics for ingestion steps. Introduces new DAO and mapper methods (Java and MyBatis XML) to fetch counts by feed aliases and provider lists. Updates metrics config to register five total gauges. Extends QueueSizeMetricsCollector to compute both filtered and total counts and wire new settings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Scheduler
participant Collector as QueueSizeMetricsCollector
participant FeedDao
participant KonturEventsDao
participant NormObsDao as NormalizedObservationsDao
participant DataLakeDao
participant Gauges as AtomicInteger Gauges
Scheduler->>Collector: collect()
rect rgba(200,230,255,0.3)
note over Collector: Filtered (enabled providers/feeds)
Collector->>FeedDao: getEnrichmentSkippedEventsCountForFeedsWithEnrichment()
FeedDao-->>Collector: count
Collector->>FeedDao: getNotEnrichedEventsCountForFeedsWithEnrichment()
FeedDao-->>Collector: count
Collector->>KonturEventsDao: getNotComposedEventsCountForFeeds(feedAliases)
KonturEventsDao-->>Collector: count
Collector->>NormObsDao: getNotRecombinedObservationsCountForProviders(providers)
NormObsDao-->>Collector: count
Collector->>DataLakeDao: getNotNormalizedObservationsCountForProviders(providers)
DataLakeDao-->>Collector: count
Collector->>Gauges: set filtered queueSize gauges
end
rect rgba(220,255,220,0.3)
note over Collector: Totals (all providers/feeds)
Collector->>FeedDao: getEnrichmentSkippedEventsCount()
FeedDao-->>Collector: total
Collector->>FeedDao: getNotEnrichedEventsCount()
FeedDao-->>Collector: total
Collector->>KonturEventsDao: getNotComposedEventsCount()
KonturEventsDao-->>Collector: total
Collector->>NormObsDao: getNotRecombinedObservationsCount()
NormObsDao-->>Collector: total
Collector->>DataLakeDao: getNotNormalizedObservationsCount()
DataLakeDao-->>Collector: total
Collector->>Gauges: set totalQueueSize gauges
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollector.java (2)
7-11: Add logging and imports for robustness helper.Introduce SLF4J logger and needed types for the helper above.
import io.kontur.eventapi.metrics.MetricCollector; import org.springframework.stereotype.Component; -import java.util.concurrent.atomic.AtomicInteger; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.IntSupplier;
12-19: Add a concise class-level Javadoc to explain “enabled vs total” metrics.Helps non-Java teammates (Python/PostGIS) understand intent.
/** * Collects queue-size gauges for pipeline steps. * “Enabled” metrics are filtered by configured feeds/providers; * “Total” metrics are unfiltered counts across all data. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (17)
CHANGELOG.md(1 hunks)docs/metrics.md(1 hunks)src/main/java/io/kontur/eventapi/dao/DataLakeDao.java(1 hunks)src/main/java/io/kontur/eventapi/dao/FeedDao.java(1 hunks)src/main/java/io/kontur/eventapi/dao/KonturEventsDao.java(1 hunks)src/main/java/io/kontur/eventapi/dao/NormalizedObservationsDao.java(1 hunks)src/main/java/io/kontur/eventapi/dao/mapper/DataLakeMapper.java(1 hunks)src/main/java/io/kontur/eventapi/dao/mapper/FeedMapper.java(1 hunks)src/main/java/io/kontur/eventapi/dao/mapper/KonturEventsMapper.java(1 hunks)src/main/java/io/kontur/eventapi/dao/mapper/NormalizedObservationsMapper.java(1 hunks)src/main/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollector.java(1 hunks)src/main/java/io/kontur/eventapi/metrics/config/MetricsConfig.java(3 hunks)src/main/resources/db/mappers/DataLakeMapper.xml(1 hunks)src/main/resources/db/mappers/FeedMapper.xml(1 hunks)src/main/resources/db/mappers/KonturEventsMapper.xml(1 hunks)src/main/resources/db/mappers/NormalizedObservationsMapper.xml(1 hunks)src/test/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollectorTest.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
**/*.java: Write enough comments in Java so Python/PostGIS-proficient engineers can understand the code
Handle exceptions properly; do not just ignore them
Files:
src/main/java/io/kontur/eventapi/dao/KonturEventsDao.javasrc/main/java/io/kontur/eventapi/dao/mapper/FeedMapper.javasrc/main/java/io/kontur/eventapi/dao/FeedDao.javasrc/main/java/io/kontur/eventapi/dao/NormalizedObservationsDao.javasrc/main/java/io/kontur/eventapi/dao/mapper/KonturEventsMapper.javasrc/main/java/io/kontur/eventapi/dao/mapper/DataLakeMapper.javasrc/main/java/io/kontur/eventapi/dao/mapper/NormalizedObservationsMapper.javasrc/main/java/io/kontur/eventapi/dao/DataLakeDao.javasrc/test/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollectorTest.javasrc/main/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollector.javasrc/main/java/io/kontur/eventapi/metrics/config/MetricsConfig.java
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep docs/ folder’s general project documentation up to date
Files:
docs/metrics.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
**/*.md: Start sentences on new lines in docs to improve git diffs
Prefer storing notes and documentation as Markdown files
Files:
docs/metrics.mdCHANGELOG.md
docs/**
📄 CodeRabbit inference engine (AGENTS.md)
docs/**: Ensure docs/ reflects reality; fix everything outdated in docs/
Consult docs/ to understand the project
Check data schema as described in docs/
Files:
docs/metrics.md
🪛 LanguageTool
docs/metrics.md
[grammar] ~1-~1: Use correct spacing
Context: # Metrics Queue size metrics expose current queue ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~4-~4: Use correct spacing
Context: ...e sizes that include disabled providers. - enrichment.queueSize and enrichment.totalQueueSize - `enri...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~6-~6: Use correct spacing
Context: ...roviders. - enrichment.queueSize and enrichment.totalQueueSize - enrichmentSkipped.queueSize and enrichmentSkipped.totalQueueSize ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~7-~7: Use correct spacing
Context: ...ze-enrichmentSkipped.queueSizeandenrichmentSkipped.totalQueueSize-feedComposition.queueSizeandfeedComposition.totalQueueSize` - ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~8-~8: Use correct spacing
Context: ...Size-feedComposition.queueSizeandfeedComposition.totalQueueSize-eventCombination.queueSizeandeventCombination.totalQueueSize` -...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~9-~9: Avoid extra words
Context: ...ize-eventCombination.queueSizeandeventCombination.totalQueueSize-normalization.queueSizeandnormalization.totalQueueSize`
(QB_NEW_EN_OTHER_ERROR_IDS_12)
CHANGELOG.md
[grammar] ~7-~7: Use correct spacing
Context: ...t total queue sizes for ingestion steps. #### Changed - Implement flood-specific sever...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (5)
src/main/java/io/kontur/eventapi/dao/mapper/KonturEventsMapper.java (1)
30-31: Confirm XML mapping uses<foreach>foraliasesIN clause
DAO correctly returns 0 whenaliasesis null or empty; ensure the<select id="getNotComposedEventsCountForFeeds">in KonturEventsMapper.xml binds thealiaseslist via<foreach>to form a properIN (…)clause and avoid SQL syntax errors.src/main/java/io/kontur/eventapi/dao/mapper/NormalizedObservationsMapper.java (1)
64-65: Empty-provider handling and XML foreach binding verified DAO method returns 0 for null or emptyproviders, and the mapper XML correctly uses<foreach collection="providers">for theINclause.docs/metrics.md (1)
1-10: LGTM — concise and matches implementation.
Names and semantics align with MetricsConfig; sentences on separate lines follow docs guidelines.src/main/java/io/kontur/eventapi/dao/mapper/DataLakeMapper.java (1)
51-51: LGTM; param name matches XML collection
Interface method and @param("providers") align with the XML’s collection="providers".src/main/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollector.java (1)
13-13: Component-scoped collector is fine.Constructor injection for gauges + DAOs keeps wiring explicit. With the robustness and guards above, this looks good.
| - Report total queue sizes for ingestion steps. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Add pointer to the new metrics doc for discoverability.
Link the bullet to docs/metrics.md so operators know where to find names and semantics.
Apply:
-- Report total queue sizes for ingestion steps.
+- Report total queue sizes for ingestion steps (see docs/metrics.md).📝 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.
| - Report total queue sizes for ingestion steps. | |
| - Report total queue sizes for ingestion steps (see docs/metrics.md). |
🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: Use correct spacing
Context: ...t total queue sizes for ingestion steps. #### Changed - Implement flood-specific sever...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In CHANGELOG.md around lines 7 to 8, the bullet "Report total queue sizes for
ingestion steps." needs a link to the new metrics doc for discoverability;
update the line to make the bullet text a Markdown link pointing to
docs/metrics.md (e.g., replace the plain text with a linked version) so
operators can click through to the metrics documentation and find metric names
and semantics.
| public Integer getNotNormalizedObservationsCountForProviders(List<String> providers) { | ||
| if (providers == null || providers.isEmpty()) { | ||
| return 0; | ||
| } | ||
| return mapper.getNotNormalizedObservationsCountForProviders(providers); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Add Javadoc clarifying 0-on-empty semantics
Behavior matches the XML contract; document it to prevent accidental misuse.
+ /**
+ * Count not-normalized observations limited to provided providers.
+ * Returns 0 and skips DB access when providers is null or empty.
+ */
public Integer getNotNormalizedObservationsCountForProviders(List<String> providers) {
if (providers == null || providers.isEmpty()) {
return 0;
}
return mapper.getNotNormalizedObservationsCountForProviders(providers);
}🏁 Script executed:
#!/bin/bash
# Ensure interface + XML mapping exist and align with @Param("providers")
rg -nC2 '\bgetNotNormalizedObservationsCountForProviders\b' src/main/java src/main/resourcesLength of output: 2704
Add Javadoc clarifying null/empty‐providers returns 0
Clarify that when providers is null or empty, the method returns 0 and skips the DB call.
+ /**
+ * Count not-normalized observations for the given providers.
+ * Returns 0 and skips DB access if `providers` is null or empty.
+ */
public Integer getNotNormalizedObservationsCountForProviders(List<String> providers) {
if (providers == null || providers.isEmpty()) {
return 0;
}
return mapper.getNotNormalizedObservationsCountForProviders(providers);
}📝 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.
| public Integer getNotNormalizedObservationsCountForProviders(List<String> providers) { | |
| if (providers == null || providers.isEmpty()) { | |
| return 0; | |
| } | |
| return mapper.getNotNormalizedObservationsCountForProviders(providers); | |
| } | |
| /** | |
| * Count not-normalized observations for the given providers. | |
| * Returns 0 and skips DB access if `providers` is null or empty. | |
| */ | |
| public Integer getNotNormalizedObservationsCountForProviders(List<String> providers) { | |
| if (providers == null || providers.isEmpty()) { | |
| return 0; | |
| } | |
| return mapper.getNotNormalizedObservationsCountForProviders(providers); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/io/kontur/eventapi/dao/DataLakeDao.java around lines 90 to 95,
add a Javadoc block above
getNotNormalizedObservationsCountForProviders(List<String> providers) that
explicitly documents that when providers is null or empty the method returns 0
and the DB/mapper call is skipped; include @param providers description and
@return Integer explanation (zero when null/empty, otherwise delegated to
mapper) so callers understand the short-circuit behavior.
| public Integer getNotEnrichedEventsCountForFeedsWithEnrichment() { | ||
| return mapper.getNotEnrichedEventsCountForFeedsWithEnrichment(); | ||
| } | ||
|
|
||
| public Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment() { | ||
| return mapper.getEnrichmentSkippedEventsCountForFeedsWithEnrichment(); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Null-safe counts and minimal docs for metrics consumers
Default to 0 defensively and document semantics for non-Java colleagues.
Apply:
- public Integer getNotEnrichedEventsCountForFeedsWithEnrichment() {
- return mapper.getNotEnrichedEventsCountForFeedsWithEnrichment();
- }
+ /**
+ * Count of not-enriched events limited to feeds with configured enrichment.
+ * Never returns null.
+ */
+ public Integer getNotEnrichedEventsCountForFeedsWithEnrichment() {
+ return java.util.Optional.ofNullable(
+ mapper.getNotEnrichedEventsCountForFeedsWithEnrichment()
+ ).orElse(0);
+ }
- public Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment() {
- return mapper.getEnrichmentSkippedEventsCountForFeedsWithEnrichment();
- }
+ /**
+ * Count of enrichment-skipped events limited to feeds with configured enrichment.
+ * Never returns null.
+ */
+ public Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment() {
+ return java.util.Optional.ofNullable(
+ mapper.getEnrichmentSkippedEventsCountForFeedsWithEnrichment()
+ ).orElse(0);
+ }📝 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.
| public Integer getNotEnrichedEventsCountForFeedsWithEnrichment() { | |
| return mapper.getNotEnrichedEventsCountForFeedsWithEnrichment(); | |
| } | |
| public Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment() { | |
| return mapper.getEnrichmentSkippedEventsCountForFeedsWithEnrichment(); | |
| } | |
| /** | |
| * Count of not-enriched events limited to feeds with configured enrichment. | |
| * Never returns null. | |
| */ | |
| public Integer getNotEnrichedEventsCountForFeedsWithEnrichment() { | |
| return java.util.Optional.ofNullable( | |
| mapper.getNotEnrichedEventsCountForFeedsWithEnrichment() | |
| ).orElse(0); | |
| } | |
| /** | |
| * Count of enrichment-skipped events limited to feeds with configured enrichment. | |
| * Never returns null. | |
| */ | |
| public Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment() { | |
| return java.util.Optional.ofNullable( | |
| mapper.getEnrichmentSkippedEventsCountForFeedsWithEnrichment() | |
| ).orElse(0); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/io/kontur/eventapi/dao/FeedDao.java around lines 90 to 97, the
two methods return Integer directly from the mapper which may be null; change
them to return 0 when mapper result is null and add minimal Javadoc above each
method describing that the returned count is non-null, defaults to 0 when no
matching rows or mapper returns null, and is intended for metrics consumers.
Implement the null-safe return by checking the mapper result and returning
Integer.valueOf(0) on null, and include concise Javadoc lines about semantics
for non-Java consumers.
| public Integer getNotComposedEventsCountForFeeds(List<String> aliases) { | ||
| if (aliases == null || aliases.isEmpty()) { | ||
| return 0; | ||
| } | ||
| return mapper.getNotComposedEventsCountForFeeds(aliases); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Document null/empty behavior to avoid misuse
Returning 0 for null/empty aliases is fine and consistent with other DAOs. Add a brief Javadoc to make this explicit for metric readers and future callers.
Apply:
+ /**
+ * Count not-composed events limited to the given feed aliases.
+ * Returns 0 and skips DB access when aliases is null or empty.
+ */
public Integer getNotComposedEventsCountForFeeds(List<String> aliases) {
if (aliases == null || aliases.isEmpty()) {
return 0;
}
return mapper.getNotComposedEventsCountForFeeds(aliases);
}🏁 Script executed:
#!/bin/bash
# Verify mapper interface and XML mapping exist and match the DAO call
rg -nC2 '\bgetNotComposedEventsCountForFeeds\b' src/main/java src/main/resourcesLength of output: 2831
Document null/empty behavior in Javadoc
+ /**
+ * Count not-composed events limited to the given feed aliases.
+ * Returns 0 and skips DB access when aliases is null or empty.
+ */
public Integer getNotComposedEventsCountForFeeds(List<String> aliases) {
if (aliases == null || aliases.isEmpty()) {
return 0;
}
return mapper.getNotComposedEventsCountForFeeds(aliases);
}📝 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.
| public Integer getNotComposedEventsCountForFeeds(List<String> aliases) { | |
| if (aliases == null || aliases.isEmpty()) { | |
| return 0; | |
| } | |
| return mapper.getNotComposedEventsCountForFeeds(aliases); | |
| } | |
| /** | |
| * Count not-composed events limited to the given feed aliases. | |
| * Returns 0 and skips DB access when aliases is null or empty. | |
| */ | |
| public Integer getNotComposedEventsCountForFeeds(List<String> aliases) { | |
| if (aliases == null || aliases.isEmpty()) { | |
| return 0; | |
| } | |
| return mapper.getNotComposedEventsCountForFeeds(aliases); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/io/kontur/eventapi/dao/KonturEventsDao.java around lines 62 to
67, the method getNotComposedEventsCountForFeeds lacks Javadoc documenting how
null or empty aliases are handled; add/update the method Javadoc to state that
if the aliases parameter is null or empty the method returns 0, otherwise it
delegates to mapper.getNotComposedEventsCountForFeeds(aliases) and returns that
Integer, and describe the meaning of the return value and any potential
nullability of the returned Integer.
| Integer getNotEnrichedEventsCountForFeedsWithEnrichment(); | ||
|
|
||
| Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment(); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Add brief Javadoc for new metrics-facing methods
Clarify that these counts include only feeds with non-empty enrichment config (feeds.enrichment array). This helps consumers distinguish from total counts.
Apply:
+ /**
+ * Count of not-enriched feed_data rows limited to feeds that have enrichment configured (feeds.enrichment not empty).
+ */
Integer getNotEnrichedEventsCountForFeedsWithEnrichment();
+ /**
+ * Count of enrichment-skipped feed_data rows limited to feeds that have enrichment configured (feeds.enrichment not empty).
+ */
Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment();📝 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.
| Integer getNotEnrichedEventsCountForFeedsWithEnrichment(); | |
| Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment(); | |
| /** | |
| * Count of not-enriched feed_data rows limited to feeds that have enrichment configured (feeds.enrichment not empty). | |
| */ | |
| Integer getNotEnrichedEventsCountForFeedsWithEnrichment(); | |
| /** | |
| * Count of enrichment-skipped feed_data rows limited to feeds that have enrichment configured (feeds.enrichment not empty). | |
| */ | |
| Integer getEnrichmentSkippedEventsCountForFeedsWithEnrichment(); |
🤖 Prompt for AI Agents
In src/main/java/io/kontur/eventapi/dao/mapper/FeedMapper.java around lines 76
to 79, add brief Javadoc comments above the two methods to state that the
returned counts only include feeds that have a non-empty enrichment
configuration (i.e., feeds.enrichment array is non-empty). For each method, add
a one- or two-sentence javadoc describing the metric and explicitly note it is
limited to feeds with non-empty enrichment config so callers can distinguish
these counts from total feed event counts.
| <select id="getEnrichmentSkippedEventsCountForFeedsWithEnrichment" resultType="java.lang.Integer"> | ||
| select count(*) from feed_data fd | ||
| join feeds f on f.feed_id = fd.feed_id | ||
| where fd.enrichment_skipped | ||
| and array_length(f.enrichment, 1) > 0; | ||
| </select> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Mirror NULL-safe enrichment check for skipped counts
Keep criteria consistent with the not-enriched counterpart.
Apply:
- where fd.enrichment_skipped
- and array_length(f.enrichment, 1) > 0;
+ where fd.enrichment_skipped
+ and coalesce(array_length(f.enrichment, 1), 0) > 0;📝 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.
| <select id="getEnrichmentSkippedEventsCountForFeedsWithEnrichment" resultType="java.lang.Integer"> | |
| select count(*) from feed_data fd | |
| join feeds f on f.feed_id = fd.feed_id | |
| where fd.enrichment_skipped | |
| and array_length(f.enrichment, 1) > 0; | |
| </select> | |
| <select id="getEnrichmentSkippedEventsCountForFeedsWithEnrichment" resultType="java.lang.Integer"> | |
| select count(*) from feed_data fd | |
| join feeds f on f.feed_id = fd.feed_id | |
| where fd.enrichment_skipped | |
| and coalesce(array_length(f.enrichment, 1), 0) > 0; | |
| </select> |
🤖 Prompt for AI Agents
In src/main/resources/db/mappers/FeedMapper.xml around lines 111 to 117, the
WHERE clause uses array_length(f.enrichment, 1) > 0 but does not mirror the
NULL-safe enrichment check used in the not-enriched query; update the condition
to the same NULL-safe form (e.g., use array_length(coalesce(f.enrichment, '{}'),
1) > 0 or the exact NULL-safe expression from the not-enriched counterpart) so
the skipped-count query and the not-enriched query use identical NULL-safe
enrichment checks.
| <select id="getNotComposedEventsCountForFeeds" resultType="java.lang.Integer"> | ||
| select count(*) from feed_event_status fes | ||
| join feeds f on f.feed_id = fes.feed_id | ||
| where not actual | ||
| and f.alias in | ||
| <foreach item="alias" collection="aliases" open="(" separator="," close=")"> | ||
| #{alias} | ||
| </foreach> | ||
| </select> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard IN-clause for empty alias lists and qualify column to avoid ambiguity
Passing null/empty aliases will produce invalid SQL f.alias in (). Also, qualify actual with table alias.
Apply:
- <select id="getNotComposedEventsCountForFeeds" resultType="java.lang.Integer">
- select count(*) from feed_event_status fes
- join feeds f on f.feed_id = fes.feed_id
- where not actual
- and f.alias in
- <foreach item="alias" collection="aliases" open="(" separator="," close=")">
- #{alias}
- </foreach>
- </select>
+ <select id="getNotComposedEventsCountForFeeds" resultType="java.lang.Integer">
+ select count(*) from feed_event_status fes
+ join feeds f on f.feed_id = fes.feed_id
+ where not fes.actual
+ <choose>
+ <when test="aliases != null and aliases.size() > 0">
+ and f.alias in
+ <foreach item="alias" collection="aliases" open="(" separator="," close=")">
+ #{alias}
+ </foreach>
+ </when>
+ <otherwise>
+ and 1 = 2
+ </otherwise>
+ </choose>
+ </select>🤖 Prompt for AI Agents
In src/main/resources/db/mappers/KonturEventsMapper.xml around lines 74-82, the
query builds an empty IN() when aliases is null/empty and leaves the column
actual unqualified; wrap the IN clause in a MyBatis conditional (e.g. <if
test="aliases != null and aliases.size() > 0"> ... foreach ... </if>) and add
a branch for the empty case that makes the predicate always false (e.g. AND 1=0)
so SQL remains valid, and fully qualify the actual column as fes.actual to avoid
ambiguity.
| <select id="getNotRecombinedObservationsCountForProviders" resultType="java.lang.Integer"> | ||
| select count(*) from normalized_observations | ||
| where not recombined | ||
| and provider in | ||
| <foreach item="provider" collection="providers" open="(" separator="," close=")"> | ||
| #{provider} | ||
| </foreach> | ||
| </select> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent invalid SQL on empty provider lists
provider in () is invalid; return 0 when providers is null/empty.
Apply:
- <select id="getNotRecombinedObservationsCountForProviders" resultType="java.lang.Integer">
- select count(*) from normalized_observations
- where not recombined
- and provider in
- <foreach item="provider" collection="providers" open="(" separator="," close=")">
- #{provider}
- </foreach>
- </select>
+ <select id="getNotRecombinedObservationsCountForProviders" resultType="java.lang.Integer">
+ select count(*) from normalized_observations
+ where not recombined
+ <choose>
+ <when test="providers != null and providers.size() > 0">
+ and provider in
+ <foreach item="provider" collection="providers" open="(" separator="," close=")">
+ #{provider}
+ </foreach>
+ </when>
+ <otherwise>
+ and 1 = 2
+ </otherwise>
+ </choose>
+ </select>📝 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.
| <select id="getNotRecombinedObservationsCountForProviders" resultType="java.lang.Integer"> | |
| select count(*) from normalized_observations | |
| where not recombined | |
| and provider in | |
| <foreach item="provider" collection="providers" open="(" separator="," close=")"> | |
| #{provider} | |
| </foreach> | |
| </select> | |
| <select id="getNotRecombinedObservationsCountForProviders" resultType="java.lang.Integer"> | |
| select count(*) from normalized_observations | |
| where not recombined | |
| <choose> | |
| <when test="providers != null and providers.size() > 0"> | |
| and provider in | |
| <foreach item="provider" collection="providers" open="(" separator="," close=")"> | |
| #{provider} | |
| </foreach> | |
| </when> | |
| <otherwise> | |
| and 1 = 2 | |
| </otherwise> | |
| </choose> | |
| </select> |
🤖 Prompt for AI Agents
In src/main/resources/db/mappers/NormalizedObservationsMapper.xml around lines
114 to 121, the SQL can produce an invalid "provider in ()" when the providers
collection is null or empty; change the mapper to branch: if providers is
non-null and size > 0 include the current select with the foreach, otherwise
return 0 (e.g. use a MyBatis <choose>/<when> test checking providers != null and
providers.size > 0 and an <otherwise> that selects 0) so the query never emits
an empty IN clause.
| when(feedDao.getEnrichmentSkippedEventsCountForFeedsWithEnrichment()).thenReturn(1); | ||
| when(feedDao.getNotEnrichedEventsCountForFeedsWithEnrichment()).thenReturn(2); | ||
| when(konturEventsDao.getNotComposedEventsCountForFeeds(anyList())).thenReturn(3); | ||
| when(normalizedObservationsDao.getNotRecombinedObservationsCountForProviders(anyList())).thenReturn(4); | ||
| when(dataLakeDao.getNotNormalizedObservationsCountForProviders(anyList())).thenReturn(5); | ||
|
|
||
| when(feedDao.getEnrichmentSkippedEventsCount()).thenReturn(6); | ||
| when(feedDao.getNotEnrichedEventsCount()).thenReturn(7); | ||
| when(konturEventsDao.getNotComposedEventsCount()).thenReturn(8); | ||
| when(normalizedObservationsDao.getNotRecombinedObservationsCount()).thenReturn(9); | ||
| when(dataLakeDao.getNotNormalizedObservationsCount()).thenReturn(10); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Verify DAO interactions to catch wiring regressions.
Also assert that the expected lists are passed to DAO methods.
@@
collector.collect();
+ // Verify enabled (filtered) calls use configured lists
+ org.mockito.Mockito.verify(feedDao).getEnrichmentSkippedEventsCountForFeedsWithEnrichment();
+ org.mockito.Mockito.verify(feedDao).getNotEnrichedEventsCountForFeedsWithEnrichment();
+ org.mockito.Mockito.verify(konturEventsDao)
+ .getNotComposedEventsCountForFeeds(org.mockito.ArgumentMatchers.eq(java.util.Arrays.asList("feed1")));
+ org.mockito.Mockito.verify(normalizedObservationsDao)
+ .getNotRecombinedObservationsCountForProviders(org.mockito.ArgumentMatchers.eq(java.util.Arrays.asList("provider1")));
+ org.mockito.Mockito.verify(dataLakeDao)
+ .getNotNormalizedObservationsCountForProviders(org.mockito.ArgumentMatchers.eq(java.util.Arrays.asList("provider2")));
+
+ // Verify totals
+ org.mockito.Mockito.verify(feedDao).getEnrichmentSkippedEventsCount();
+ org.mockito.Mockito.verify(feedDao).getNotEnrichedEventsCount();
+ org.mockito.Mockito.verify(konturEventsDao).getNotComposedEventsCount();
+ org.mockito.Mockito.verify(normalizedObservationsDao).getNotRecombinedObservationsCount();
+ org.mockito.Mockito.verify(dataLakeDao).getNotNormalizedObservationsCount();Also applies to: 56-69
🤖 Prompt for AI Agents
In
src/test/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollectorTest.java
around lines 44-55 (and similarly apply to 56-69), the test stubs DAO returns
but does not verify that the collector calls the correct DAO methods or that the
expected lists are passed; add Mockito.verify calls for each DAO method used by
the collector, and for the overloads that accept lists assert the exact list
argument (use ArgumentCaptor or eq(expectedList) instead of anyList()) to catch
wiring regressions and ensure the collector is passing the expected
feed/provider lists.
| collector.collect(); | ||
|
|
||
| assertEquals(1, enrichmentSkipped.get(), "Enrichment skipped enabled queue size mismatch"); | ||
| assertEquals(2, enrichment.get(), "Enrichment enabled queue size mismatch"); | ||
| assertEquals(3, feedComposition.get(), "Feed composition enabled queue size mismatch"); | ||
| assertEquals(4, eventCombination.get(), "Event combination enabled queue size mismatch"); | ||
| assertEquals(5, normalization.get(), "Normalization enabled queue size mismatch"); | ||
|
|
||
| assertEquals(6, enrichmentSkippedTotal.get(), "Enrichment skipped total queue size mismatch"); | ||
| assertEquals(7, enrichmentTotal.get(), "Enrichment total queue size mismatch"); | ||
| assertEquals(8, feedCompositionTotal.get(), "Feed composition total queue size mismatch"); | ||
| assertEquals(9, eventCombinationTotal.get(), "Event combination total queue size mismatch"); | ||
| assertEquals(10, normalizationTotal.get(), "Normalization total queue size mismatch"); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Group assertions with assertAll for clearer failures.
All 10 assertions should be grouped so one failure doesn’t short-circuit the rest.
@@
- assertEquals(1, enrichmentSkipped.get(), "Enrichment skipped enabled queue size mismatch");
- assertEquals(2, enrichment.get(), "Enrichment enabled queue size mismatch");
- assertEquals(3, feedComposition.get(), "Feed composition enabled queue size mismatch");
- assertEquals(4, eventCombination.get(), "Event combination enabled queue size mismatch");
- assertEquals(5, normalization.get(), "Normalization enabled queue size mismatch");
-
- assertEquals(6, enrichmentSkippedTotal.get(), "Enrichment skipped total queue size mismatch");
- assertEquals(7, enrichmentTotal.get(), "Enrichment total queue size mismatch");
- assertEquals(8, feedCompositionTotal.get(), "Feed composition total queue size mismatch");
- assertEquals(9, eventCombinationTotal.get(), "Event combination total queue size mismatch");
- assertEquals(10, normalizationTotal.get(), "Normalization total queue size mismatch");
+ org.junit.jupiter.api.Assertions.assertAll("enabled & total queue sizes",
+ () -> assertEquals(1, enrichmentSkipped.get(), "Enrichment skipped enabled queue size mismatch"),
+ () -> assertEquals(2, enrichment.get(), "Enrichment enabled queue size mismatch"),
+ () -> assertEquals(3, feedComposition.get(), "Feed composition enabled queue size mismatch"),
+ () -> assertEquals(4, eventCombination.get(), "Event combination enabled queue size mismatch"),
+ () -> assertEquals(5, normalization.get(), "Normalization enabled queue size mismatch"),
+ () -> assertEquals(6, enrichmentSkippedTotal.get(), "Enrichment skipped total queue size mismatch"),
+ () -> assertEquals(7, enrichmentTotal.get(), "Enrichment total queue size mismatch"),
+ () -> assertEquals(8, feedCompositionTotal.get(), "Feed composition total queue size mismatch"),
+ () -> assertEquals(9, eventCombinationTotal.get(), "Event combination total queue size mismatch"),
+ () -> assertEquals(10, normalizationTotal.get(), "Normalization total queue size mismatch")
+ );📝 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.
| collector.collect(); | |
| assertEquals(1, enrichmentSkipped.get(), "Enrichment skipped enabled queue size mismatch"); | |
| assertEquals(2, enrichment.get(), "Enrichment enabled queue size mismatch"); | |
| assertEquals(3, feedComposition.get(), "Feed composition enabled queue size mismatch"); | |
| assertEquals(4, eventCombination.get(), "Event combination enabled queue size mismatch"); | |
| assertEquals(5, normalization.get(), "Normalization enabled queue size mismatch"); | |
| assertEquals(6, enrichmentSkippedTotal.get(), "Enrichment skipped total queue size mismatch"); | |
| assertEquals(7, enrichmentTotal.get(), "Enrichment total queue size mismatch"); | |
| assertEquals(8, feedCompositionTotal.get(), "Feed composition total queue size mismatch"); | |
| assertEquals(9, eventCombinationTotal.get(), "Event combination total queue size mismatch"); | |
| assertEquals(10, normalizationTotal.get(), "Normalization total queue size mismatch"); | |
| } | |
| collector.collect(); | |
| org.junit.jupiter.api.Assertions.assertAll("enabled & total queue sizes", | |
| () -> assertEquals(1, enrichmentSkipped.get(), "Enrichment skipped enabled queue size mismatch"), | |
| () -> assertEquals(2, enrichment.get(), "Enrichment enabled queue size mismatch"), | |
| () -> assertEquals(3, feedComposition.get(), "Feed composition enabled queue size mismatch"), | |
| () -> assertEquals(4, eventCombination.get(), "Event combination enabled queue size mismatch"), | |
| () -> assertEquals(5, normalization.get(), "Normalization enabled queue size mismatch"), | |
| () -> assertEquals(6, enrichmentSkippedTotal.get(), "Enrichment skipped total queue size mismatch"), | |
| () -> assertEquals(7, enrichmentTotal.get(), "Enrichment total queue size mismatch"), | |
| () -> assertEquals(8, feedCompositionTotal.get(), "Feed composition total queue size mismatch"), | |
| () -> assertEquals(9, eventCombinationTotal.get(), "Event combination total queue size mismatch"), | |
| () -> assertEquals(10, normalizationTotal.get(), "Normalization total queue size mismatch") | |
| ); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/io/kontur/eventapi/metrics/collector/QueueSizeMetricsCollectorTest.java
around lines 56 to 69, the ten individual assertEquals calls should be grouped
with a single assertAll so one failure doesn’t short-circuit the rest; replace
the consecutive assertions with a single assertAll(...) that contains each
assertion as a lambda (preserving the expected value and message), and ensure
the test imports/uses org.junit.jupiter.api.Assertions.assertAll (or add a
static import) so all assertions run and report collectively.
Summary
MetricsConfigTesting
make precommithttps://chatgpt.com/codex/tasks/task_e_68b43fbfe4c08324884c75e0284933fb
Summary by CodeRabbit