QA engine: add adoption metrics to the QA report #21917

alafanechere · 2023-01-26T14:51:32Z

What

Closes #21721
We want to have adoption metrics per connector in the QA report:

number_of_connections:
number_of_users
sync_success_rate
total_syncs_count
failed_syncs_count
succeeded_syncs_count

We implemented the logic to fetch this data in a previous PR.
This PR adds this data to the QA report dataframe but does not persist it to GCS as we consider this data private.

alafanechere · 2023-01-26T15:01:28Z

tools/ci_connector_ops/ci_connector_ops/qa_engine/inputs.py

@@ -50,6 +49,3 @@ def fetch_adoption_metrics_per_connector_version() -> pd.DataFrame:
        "total_syncs_count",
        "sync_success_rate",
    ]]
-
-CLOUD_CATALOG = fetch_remote_catalog(CLOUD_CATALOG_URL)


I removed these constants from the module because they cause unnecessary network call on module import. I preferred to use dependency injections and test fixtures to expose these (direct calls to the fetch_remote_catalog function.

…report

github-actions · 2023-01-26T15:03:58Z

Airbyte Code Coverage

There is no coverage information present for the Files changed

Total Project Coverage	24%	❌

…report

alafanechere · 2023-01-26T19:18:00Z

Switching it to draft because I'm considering changing to storage backend to bigquery.

bnchrch

Seems strainght forward to me.

Im going to hold off approving for the first week or so though 😛

But for my own understanding. What is the purpose of the

QA report dataframe

?

bnchrch · 2023-01-26T21:06:50Z

tools/ci_connector_ops/ci_connector_ops/qa_engine/validations.py

@@ -20,7 +19,7 @@ def url_is_reachable(url: str) -> bool:
 def is_appropriate_for_cloud_use(definition_id: str) -> bool:
    return definition_id not in INAPPROPRIATE_FOR_CLOUD_USE_CONNECTORS

-def get_qa_report(enriched_catalog: pd.DataFrame) -> pd.DataFrame:
+def get_qa_report(enriched_catalog: pd.DataFrame, oss_catalog_length: int) -> pd.DataFrame:


Always a fan of bring only the data you need 👍

bnchrch · 2023-01-26T21:10:27Z

tools/ci_connector_ops/tests/test_qa_engine/test_main.py

+    )
+    main.validations.get_qa_report.assert_called_with(
+        main.enrichments.get_enriched_catalog.return_value,
+        3 # len of the "oss" string...


Could you do this instead?

Suggested change

3 # len of the "oss" string...

len(main.enrichments.get_enriched_catalog.return_value)

alafanechere · 2023-01-27T10:05:54Z

After discussion with @evantahler the adoption metrics should be considered private.
I made the required changes not to persist these fields in our public GCS bucket and disabled GCS persistence for extra safety.

The storage persistence logic is going to change soon following discussions with @airbytehq/airbyte-analytics :

We might store the QA report without adoption metrics to GCS and ingest this data into BigQuery
The adoption metrics computation will be done by a DBT model and not computed from the QA engine
The QA report consumers will query BigQuery to get the QA report with adoption metrics instead of consuming a JSON file.

In the meantime we can continue the work planned on this project:

The QA report with adoption metrics can be consumed in memory. The Cloud Availability Updater will call get_qa_report instead of reading a file from GCS.

…report

alafanechere · 2023-01-27T10:07:26Z

What is the purpose of the QA report dataframe

@bechurch this dataframe will be used to spot which connectors not on Airbyte cloud are eligible for it according to the QA flags we computed.

alafanechere · 2023-01-27T10:08:18Z

tools/ci_connector_ops/ci_connector_ops/qa_engine/outputs.py

+
+from .models import ConnectorQAReport
+
+def persist_qa_report(qa_report: pd.DataFrame, path: str, public_fields_only: bool =True):


This is not called as I disabled GCS persistence in main for safety.

👍🏻 I assume we'll usually persist with all of the info, but can use the public-only if we plan to use it in the public repo anywhere

erohmensing · 2023-01-27T15:22:41Z

tools/ci_connector_ops/ci_connector_ops/qa_engine/enrichments.py

    enriched_catalog.columns = enriched_catalog.columns.str.replace(
        "(?<=[a-z])(?=[A-Z])", "_", regex=True
    ).str.lower() # column names to snake case


Nit: might be nice to move this into a function instead of needing comment to explain what its doing:

enriched_catalog.columns = enriched_catalog.columns.to_snake_case()

erohmensing · 2023-01-27T15:30:36Z

tools/ci_connector_ops/ci_connector_ops/qa_engine/enrichments.py

@@ -33,10 +38,13 @@ def get_enriched_catalog(oss_catalog: pd.DataFrame, cloud_catalog: pd.DataFrame)
    enriched_catalog.columns = enriched_catalog.columns.str.replace(
        "(?<=[a-z])(?=[A-Z])", "_", regex=True
    ).str.lower() # column names to snake case
+    enriched_catalog = enriched_catalog[[c for c in enriched_catalog.columns if "_del" not in c]]


Optional: I took a minute to understand why we were removing these - maybe using the suffix _cloud would make it clear that when merging the two, we prefer the OSS column and drop the cloud one

erohmensing · 2023-01-27T15:33:01Z

tools/ci_connector_ops/ci_connector_ops/qa_engine/models.py

+PUBLIC_FIELD = Field(..., is_public=True)
+PRIVATE_FIELD = Field(..., is_public=False)


erohmensing · 2023-01-27T15:37:46Z

tools/ci_connector_ops/ci_connector_ops/qa_engine/outputs.py

+
+from .models import ConnectorQAReport
+
+def persist_qa_report(qa_report: pd.DataFrame, path: str, public_fields_only: bool =True):


👍🏻 I assume we'll usually persist with all of the info, but can use the public-only if we plan to use it in the public repo anywhere

…report

alafanechere temporarily deployed to more-secrets January 26, 2023 14:53 — with GitHub Actions Inactive

alafanechere force-pushed the augustin/qa-engine/adoption-metrics-in-qa-report branch from cff4b3b to 9a4225e Compare January 26, 2023 14:57

alafanechere temporarily deployed to more-secrets January 26, 2023 14:59 — with GitHub Actions Inactive

add adoption metrics to the qa report

15a056b

alafanechere commented Jan 26, 2023

View reviewed changes

alafanechere force-pushed the augustin/qa-engine/adoption-metrics-in-qa-report branch from 9a4225e to 15a056b Compare January 26, 2023 15:02

Merge branch 'master' into augustin/qa-engine/adoption-metrics-in-qa-…

943a90c

…report

alafanechere marked this pull request as ready for review January 26, 2023 15:02

alafanechere requested a review from a team January 26, 2023 15:03

alafanechere temporarily deployed to more-secrets January 26, 2023 15:04 — with GitHub Actions Inactive

Merge branch 'master' into augustin/qa-engine/adoption-metrics-in-qa-…

ad0136a

…report

alafanechere temporarily deployed to more-secrets January 26, 2023 15:12 — with GitHub Actions Inactive

alafanechere marked this pull request as draft January 26, 2023 19:18

bnchrch reviewed Jan 26, 2023

View reviewed changes

alafanechere temporarily deployed to more-secrets January 27, 2023 09:50 — with GitHub Actions Inactive

alafanechere temporarily deployed to more-secrets January 27, 2023 09:51 — with GitHub Actions Inactive

make adoption fields not public

95dd606

alafanechere force-pushed the augustin/qa-engine/adoption-metrics-in-qa-report branch from f337ef5 to 95dd606 Compare January 27, 2023 09:52

alafanechere temporarily deployed to more-secrets January 27, 2023 09:54 — with GitHub Actions Inactive

disable GCS persistence

6d4feb3

alafanechere marked this pull request as ready for review January 27, 2023 10:05

Merge branch 'master' into augustin/qa-engine/adoption-metrics-in-qa-…

76e3cbf

…report

alafanechere requested review from bnchrch and evantahler January 27, 2023 10:06

alafanechere temporarily deployed to more-secrets January 27, 2023 10:07 — with GitHub Actions Inactive

alafanechere temporarily deployed to more-secrets January 27, 2023 10:08 — with GitHub Actions Inactive

alafanechere commented Jan 27, 2023

View reviewed changes

alafanechere mentioned this pull request Jan 27, 2023

qa-engine: define cloud eligibility #21961

Merged

erohmensing approved these changes Jan 27, 2023

View reviewed changes

rename _del to _cloud

dd33060

octavia-squidington-iv added the area/frontend Related to the Airbyte webapp label Jan 27, 2023

alafanechere temporarily deployed to more-secrets January 27, 2023 15:57 — with GitHub Actions Inactive

alafanechere force-pushed the augustin/qa-engine/adoption-metrics-in-qa-report branch from c53fb6f to dd33060 Compare January 27, 2023 15:57

octavia-squidington-iv added area/frontend Related to the Airbyte webapp and removed area/frontend Related to the Airbyte webapp labels Jan 27, 2023

fix conflicts

a687435

alafanechere force-pushed the augustin/qa-engine/adoption-metrics-in-qa-report branch from 46f8662 to a687435 Compare January 27, 2023 16:02

octavia-squidington-iv removed the area/frontend Related to the Airbyte webapp label Jan 27, 2023

alafanechere and others added 2 commits January 27, 2023 17:08

bump

6a6396d

Merge branch 'master' into augustin/qa-engine/adoption-metrics-in-qa-…

3b8c2c3

…report

alafanechere temporarily deployed to more-secrets January 27, 2023 16:13 — with GitHub Actions Inactive

alafanechere enabled auto-merge (squash) January 27, 2023 16:29

alafanechere merged commit 3a81ffc into master Jan 27, 2023

alafanechere deleted the augustin/qa-engine/adoption-metrics-in-qa-report branch January 27, 2023 16:46

This was referenced Jan 27, 2023

Bump Airbyte version from 0.40.29 to 0.40.30 #21991

Closed

Bump Airbyte version from 0.40.29 to 0.40.30 #22045

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QA engine: add adoption metrics to the QA report #21917

QA engine: add adoption metrics to the QA report #21917

alafanechere commented Jan 26, 2023 •

edited

Loading

alafanechere Jan 26, 2023

github-actions bot commented Jan 26, 2023 •

edited

Loading

alafanechere commented Jan 26, 2023

bnchrch left a comment

bnchrch Jan 26, 2023

bnchrch Jan 26, 2023

alafanechere commented Jan 27, 2023

alafanechere commented Jan 27, 2023

alafanechere Jan 27, 2023 •

edited

Loading

erohmensing Jan 27, 2023

erohmensing Jan 27, 2023

erohmensing Jan 27, 2023

erohmensing Jan 27, 2023

erohmensing Jan 27, 2023

	3 # len of the "oss" string...
	len(main.enrichments.get_enriched_catalog.return_value)


		from .models import ConnectorQAReport

		def persist_qa_report(qa_report: pd.DataFrame, path: str, public_fields_only: bool =True):

		PUBLIC_FIELD = Field(..., is_public=True)
		PRIVATE_FIELD = Field(..., is_public=False)

QA engine: add adoption metrics to the QA report #21917

QA engine: add adoption metrics to the QA report #21917

Conversation

alafanechere commented Jan 26, 2023 • edited Loading

What

Choose a reason for hiding this comment

github-actions bot commented Jan 26, 2023 • edited Loading

Airbyte Code Coverage

alafanechere commented Jan 26, 2023

bnchrch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alafanechere commented Jan 27, 2023

alafanechere commented Jan 27, 2023

alafanechere Jan 27, 2023 • 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

alafanechere commented Jan 26, 2023 •

edited

Loading

github-actions bot commented Jan 26, 2023 •

edited

Loading

alafanechere Jan 27, 2023 •

edited

Loading