Repository navigation
Conversation
taskr run 0147. Per-service and per-day step keys are now unique in the cost-analysis and web-analytics workflows, a new `WorkflowStatsBatchAggregationTest` passes, and the planned spotless check passes.
Code Review ✅ Approved🟡 Medium risk · Qualifying repeated workflow steps by service or day changes persisted AppRunRecord aggregation; non-unique qualifiers could leave execution totals undercounted. Fixes the Data Insights run-record undercount in CostAnalysisWorkflow and WebAnalyticsWorkflow by suffixing constant step names with the per-service or per-day source name, so per-iteration stats are no longer overwritten. No issues found. VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledFor more verification details, comment OptionsDisplay: compact → Showing compact verification results. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
This branch has not been deployed
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.
Summary
CostAnalysisWorkflow and WebAnalyticsWorkflow recreated processor, sink, and aggregator objects with constant step names on every per-service or per-day iteration, so the plain put in WorkflowStats.updateWorkflowStepStats overwrote each previous iteration and the persisted AppRunRecord stats reflected only the last service or day. Following the approach recommended in issue #7403, both workflows now format each constant-named step as a base name qualified with the per-service or per-day source name, producing unique step keys per iteration. WorkflowStats is intentionally unchanged because its replace semantics are relied on by DataAssetsWorkflow's cumulative per-entity-type and
[Enricher]-prefixed step keys, and DataInsightsApp is intentionally unchanged because updateStats already computes job-level totals as a sum over every entity-stats entry, which now aggregates all services and all days. Reviewers should look first at the step-name changes in CostAnalysisWorkflow.java and WebAnalyticsWorkflow.java; the new WorkflowStatsBatchAggregationTest covers per-step key uniqueness and job-level aggregation.Implementer's note: Per-service and per-day step keys are now unique in the cost-analysis and web-analytics workflows, a new
WorkflowStatsBatchAggregationTestpasses, and the planned spotless check passes.Plan
OpenMetadatasubmodule must be checked out in the implementation worktree(
git submodule update --init --remote OpenMetadata). All edits go in the submodule working tree, perthe task card (issue UI : Explore page data loading issue #7403 points at these submodule paths; the plan worktree's submodule is empty).
suffix every constant-named step:
sourcewhose name (source.getName(), e.g.[CostAnalysisWorkflow] <FQN>) isunique per service; sources already use it as their step key (CostAnalysisWorkflow.java:210) — leave
that unchanged.
processRawCostAnalysisReportData(List, Map)andprocessAggregatedCostAnalysisReportData(Map, Map)an extra
String stepKeyQualifierparameter; passsource.getName()from the loop (call sites atCostAnalysisWorkflow.java:222 and :231).
String.format("%s (%s)", baseName, stepKeyQualifier)— apply to: raw-data processor (:242-246), raw-data sink (:266-270), aggregator (:292-293),
aggregated-data processor (:315-319), aggregated-data sink (:339-343). Only the names change; stats
collection and
updateWorkflowStats(put + recompute totals, :390-403) stay as-is.processEntityViewData(Map, Map)andprocessUserActivityData(Map, Map, Map)an extraString stepKeyQualifierparameter; passsource.getName()(already unique per day — it embeds thedate, PaginatedWebAnalyticEventDataSource.java:53-56) from the loop (:168, :177).
(:277-278), user-activity processor (:293-297), user-activity sink (:318-322).
updateWorkflowStats(:357-370) unchanged.org.openmetadata.service.apps.bundles.insights.workflows. Drives the real workflow recording path soit fails (200 / 70) before the fix and passes (300 / 120) after:
new CostAnalysisWorkflow(new CostAnalysisConfig().withEnabled(true), timestamp, 100, Optional.empty())(constructor is DB-free). Invoke the private
processRawCostAnalysisReportDatareflectively twicewith qualifiers
svc-a/svc-band inputs of 100 / 200 raw report data rows,contextDatacarryingTimestampUtils.TIMESTAMP_KEY.CreateReportDataProcessor.processis pure;ReportDataSink.writeneedsEntity.getEntityTimeSeriesRepository(...)— follow the established pattern inPaginatedWebAnalyticEventDataSourceReadFailureTest.java:57:
mockStatic(Entity.class, CALLS_REAL_METHODS)returningMockito.mock(ReportDataRepository.class)(its
createNewRecordneeds no stubbing).Verification
Review (glm): approve
Files
Closes #7403
Opened by taskr run 0147.