[AIRFLOW-6065] Add Stackdriver Task Handler #6660

mik-laj · 2019-11-25T22:06:47Z

Make sure you have checked all steps below.

Jira

My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-6065
- In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
- In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
- In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

Here are some details about my PR, including screenshots of any UI changes:

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters (not including Jira issue reference)
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

Documentation

In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

codecov-io · 2019-12-04T17:58:26Z

Codecov Report

Merging #6660 into master will decrease coverage by 0.44%.
The diff coverage is 92.79%.

@@            Coverage Diff             @@
##           master    #6660      +/-   ##
==========================================
- Coverage    86.6%   86.16%   -0.45%     
==========================================
  Files         873      874       +1     
  Lines       40725    40836     +111     
==========================================
- Hits        35271    35186      -85     
- Misses       5454     5650     +196

Impacted Files	Coverage Δ
airflow/config_templates/airflow_local_settings.py	`65.38% <16.66%> (-6.36%)`	⬇️
airflow/utils/log/stackdriver_task_handler.py	`97.14% <97.14%> (ø)`
...w/providers/apache/hive/operators/mysql_to_hive.py	`35.84% <0%> (-64.16%)`	⬇️
airflow/kubernetes/volume_mount.py	`44.44% <0%> (-55.56%)`	⬇️
airflow/kubernetes/volume.py	`52.94% <0%> (-47.06%)`	⬇️
airflow/security/kerberos.py	`30.43% <0%> (-45.66%)`	⬇️
airflow/kubernetes/pod_launcher.py	`47.18% <0%> (-45.08%)`	⬇️
airflow/providers/mysql/operators/mysql.py	`55% <0%> (-45%)`	⬇️
airflow/kubernetes/refresh_config.py	`50.98% <0%> (-23.53%)`	⬇️
...viders/cncf/kubernetes/operators/kubernetes_pod.py	`70.21% <0%> (-23.41%)`	⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e00243...ea1c391. Read the comment docs.

airflow/utils/log/stackdriver_task_handler.py

potiuk · 2019-12-05T16:58:17Z

airflow/utils/log/stackdriver_task_handler.py

Do we want to have both read and emit in the same class? I think it will be better if we have separate "Logging Handler" and separate LogReader - maybe deriving from the same base class. I think those two have fundamentally different purpose and should be separated.

Also it should be described how to configure reading the logs from stackdriver. This can be separated out to a completely separate PR BTW.

I would like to avoid modifying the API, but in this change I would like to implement the current Airflow interface. Currently in Airflow the process of saving and reading is combined in one class.
If you want, you can configure another handler to read logs using task_log_reader option in airflow.cfg file.
https://github.com/apache/airflow/blob/master/airflow/config_templates/default_airflow.cfg#L191-L193
I agree that the log API needs improvement, but this is not the scope of this PR.

Can you look at my comment that discusses API issues?
#5177 (comment)

docs/howto/write-logs.rst

ashb · 2019-12-10T10:42:10Z

docs/howto/write-logs.rst

Suggested change

:class:`logging.config.dictConfig`. If your file is not importable, then you should set a :any:``PYTHONPATH`` environment.

:class:`logging.config.dictConfig`. If your file is not in a standard import location, then you should set a :any:``PYTHONPATH``

environment. Airflow adds the ``config/`` folder inside ``AIRFLOW_HOME`` to the import path for this purpose.

ashb · 2019-12-10T10:43:52Z

docs/howto/write-logs.rst

I don't think the __init__ is needed here, as ~/airflow/config is already in the python path so can be imported as log_config

Right. In Python 2.7 this was required.

ashb · 2019-12-10T10:46:04Z

docs/howto/write-logs.rst

We've had problems in the past with the default config changing and people's installs breaking.

I think we should instead recommend people do this as

form copy import deepcopy from airflow.config_templates.airflow_local_settings import DEFAULT_LOGGING_CONFIG LOGGING_CONFIG = deepcopy(DEFAULT_LOGGING_CONFIG) # Update LOGGING_CONFIG as needed.

I updated the description

docs/howto/write-logs.rst

potiuk · 2020-02-10T12:32:44Z

Please rebase to latest master @mik-laj - we had a six drama on Travis but it's solved now.

galuszkak · 2020-02-13T07:11:33Z

@potiuk @mik-laj nice, we started on same thing 2 weeks ago in our hourly morning mini hackathons. Sounds you did it a lot before us.

mik-laj · 2020-02-13T10:12:41Z

If you are interested, you can work on integrating Web UI with Stackdriver.
Here is POC: PolideaInternal#660
Unfortunately, I don't have time to finish it and test it.

galuszkak · 2020-02-20T07:27:27Z

@mik-laj we have every week mini hackathon so if this is not urgent we are doing this every Thursday 8:00-9:30 with 5-6 people.

Now we are working on enabling multiple remote loggers in Airflow, and we can also try to take this Stackdriver logs in Web UI.

mik-laj · 2020-02-20T12:37:43Z

If you would like to work more on OOS then we are recruiting ;-)
https://www.polidea.com/hiring/

This is absolutely not urgent. This is just one of my ideas that I came up with while working on this PR.

mik-laj force-pushed the AIRFLOW-6065 branch from a86098c to 6cab6ee Compare November 25, 2019 22:12

mik-laj changed the title ~~[AIRFLOW-6065][depends on AIRFLOW-6047] Add StackdriverTaskHandler~~ [AIRFLOW-6065][depends on AIRFLOW-6047][WIP] Add StackdriverTaskHandler Nov 25, 2019

mik-laj force-pushed the AIRFLOW-6065 branch 2 times, most recently from 52cad3b to 6f55793 Compare November 26, 2019 11:32

mik-laj changed the title ~~[AIRFLOW-6065][depends on AIRFLOW-6047][WIP] Add StackdriverTaskHandler~~ [AIRFLOW-6065][depends on AIRFLOW-6047][WIP] Add Stackdriver Task Handler Nov 26, 2019

mik-laj added the provider:google Google (including GCP) related issues label Nov 26, 2019

mik-laj force-pushed the AIRFLOW-6065 branch 2 times, most recently from 30bb377 to 9ebec28 Compare November 27, 2019 15:06

mik-laj changed the title ~~[AIRFLOW-6065][depends on AIRFLOW-6047][WIP] Add Stackdriver Task Handler~~ [AIRFLOW-6065] Add Stackdriver Task Handler Dec 3, 2019

mik-laj force-pushed the AIRFLOW-6065 branch from 9ebec28 to 1b53baf Compare December 3, 2019 22:27

potiuk requested changes Dec 5, 2019

View reviewed changes

mik-laj force-pushed the AIRFLOW-6065 branch from 2614057 to b5e6380 Compare December 5, 2019 19:19

mik-laj mentioned this pull request Dec 6, 2019

[AIRFLOW-5931] Use os.fork when appropriate to speed up task execution. #6627

Merged

8 tasks

ashb reviewed Dec 10, 2019

View reviewed changes

docs/howto/write-logs.rst Outdated Show resolved Hide resolved

turbaszek approved these changes Jan 4, 2020

View reviewed changes

mik-laj force-pushed the AIRFLOW-6065 branch from 288b2ac to f9de3f0 Compare January 15, 2020 16:03

boring-cyborg bot added area:docs area:logging labels Jan 15, 2020

mik-laj force-pushed the AIRFLOW-6065 branch from 9d75c52 to b073090 Compare February 10, 2020 11:55

mik-laj requested a review from potiuk February 10, 2020 11:55

mik-laj force-pushed the AIRFLOW-6065 branch 2 times, most recently from 2f181f4 to 66cd96c Compare February 10, 2020 14:10

Kamil Breguła added 3 commits February 11, 2020 11:42

[AIRFLOW-6065] Add Stackdriver Task Handler

90662ed

fixup! [AIRFLOW-6065] Add Stackdriver Task Handler

32d0511

fixup! fixup! [AIRFLOW-6065] Add Stackdriver Task Handler

ea1c391

mik-laj force-pushed the AIRFLOW-6065 branch from 66cd96c to ea1c391 Compare February 11, 2020 10:43

potiuk approved these changes Feb 11, 2020

View reviewed changes

mik-laj merged commit 6b19889 into apache:master Feb 12, 2020

galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020

[AIRFLOW-6065] Add Stackdriver Task Handler (apache#6660)

764c31b

mik-laj mentioned this pull request May 31, 2020

Links to Stackdriver in WebUI #9083

Closed

	:class:`logging.config.dictConfig`. If your file is not importable, then you should set a :any:``PYTHONPATH`` environment.
	:class:`logging.config.dictConfig`. If your file is not in a standard import location, then you should set a :any:``PYTHONPATH``
	environment. Airflow adds the ``config/`` folder inside ``AIRFLOW_HOME`` to the import path for this purpose.

[AIRFLOW-6065] Add Stackdriver Task Handler #6660

[AIRFLOW-6065] Add Stackdriver Task Handler #6660

Uh oh!

Conversation

mik-laj commented Nov 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jira

Description

Tests

Commits

Documentation

Uh oh!

codecov-io commented Dec 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

potiuk Dec 5, 2019

Choose a reason for hiding this comment

Uh oh!

mik-laj Dec 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ashb Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ashb Dec 10, 2019

Choose a reason for hiding this comment

Uh oh!

mik-laj Jan 4, 2020

Choose a reason for hiding this comment

Uh oh!

ashb Dec 10, 2019

Choose a reason for hiding this comment

Uh oh!

mik-laj Jan 4, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

potiuk commented Feb 10, 2020

Uh oh!

galuszkak commented Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mik-laj commented Feb 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

galuszkak commented Feb 20, 2020

Uh oh!

mik-laj commented Feb 20, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mik-laj commented Nov 25, 2019 •

edited

Loading

codecov-io commented Dec 4, 2019 •

edited

Loading

mik-laj Dec 5, 2019 •

edited

Loading

ashb Dec 10, 2019 •

edited

Loading

galuszkak commented Feb 13, 2020 •

edited

Loading

mik-laj commented Feb 13, 2020 •

edited

Loading