GEN-970: Refactor redshift system metrics to support freshness test

[sushi30]

Describe your changes:

• moved redshift system metrics to the redshift source module
• use Timestamp in data quality
• added plugin feature to test utils

• run e2e tests

E2E runs

https://github.com/open-metadata/OpenMetadata/actions/workflows/py-cli-e2e-tests.yml?query=branch%3Aredshift-system-metrics-refactor

Type of change:

• Bug fix
• Improvement
• New feature
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

[notion-workspace]

Freshness test case by system metrics

[github-actions]

The Python checkstyle failed.

Please run make py_format and py_format_check in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Python code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

[sonarcloud]

Quality Gate passed for 'open-metadata-ingestion'

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
67.2% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

[TeddyCr]

@sushi30 It makes sense to move the computation logic in the connector source itself, though, let's move all of them and keep an entry point to compute the system metric in ingestion/src/metadata/profiler/metrics/system/system.py. so that we have the profiler calling a system.py and inside system.py get the correct System class (e.g. redshift, Snowflake, etc.) and have it call something like SystemMetricsInterface redshift_system_metrics = RedshiftSystemMetric(....); redshift_system_metrics.compute

This way we keep profiler metric computation logic close to the profiler and the implementation close to the source.

[sushi30]

I am moving them one by one as there are side effects and hooks involved. this change is pretty big as it is.

[TeddyCr]

What do we solve with this?

[sushi30]

sqlalchemy hooks for system do not get picked up if this class is not imported explicitly. the system profile for redshift will not run with current setup. we currently rely on this side effect when importing get_system_metrics_for_dialect

[TeddyCr]

I think we should take a different approach as suggested here #17981 (comment) to not have to rely on the dispatch

[sushi30]

This requires a larger change in how the system profiler works. I can add it as the next iteration.

[TeddyCr]

You can do it in stage no? Keep get_system_metrics_for_dialect (which we should probably keep anyway as the entry point), move all the @get_system_metrics_for_dialect.register(Dialects.BigQuery) to their own classes (you can keep it in the same file for now) and simply call a factory method in get_system_metrics_for_dialect to instantiate the right system metric builder class (that would inherit from the interface) and call interface.process.

That seems limited in scope and avoid introducing the side effect import to support the dispatch in the redshift package, wdyt?

[sushi30]

How do you suggest to pick up the @get_system_metrics_for_dialect.register without importing the file as side effect? The file containing the function needs to imported in order for the dispatcher to be registered. There is nothing explicitly importing it bc the profiler relies only on the "parent" get_system_metrics_for_dialect.