feat(tableau): adds more reporting metrics to better understand lineage construction in tableau ingestion #12008

sgomezvillamor · 2024-12-03T10:02:24Z

This adds metrics to track table and column lineage generation as well as some error scenarios preventing us to generate lineage.

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

…stion

hsheth2 · 2024-12-03T19:28:32Z

metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py

@@ -2114,6 +2117,15 @@ def parse_custom_sql(
            schema_aware=not self.config.sql_parsing_disable_schema_awareness,
        )

+        if parsed_result is None or parsed_result.debug_info.error:
+            message = f"Failed to extract column level lineage from datasource {datasource_urn}"
+            if parsed_result is not None and parsed_result.debug_info.error:


this check is redundant, right?

as per my understanding, not redundant

I just rewrote code below and moved it into the parse_custom_sql function, as there were two callers of the function doing about the same:

if parsed_result is None: logger.info( f"Failed to extract column level lineage from datasource {datasource_urn}" ) return [] if parsed_result.debug_info.error: logger.info( f"Failed to extract column level lineage from datasource {datasource_urn}: {parsed_result.debug_info.error}" ) return []

so create_lineage_sql_parsed_result will never return None - if there was an error, it would be part of the SqlParsingResult object

in debug_info, error is an alias for .table_error or .column_error (since only one of those will be set).

datahub/metadata-ingestion/src/datahub/sql_parsing/sqlglot_lineage.py

Lines 156 to 158 in 1bfd4ee

@property

def error(self) -> Optional[Exception]:

return self.table_error or self.column_error

if there's a column error, we still can generate table-level lineage - so I think the error messages might need to be tweaked

so create_lineage_sql_parsed_result will never return None

You are right. Then definitely parsed_result is None becomes unnecessary.

I made warning messages more detailed depending on the error type with commit e585d14 .

hsheth2 · 2024-12-03T19:29:30Z

metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py

@@ -599,7 +599,13 @@ class TableauSourceReport(StaleEntityRemovalSourceReport):
    num_datasource_field_skipped_no_name: int = 0
    num_csql_field_skipped_no_name: int = 0
    num_table_field_skipped_no_name: int = 0
+    # lineage
+    num_upstream_table_lineage: int = 0


is this the number of tableau entities with upstream lineage, or the number of upstream lineage entries across all upstreamLineage aspects?

imo the former would be more useful

the second, every time an UpstreamLineage aspect is emitted, the counter is increased

hsheth2 · 2024-12-04T20:32:14Z

metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py

@@ -2352,6 +2360,10 @@ def emit_datasource(
                    aspect_name=c.UPSTREAM_LINEAGE,
                    aspect=upstream_lineage,
                )
+                self.report.num_upstream_table_lineage += len(upstream_tables)


imo having a simple count of the # of tables with an upstream lineage is more useful. it's likely pretty rare that tables have more than one upstream, but this lets us say things like "we have X% lineage coverage" and is more inline with what we report in other sources

Suggested change

self.report.num_upstream_table_lineage += len(upstream_tables)

self.report.num_upstreams_with_table_lineage += 1

it's likely pretty rare that tables have more than one upstream

A table or view created with a select statement (CTAS) will have all tables in the FROM as upstream lineage, right? in that case I expect that selects with multiple tables in the FROM will be very common.

Anyway, we can easily have both counters and validate our assumptions.

addressed in commit c6875e9

Eg. these are the metrics I got when testing with Acryl's tableau

'num_tables_with_upstream_lineage': 29, 'num_upstream_table_lineage': 49, 'num_upstream_fine_grained_lineage': 516, 'num_upstream_table_skipped_no_columns': 3,

hsheth2 · 2024-12-04T20:34:01Z

metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py

@@ -2114,6 +2117,15 @@ def parse_custom_sql(
            schema_aware=not self.config.sql_parsing_disable_schema_awareness,
        )

+        if parsed_result is None or parsed_result.debug_info.error:
+            message = f"Failed to extract column level lineage from datasource {datasource_urn}"
+            if parsed_result is not None and parsed_result.debug_info.error:


so create_lineage_sql_parsed_result will never return None - if there was an error, it would be part of the SqlParsingResult object

in debug_info, error is an alias for .table_error or .column_error (since only one of those will be set).

datahub/metadata-ingestion/src/datahub/sql_parsing/sqlglot_lineage.py

Lines 156 to 158 in 1bfd4ee

@property

def error(self) -> Optional[Exception]:

return self.table_error or self.column_error

if there's a column error, we still can generate table-level lineage - so I think the error messages might need to be tweaked

metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py

…u.py

sgomezvillamor added 2 commits December 3, 2024 10:59

feat: adds reporting metrics for lineage construction in tableau inge…

7697ab7

…stion

Merge branch 'master' into feat-ingestion-tableau-report-metrics

f618e8b

github-actions bot added ingestion PR or Issue related to the ingestion of metadata community-contribution PR or Issue raised by member(s) of DataHub Community labels Dec 3, 2024

datahub-cyborg bot added the needs-review Label for PRs that need review from a maintainer. label Dec 3, 2024

vercel bot deployed to Preview December 3, 2024 10:37 View deployment

sgomezvillamor changed the title ~~feat: adds more reporting metrics to better understand lineage construction in tableau ingestion~~ feat(tablea): adds more reporting metrics to better understand lineage construction in tableau ingestion Dec 3, 2024

sgomezvillamor changed the title ~~feat(tablea): adds more reporting metrics to better understand lineage construction in tableau ingestion~~ feat(tableau): adds more reporting metrics to better understand lineage construction in tableau ingestion Dec 3, 2024

hsheth2 requested changes Dec 3, 2024

View reviewed changes

datahub-cyborg bot added pending-submitter-response Issue/request has been reviewed but requires a response from the submitter and removed needs-review Label for PRs that need review from a maintainer. labels Dec 3, 2024

Merge branch 'master' into feat-ingestion-tableau-report-metrics

fe4af06

vercel bot deployed to Preview December 4, 2024 07:26 View deployment

Merge branch 'master' into feat-ingestion-tableau-report-metrics

cb0bd5b

vercel bot deployed to Preview December 4, 2024 15:18 View deployment

Merge branch 'master' into feat-ingestion-tableau-report-metrics

617f802

anshbansal removed the community-contribution PR or Issue raised by member(s) of DataHub Community label Dec 4, 2024

vercel bot deployed to Preview December 4, 2024 16:31 View deployment

hsheth2 reviewed Dec 4, 2024

View reviewed changes

num_tables_with_upstream_lineage

c6875e9

datahub-cyborg bot added needs-review Label for PRs that need review from a maintainer. and removed pending-submitter-response Issue/request has been reviewed but requires a response from the submitter labels Dec 5, 2024

sgomezvillamor added 2 commits December 5, 2024 07:54

tweak failed parse sql warning messages

e585d14

Merge branch 'master' into feat-ingestion-tableau-report-metrics

dddc744

vercel bot deployed to Preview December 5, 2024 07:29 View deployment

hsheth2 approved these changes Dec 5, 2024

View reviewed changes

metadata-ingestion/src/datahub/ingestion/source/tableau/tableau.py Outdated Show resolved Hide resolved

Update metadata-ingestion/src/datahub/ingestion/source/tableau/tablea…

ffc61b4

…u.py

datahub-cyborg bot added pending-submitter-response Issue/request has been reviewed but requires a response from the submitter and removed needs-review Label for PRs that need review from a maintainer. labels Dec 5, 2024

vercel bot deployed to Preview December 5, 2024 16:56 View deployment

Merge branch 'master' into feat-ingestion-tableau-report-metrics

9c98064

vercel bot deployed to Preview December 5, 2024 17:33 View deployment

Merge branch 'master' into feat-ingestion-tableau-report-metrics

7618944

sgomezvillamor enabled auto-merge (squash) December 9, 2024 07:20

sgomezvillamor merged commit 0e7ebaf into datahub-project:master Dec 9, 2024
63 of 64 checks passed

sgomezvillamor deleted the feat-ingestion-tableau-report-metrics branch December 9, 2024 07:30

vercel bot deployed to Preview December 9, 2024 07:37 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(tableau): adds more reporting metrics to better understand lineage construction in tableau ingestion #12008

feat(tableau): adds more reporting metrics to better understand lineage construction in tableau ingestion #12008

sgomezvillamor commented Dec 3, 2024

hsheth2 Dec 3, 2024

sgomezvillamor Dec 4, 2024 •

edited

Loading

hsheth2 Dec 4, 2024

sgomezvillamor Dec 5, 2024

hsheth2 Dec 3, 2024

sgomezvillamor Dec 4, 2024

hsheth2 Dec 4, 2024

sgomezvillamor Dec 5, 2024

sgomezvillamor Dec 5, 2024

sgomezvillamor Dec 5, 2024

hsheth2 Dec 4, 2024

	@property
	def error(self) -> Optional[Exception]:
	return self.table_error or self.column_error

	self.report.num_upstream_table_lineage += len(upstream_tables)
	self.report.num_upstreams_with_table_lineage += 1

feat(tableau): adds more reporting metrics to better understand lineage construction in tableau ingestion #12008

feat(tableau): adds more reporting metrics to better understand lineage construction in tableau ingestion #12008

Conversation

sgomezvillamor commented Dec 3, 2024

Checklist

Choose a reason for hiding this comment

sgomezvillamor Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgomezvillamor Dec 4, 2024 •

edited

Loading