From 52a7f99145b6cf2a3613314304c25cd6c143fb0c Mon Sep 17 00:00:00 2001 From: Mayur Singal <39544459+ulixius9@users.noreply.github.com> Date: Sat, 10 Aug 2024 14:20:14 +0530 Subject: [PATCH] MINOR: Data insight feedbacks (#17337) * MINOR: DI add no tier in system chart * checkstyle * minor ui fixes * tier filter fix * ignore tags and glossaries from tier table * enhance advance search provider * Ignore data products as well * Empty Filter Condition & Lowecase Owner Displayname * opensearch * sort graph data to confirm order of graph * address comments * handle domain in advanced filter --------- Co-authored-by: Chira Madlani Co-authored-by: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> --- .../DataInsightsEntityEnricherProcessor.java | 12 +- .../migration/utils/v150/MigrationUtil.java | 7 +- ...SearchDynamicChartAggregatorInterface.java | 4 +- .../ElasticSearchSummaryCardAggregator.java | 2 +- ...SearchDynamicChartAggregatorInterface.java | 4 +- .../OpenSearchSummaryCardAggregator.java | 2 +- .../elasticsearch/indexMappingsTemplate.json | 131 ++++++++++++++++++ .../opensearch/indexMappingsTemplate.json | 131 ++++++++++++++++++ .../DataInsight/DataInsightChartCard.tsx | 53 ++++--- .../DataInsight/DataInsightProgressBar.tsx | 4 +- .../Explore/AdvanceSearchModal.component.tsx | 14 +- .../AdvanceSearchProvider.component.tsx | 10 +- .../AdvanceSearchProvider.interface.ts | 9 ++ .../ui/src/utils/DataInsightUtils.tsx | 37 +++-- 14 files changed, 359 insertions(+), 61 deletions(-) diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/insights/workflows/dataAssets/processors/DataInsightsEntityEnricherProcessor.java b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/insights/workflows/dataAssets/processors/DataInsightsEntityEnricherProcessor.java index 4be08f85fea..e9bb1497df8 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/insights/workflows/dataAssets/processors/DataInsightsEntityEnricherProcessor.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/apps/bundles/insights/workflows/dataAssets/processors/DataInsightsEntityEnricherProcessor.java @@ -12,6 +12,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import lombok.extern.slf4j.Slf4j; import org.glassfish.jersey.internal.util.ExceptionUtils; import org.openmetadata.common.utils.CommonUtil; @@ -37,6 +38,7 @@ public class DataInsightsEntityEnricherProcessor implements Processor>, ResultList> { private final StepStats stats = new StepStats(); + private static final Set NON_TIER_ENTITIES = Set.of("tag", "glossaryTerm", "dataProduct"); public DataInsightsEntityEnricherProcessor(int total) { this.stats.withTotalRecords(total).withSuccessRecords(0).withFailedRecords(0); @@ -172,7 +174,15 @@ public class DataInsightsEntityEnricherProcessor if (oEntityTags.isPresent()) { Optional oEntityTier = getEntityTier(oEntityTags.get().stream().map(TagLabel::getTagFQN).toList()); - oEntityTier.ifPresent(s -> entityMap.put("tier", s)); + oEntityTier.ifPresentOrElse( + s -> entityMap.put("tier", s), + () -> { + if (!NON_TIER_ENTITIES.contains(entityType)) { + entityMap.put("tier", "NoTier"); + } + }); + } else if (!NON_TIER_ENTITIES.contains(entityType)) { + entityMap.put("tier", "NoTier"); } // Enrich with Description Stats diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v150/MigrationUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v150/MigrationUtil.java index 98cb58d85e5..59cd8da2068 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v150/MigrationUtil.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/migration/utils/v150/MigrationUtil.java @@ -287,7 +287,10 @@ public class MigrationUtil { dataInsightSystemChartRepository = new DataInsightSystemChartRepository(); String exclude_tags_filter = - "{\"query\":{\"bool\":{\"must\":[{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"tag\"}}}},{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"glossaryTerm\"}}}}]}}}"; + "{\"query\":{\"bool\":{\"must\":[{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"tag\"}}}},{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"glossaryTerm\"}}}},{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"dataProduct\"}}}}]}}}"; + + String exclude_tier_filter = + "{\"query\":{\"bool\":{\"must\":[{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"tag\"}}}},{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"glossaryTerm\"}}}},{\"bool\":{\"must_not\":{\"term\":{\"entityType.keyword\":\"dataProduct\"}}}},{\"bool\":{\"must_not\":{\"term\":{\"tier.keyword\":\"NoTier\"}}}}]}}}"; // total data assets List excludeList = List.of("tag", "glossaryTerm"); createChart( @@ -359,7 +362,7 @@ public class MigrationUtil { "total_data_assets_with_tier_summary_card", new SummaryCard() .withFormula("(count(k='id.keyword',q='tier.keyword: *')/count(k='id.keyword'))*100") - .withFilter(exclude_tags_filter)); + .withFilter(exclude_tier_filter)); // percentage of Data Asset with Description KPI createChart( diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchDynamicChartAggregatorInterface.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchDynamicChartAggregatorInterface.java index 4941fed68f3..72164857917 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchDynamicChartAggregatorInterface.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchDynamicChartAggregatorInterface.java @@ -147,7 +147,7 @@ public interface ElasticSearchDynamicChartAggregatorInterface { throws IOException { if (formula != null) { - if (filter != null) { + if (filter != null && !filter.equals("{}")) { XContentParser filterParser = XContentType.JSON .xContent() @@ -162,7 +162,7 @@ public interface ElasticSearchDynamicChartAggregatorInterface { // process non formula date histogram ValuesSourceAggregationBuilder subAgg = getSubAggregationsByFunction(function, field, 0); - if (filter != null) { + if (filter != null && !filter.equals("{}")) { XContentParser filterParser = XContentType.JSON .xContent() diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchSummaryCardAggregator.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchSummaryCardAggregator.java index 3291748c12f..3c23fb1a0e5 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchSummaryCardAggregator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/elasticsearch/dataInsightAggregators/ElasticSearchSummaryCardAggregator.java @@ -71,7 +71,7 @@ public class ElasticSearchSummaryCardAggregator searchResponse.getAggregations().asList(), summaryCard.getFormula(), null, formulas); List finalResults = new ArrayList<>(); - for (int i = results.size() - 1; i >= 0; i++) { + for (int i = results.size() - 1; i >= 0; i--) { if (results.get(i).getCount() != null) { finalResults.add(results.get(i)); resultList.setResults(finalResults); diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchDynamicChartAggregatorInterface.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchDynamicChartAggregatorInterface.java index 6096e6c2bdf..aaf15cbcfc6 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchDynamicChartAggregatorInterface.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchDynamicChartAggregatorInterface.java @@ -146,7 +146,7 @@ public interface OpenSearchDynamicChartAggregatorInterface { List formulas) throws IOException { if (formula != null) { - if (filter != null) { + if (filter != null && !filter.equals("{}")) { XContentParser filterParser = XContentType.JSON .xContent() @@ -161,7 +161,7 @@ public interface OpenSearchDynamicChartAggregatorInterface { // process non formula date histogram ValuesSourceAggregationBuilder subAgg = getSubAggregationsByFunction(function, field, 0); - if (filter != null) { + if (filter != null && !filter.equals("{}")) { XContentParser filterParser = XContentType.JSON .xContent() diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchSummaryCardAggregator.java b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchSummaryCardAggregator.java index 57b041a1274..bb45455f90b 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchSummaryCardAggregator.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/search/opensearch/dataInsightAggregator/OpenSearchSummaryCardAggregator.java @@ -69,7 +69,7 @@ public class OpenSearchSummaryCardAggregator implements OpenSearchDynamicChartAg searchResponse.getAggregations().asList(), summaryCard.getFormula(), null, formulas); List finalResults = new ArrayList<>(); - for (int i = results.size() - 1; i >= 0; i++) { + for (int i = results.size() - 1; i >= 0; i--) { if (results.get(i).getCount() != null) { finalResults.add(results.get(i)); resultList.setResults(finalResults); diff --git a/openmetadata-service/src/main/resources/dataInsights/elasticsearch/indexMappingsTemplate.json b/openmetadata-service/src/main/resources/dataInsights/elasticsearch/indexMappingsTemplate.json index db90bec9423..f8fdd93ff83 100644 --- a/openmetadata-service/src/main/resources/dataInsights/elasticsearch/indexMappingsTemplate.json +++ b/openmetadata-service/src/main/resources/dataInsights/elasticsearch/indexMappingsTemplate.json @@ -1,9 +1,140 @@ { "template": { + "settings": { + "analysis": { + "normalizer": { + "lowercase_normalizer": { + "type": "custom", + "char_filter": [], + "filter": [ + "lowercase" + ] + } + }, + "analyzer": { + "om_analyzer": { + "tokenizer": "letter", + "filter": [ + "lowercase", + "om_stemmer" + ] + }, + "om_ngram": { + "tokenizer": "ngram", + "min_gram": 3, + "max_gram": 10, + "filter": [ + "lowercase" + ] + } + }, + "filter": { + "om_stemmer": { + "type": "stemmer", + "name": "english" + } + } + } + }, "mappings": { "properties": { "@timestamp": { "type": "date" + }, + "owners": { + "properties": { + "id": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 36 + } + } + }, + "type": { + "type": "keyword" + }, + "name": { + "type": "keyword", + "normalizer": "lowercase_normalizer", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "displayName": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "normalizer": "lowercase_normalizer", + "ignore_above": 256 + } + } + }, + "fullyQualifiedName": { + "type": "text" + }, + "description": { + "type": "text" + }, + "deleted": { + "type": "text" + }, + "href": { + "type": "text" + } + } + }, + "domain": { + "properties": { + "id": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 36 + } + } + }, + "type": { + "type": "keyword" + }, + "name": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "displayName": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "normalizer": "lowercase_normalizer", + "ignore_above": 256 + } + } + }, + "fullyQualifiedName": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "deleted": { + "type": "text" + }, + "href": { + "type": "text" + } + } } } } diff --git a/openmetadata-service/src/main/resources/dataInsights/opensearch/indexMappingsTemplate.json b/openmetadata-service/src/main/resources/dataInsights/opensearch/indexMappingsTemplate.json index db90bec9423..f8fdd93ff83 100644 --- a/openmetadata-service/src/main/resources/dataInsights/opensearch/indexMappingsTemplate.json +++ b/openmetadata-service/src/main/resources/dataInsights/opensearch/indexMappingsTemplate.json @@ -1,9 +1,140 @@ { "template": { + "settings": { + "analysis": { + "normalizer": { + "lowercase_normalizer": { + "type": "custom", + "char_filter": [], + "filter": [ + "lowercase" + ] + } + }, + "analyzer": { + "om_analyzer": { + "tokenizer": "letter", + "filter": [ + "lowercase", + "om_stemmer" + ] + }, + "om_ngram": { + "tokenizer": "ngram", + "min_gram": 3, + "max_gram": 10, + "filter": [ + "lowercase" + ] + } + }, + "filter": { + "om_stemmer": { + "type": "stemmer", + "name": "english" + } + } + } + }, "mappings": { "properties": { "@timestamp": { "type": "date" + }, + "owners": { + "properties": { + "id": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 36 + } + } + }, + "type": { + "type": "keyword" + }, + "name": { + "type": "keyword", + "normalizer": "lowercase_normalizer", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "displayName": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "normalizer": "lowercase_normalizer", + "ignore_above": 256 + } + } + }, + "fullyQualifiedName": { + "type": "text" + }, + "description": { + "type": "text" + }, + "deleted": { + "type": "text" + }, + "href": { + "type": "text" + } + } + }, + "domain": { + "properties": { + "id": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 36 + } + } + }, + "type": { + "type": "keyword" + }, + "name": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "ignore_above": 256 + } + } + }, + "displayName": { + "type": "keyword", + "fields": { + "keyword": { + "type": "keyword", + "normalizer": "lowercase_normalizer", + "ignore_above": 256 + } + } + }, + "fullyQualifiedName": { + "type": "keyword" + }, + "description": { + "type": "text" + }, + "deleted": { + "type": "text" + }, + "href": { + "type": "text" + } + } } } } diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightChartCard.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightChartCard.tsx index 5cbb52ae927..02137912f08 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightChartCard.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightChartCard.tsx @@ -18,7 +18,8 @@ import { groupBy, includes, last, - map, + omit, + reduce, round, sortBy, startCase, @@ -100,39 +101,50 @@ export const DataInsightChartCard = ({ const { rightSideEntityList, latestData, graphData, changeInValue } = useMemo(() => { const results = chartData.results ?? []; + const timeStampResults = groupBy(results, 'day'); - const groupedResults = groupBy(results, 'group'); - const latestData: Record = {}; - let total = 0; + const graphResults = Object.entries(timeStampResults).map( + ([key, value]) => { + const keys = value.reduce((acc, curr) => { + return { ...acc, [curr.group ?? 'count']: curr.count }; + }, {}); - let firstRecordTotal = 0; + return { + day: +key, + ...keys, + }; + } + ); - Object.entries(groupedResults).forEach(([key, value]) => { - const newValues = sortBy(value, 'day'); + const finalData = sortBy(graphResults, 'day'); - latestData[key] = last(newValues)?.count ?? 0; + const latestData: Record = omit( + last(finalData ?? {}), + 'day' + ); - total += latestData[key]; - firstRecordTotal += first(newValues)?.count ?? 0; - }); + const total = reduce(latestData, (acc, value) => acc + value, 0); + + const firstRecordTotal = reduce( + omit(first(finalData) ?? {}, 'day'), + (acc, value) => acc + value, + 0 + ); + + const uniqueLabels = Object.entries(latestData) + .sort(([, valueA], [, valueB]) => valueB - valueA) + .map(([key]) => key); const changeInValue = firstRecordTotal ? (total - firstRecordTotal) / firstRecordTotal : 0; - const graphData = map(groupedResults, (value, key) => ({ - name: key, - data: value, - })); - - const labels = Object.keys(groupedResults); - return { - rightSideEntityList: labels.filter((entity) => + rightSideEntityList: uniqueLabels.filter((entity) => includes(toLower(entity), toLower(searchEntityKeyWord)) ), latestData, - graphData, + graphData: finalData, changeInValue, }; }, [chartData.results, searchEntityKeyWord]); @@ -267,6 +279,7 @@ export const DataInsightChartCard = ({ id={`${type}-graph`}> {renderDataInsightLineChart( graphData, + rightSideEntityList, activeKeys, activeMouseHoverKey, isPercentageGraph diff --git a/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightProgressBar.tsx b/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightProgressBar.tsx index 9d3e0ca3fb9..0975837122d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightProgressBar.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/DataInsight/DataInsightProgressBar.tsx @@ -63,7 +63,7 @@ const DataInsightProgressBar = ({ className="data-insight-progress-bar" format={() => ( <> - {target && ( + {target ? ( @@ -72,7 +72,7 @@ const DataInsightProgressBar = ({ {suffix} - )} + ) : null} )} percent={progress} diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchModal.component.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchModal.component.tsx index a01f27b8655..d3aaafd1e20 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchModal.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchModal.component.tsx @@ -30,7 +30,8 @@ export const AdvancedSearchModal: FunctionComponent = ({ onCancel, }: Props) => { const { t } = useTranslation(); - const { config, treeInternal, onTreeUpdate, onReset } = useAdvanceSearch(); + const { config, treeInternal, onTreeUpdate, onReset, modalProps } = + useAdvanceSearch(); return ( = ({ maskClosable={false} okText={t('label.submit')} open={visible} - title={t('label.advanced-entity', { - entity: t('label.search'), - })} + title={ + modalProps?.title ?? + t('label.advanced-entity', { + entity: t('label.search'), + }) + } width={950} onCancel={onCancel}> - {t('message.advanced-search-message')} + {modalProps?.subTitle ?? t('message.advanced-search-message')} ( export const AdvanceSearchProvider = ({ children, + isExplorePage = true, + modalProps, }: AdvanceSearchProviderProps) => { const tierOptions = useMemo(getTierOptions, []); @@ -74,11 +75,6 @@ export const AdvanceSearchProvider = ({ return tabInfo[0] as SearchIndex; }, [tabsInfo, tab]); - const isExplorePage = useMemo( - () => location.pathname.startsWith(ROUTES.EXPLORE), - [location] - ); - const [searchIndex, setSearchIndex] = useState< SearchIndex | Array >(getSearchIndexFromTabInfo()); @@ -331,6 +327,7 @@ export const AdvanceSearchProvider = ({ onUpdateConfig: handleConfigUpdate, onChangeSearchIndex: changeSearchIndex, onSubmit: handleSubmit, + modalProps, }), [ queryFilter, @@ -345,6 +342,7 @@ export const AdvanceSearchProvider = ({ handleConfigUpdate, changeSearchIndex, handleSubmit, + modalProps, ] ); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchProvider/AdvanceSearchProvider.interface.ts b/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchProvider/AdvanceSearchProvider.interface.ts index f59fabb5188..ace1ad593a6 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchProvider/AdvanceSearchProvider.interface.ts +++ b/openmetadata-ui/src/main/resources/ui/src/components/Explore/AdvanceSearchProvider/AdvanceSearchProvider.interface.ts @@ -16,6 +16,11 @@ import { SearchIndex } from '../../../enums/search.enum'; export interface AdvanceSearchProviderProps { children: ReactNode; + isExplorePage?: boolean; + modalProps?: { + title?: string; + subTitle?: string; + }; } export interface AdvanceSearchContext { @@ -31,6 +36,10 @@ export interface AdvanceSearchContext { onChangeSearchIndex: (index: SearchIndex | Array) => void; searchIndex: string | Array; onSubmit: () => void; + modalProps?: { + title?: string; + subTitle?: string; + }; } export type FilterObject = Record; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/DataInsightUtils.tsx b/openmetadata-ui/src/main/resources/ui/src/utils/DataInsightUtils.tsx index 3d001c0f9dc..ed32e89e081 100644 --- a/openmetadata-ui/src/main/resources/ui/src/utils/DataInsightUtils.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/utils/DataInsightUtils.tsx @@ -601,21 +601,18 @@ export const isPercentageSystemGraph = (graph: SystemChartType) => { SystemChartType.PercentageOfDataAssetWithOwner, SystemChartType.PercentageOfServiceWithDescription, SystemChartType.PercentageOfServiceWithOwner, - SystemChartType.TotalDataAssetsByTier, ].includes(graph); }; export const renderDataInsightLineChart = ( - graphData: { - name: string; - data: { day: number; count: number }[]; - }[], + graphData: Array>, + labels: string[], activeKeys: string[], activeMouseHoverKey: string, - isPercentage = true + isPercentage: boolean ) => { return ( - + axisTickFormatter(value, '%')} + tickFormatter={ + isPercentage + ? (value: number) => axisTickFormatter(value, '%') + : undefined + } /> - {graphData.map((s, i) => ( + {labels.map((s, i) => (