Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

openlineage: adjust log levels #34801

Merged
merged 1 commit into from Oct 11, 2023

Conversation

JDarDagran
Copy link
Contributor

OpenLineage provider is reported to be a bit too verbose. This PR attempts to lower logging levels of some messages and also cleans up the code.

@JDarDagran JDarDagran force-pushed the openlineage/adjust-log-levels branch 2 times, most recently from 2e30896 to 0f9fe36 Compare October 8, 2023 16:10
@JDarDagran JDarDagran force-pushed the openlineage/adjust-log-levels branch 2 times, most recently from ff708c5 to 50cd52e Compare October 10, 2023 09:32
@@ -82,7 +82,7 @@ def extract(self) -> OperatorLineage | None:
self.operator.__class__.__module__ + "." + self.operator.__class__.__name__
)
if fully_qualified_class_name in self.disabled_operators:
self.log.warning(
self.log.info(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be DEBUG too, no? i.e user performed an action to disable the lineage for specific operators

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

@@ -144,10 +144,10 @@ def _get_openlineage_facets(self, get_facets_method, *args) -> OperatorLineage |
job_facets=facets.job_facets,
)
except ImportError:
self.log.exception(
self.log.error(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.log.error(
self.log.exception(

exception seems fine here

return
self.executor.submit(self.adapter.dag_success, dag_run=dag_run, msg=msg)

@hookimpl
def on_dag_run_failed(self, dag_run: DagRun, msg: str):
if not self.executor:
self.log.error("Executor have not started before `on_dag_run_failed`")
self.log.warning("Executor have not started before `on_dag_run_failed`")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.log.warning("Executor have not started before `on_dag_run_failed`")
self.log.debug("Executor have not started before `on_dag_run_failed`")

@@ -194,14 +194,14 @@ def on_dag_run_running(self, dag_run: DagRun, msg: str):
@hookimpl
def on_dag_run_success(self, dag_run: DagRun, msg: str):
if not self.executor:
self.log.error("Executor have not started before `on_dag_run_success`")
self.log.warning("Executor have not started before `on_dag_run_success`")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
self.log.warning("Executor have not started before `on_dag_run_success`")
self.log.debug("Executor have not started before `on_dag_run_success`")

Signed-off-by: Jakub Dardzinski <kuba0221@gmail.com>
@JDarDagran
Copy link
Contributor Author

Thanks @kaxil ! I've commited changes as suggested.

@kaxil kaxil merged commit 73dd877 into apache:main Oct 11, 2023
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants