Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 17 additions & 89 deletions airflow/bin/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import socket
import subprocess
import textwrap
import warnings
from importlib import import_module

import argparse
Expand Down Expand Up @@ -54,8 +53,6 @@

from airflow.ti_deps.dep_context import (DepContext, SCHEDULER_DEPS)
from airflow.utils import db as db_utils
from airflow.utils import logging as logging_utils
from airflow.utils.file import mkdirs
from airflow.www.app import cached_app

from sqlalchemy import func
Expand Down Expand Up @@ -357,61 +354,22 @@ def run(args, dag=None):
ti = TaskInstance(task, args.execution_date)
ti.refresh_from_db()

logging.root.handlers = []
logger = logging.getLogger('airflow.task')
if args.raw:
# Output to STDOUT for the parent process to read and log
logging.basicConfig(
stream=sys.stdout,
level=settings.LOGGING_LEVEL,
format=settings.LOG_FORMAT)
else:
# Setting up logging to a file.

# To handle log writing when tasks are impersonated, the log files need to
# be writable by the user that runs the Airflow command and the user
# that is impersonated. This is mainly to handle corner cases with the
# SubDagOperator. When the SubDagOperator is run, all of the operators
# run under the impersonated user and create appropriate log files
# as the impersonated user. However, if the user manually runs tasks
# of the SubDagOperator through the UI, then the log files are created
# by the user that runs the Airflow command. For example, the Airflow
# run command may be run by the `airflow_sudoable` user, but the Airflow
# tasks may be run by the `airflow` user. If the log files are not
# writable by both users, then it's possible that re-running a task
# via the UI (or vice versa) results in a permission error as the task
# tries to write to a log file created by the other user.
try_number = ti.try_number
log_base = os.path.expanduser(conf.get('core', 'BASE_LOG_FOLDER'))
log_relative_dir = logging_utils.get_log_directory(args.dag_id, args.task_id,
args.execution_date)
directory = os.path.join(log_base, log_relative_dir)
# Create the log file and give it group writable permissions
# TODO(aoen): Make log dirs and logs globally readable for now since the SubDag
# operator is not compatible with impersonation (e.g. if a Celery executor is used
# for a SubDag operator and the SubDag operator has a different owner than the
# parent DAG)
if not os.path.isdir(directory):
# Create the directory as globally writable using custom mkdirs
# as os.makedirs doesn't set mode properly.
mkdirs(directory, 0o775)
log_relative = logging_utils.get_log_filename(
args.dag_id, args.task_id, args.execution_date, try_number)
filename = os.path.join(log_base, log_relative)

if not os.path.exists(filename):
open(filename, "a").close()
os.chmod(filename, 0o666)

logging.basicConfig(
filename=filename,
level=settings.LOGGING_LEVEL,
format=settings.LOG_FORMAT)
logger = logging.getLogger('airflow.task.raw')

for handler in logger.handlers:
try:
handler.set_context(ti)
except AttributeError:
# Not all handlers need to have context passed in so we ignore
# the error when handlers do not have set_context defined.
pass

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's use hasattr instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's more explicit

@allisonwang allisonwang Jul 26, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

https://hynek.me/articles/hasattr This article points out using hasattr in Python 2 can be harmful since it shadows errors in properties. In this case it's probably fine for now to use hasattr but it doesn't seem to be a good practice to use in Python 2.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch. Feel free to leave as is


hostname = socket.getfqdn()
logging.info("Running on host {}".format(hostname))

if args.local:
print("Logging into: " + filename)
run_job = jobs.LocalTaskJob(
task_instance=ti,
mark_success=args.mark_success,
Expand Down Expand Up @@ -469,43 +427,13 @@ def run(args, dag=None):
if args.raw:
return

# Force the log to flush, and set the handler to go back to normal so we
# don't continue logging to the task's log file. The flush is important
# because we subsequently read from the log to insert into S3 or Google
# cloud storage.
logging.root.handlers[0].flush()
logging.root.handlers = []

# store logs remotely
remote_base = conf.get('core', 'REMOTE_BASE_LOG_FOLDER')

# deprecated as of March 2016
if not remote_base and conf.get('core', 'S3_LOG_FOLDER'):
warnings.warn(
'The S3_LOG_FOLDER conf key has been replaced by '
'REMOTE_BASE_LOG_FOLDER. Your conf still works but please '
'update airflow.cfg to ensure future compatibility.',
DeprecationWarning)
remote_base = conf.get('core', 'S3_LOG_FOLDER')

if os.path.exists(filename):
# read log and remove old logs to get just the latest additions

with open(filename, 'r') as logfile:
log = logfile.read()

remote_log_location = os.path.join(remote_base, log_relative)
logging.debug("Uploading to remote log location {}".format(remote_log_location))
# S3
if remote_base.startswith('s3:/'):
logging_utils.S3Log().write(log, remote_log_location)
# GCS
elif remote_base.startswith('gs:/'):
logging_utils.GCSLog().write(log, remote_log_location)
# Other
elif remote_base and remote_base != 'None':
logging.error(
'Unsupported remote log location: {}'.format(remote_base))
# Force the log to flush. The flush is important because we
# might subsequently read from the log to insert into S3 or
# Google cloud storage. Explicitly close the handler is
# needed in order to upload to remote storage services.
for handler in logger.handlers:
handler.flush()
handler.close()


def task_failed_deps(args):
Expand Down
13 changes: 13 additions & 0 deletions airflow/config_templates/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# -*- coding: utf-8 -*-
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
7 changes: 7 additions & 0 deletions airflow/config_templates/default_airflow.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,13 @@ security =
# values at runtime)
unit_test_mode = False

# User defined logging configuration file path.
logging_config_path =

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why no time pull this from the logging config, can be global (in the dict) but seems to be tight to a Handler

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh this specifies the filename of the user defined logging configuration if they do not want to use the default default_airflow_logging.py. It's like specifying the path for this config file.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha


# Name of handler to read task instance logs.
# Default to use file task handler.
task_log_reader = file.task

[cli]
# In what way should the cli access the API. The LocalClient will use the
# database directly, while the json_client will use the api running on the
Expand Down
94 changes: 94 additions & 0 deletions airflow/config_templates/default_airflow_logging.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# -*- coding: utf-8 -*-
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import os

from airflow import configuration as conf

# TODO: Logging format and level should be configured
# in this file instead of from airflow.cfg. Currently
# there are other log format and level configurations in
# settings.py and cli.py. Please see AIRFLOW-1455.

LOG_LEVEL = conf.get('core', 'LOGGING_LEVEL').upper()
LOG_FORMAT = conf.get('core', 'log_format')

BASE_LOG_FOLDER = conf.get('core', 'BASE_LOG_FOLDER')

# TODO: REMOTE_BASE_LOG_FOLDER should be deprecated and
# directly specify in the handler definitions. This is to
# provide compatibility to older remote log folder settings.
REMOTE_BASE_LOG_FOLDER = conf.get('core', 'REMOTE_BASE_LOG_FOLDER')
S3_LOG_FOLDER = ''
GCS_LOG_FOLDER = ''
if REMOTE_BASE_LOG_FOLDER.startswith('s3:/'):
S3_LOG_FOLDER = REMOTE_BASE_LOG_FOLDER
elif REMOTE_BASE_LOG_FOLDER.startswith('gs:/'):
GCS_LOG_FOLDER = REMOTE_BASE_LOG_FOLDER

FILENAME_TEMPLATE = '{dag_id}/{task_id}/{execution_date}/{try_number}.log'

DEFAULT_LOGGING_CONFIG = {
'version': 1,
'disable_existing_loggers': False,
'formatters': {
'airflow.task': {
'format': LOG_FORMAT,
},
},
'handlers': {
'console': {
'class': 'logging.StreamHandler',
'formatter': 'airflow.task',
'stream': 'ext://sys.stdout'
},
'file.task': {
'class': 'airflow.utils.log.file_task_handler.FileTaskHandler',
'formatter': 'airflow.task',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
'filename_template': FILENAME_TEMPLATE,
},
's3.task': {
'class': 'airflow.utils.log.s3_task_handler.S3TaskHandler',
'formatter': 'airflow.task',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
's3_log_folder': S3_LOG_FOLDER,
'filename_template': FILENAME_TEMPLATE,
},
'gcs.task': {
'class': 'airflow.utils.log.gcs_task_handler.GCSTaskHandler',
'formatter': 'airflow.task',
'base_log_folder': os.path.expanduser(BASE_LOG_FOLDER),
'gcs_log_folder': GCS_LOG_FOLDER,
'filename_template': FILENAME_TEMPLATE,
},
},
'loggers': {
'airflow.task': {
'handlers': ['file.task'],
'level': LOG_LEVEL,
'propagate': False,
},
'airflow.task_runner': {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this not a child of airflow.task? It would not require to repeat the same config

@allisonwang allisonwang Aug 8, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bolkedebruin Task runner in current structure is under module Airflow not airflow.task. It uses a LoggingMixin that gets the corresponding logger with the module name. We can make another folder called under airflow/task and put task runner inside if needed. But we can always reorganize the folder structure later.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok. Leave it for now then

'handlers': ['file.task'],
'level': LOG_LEVEL,
'propagate': True,
},
'airflow.task.raw': {
'handlers': ['console'],
'level': LOG_LEVEL,
'propagate': False,
},
}
}
1 change: 0 additions & 1 deletion airflow/dag/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,3 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
14 changes: 14 additions & 0 deletions airflow/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from __future__ import unicode_literals

import logging
import logging.config
import os
import sys

Expand Down Expand Up @@ -169,6 +170,19 @@ def configure_orm(disable_connection_pool=False):
configure_vars()
configure_orm()

# TODO: Unify airflow logging setups. Please see AIRFLOW-1457.
logging_config_path = conf.get('core', 'logging_config_path')
try:
from logging_config_path import LOGGING_CONFIG
logging.debug("Successfully imported user-defined logging config.")
except Exception as e:
# Import default logging configurations.
logging.debug("Unable to load custom logging config file: {}."
" Using default airflow logging config instead".format(str(e)))
from airflow.config_templates.default_airflow_logging import \
DEFAULT_LOGGING_CONFIG as LOGGING_CONFIG
logging.config.dictConfig(LOGGING_CONFIG)

# Const stuff

KILOBYTE = 1024
Expand Down
2 changes: 2 additions & 0 deletions airflow/task_runner/base_task_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ def __init__(self, local_task_job):
associated task instance.
:type local_task_job: airflow.jobs.LocalTaskJob
"""
# Pass task instance context into log handlers to setup the logger.
self._task_instance = local_task_job.task_instance
self.set_logger_contexts(self._task_instance)

popen_prepend = []
cfg_path = None
Expand Down
13 changes: 13 additions & 0 deletions airflow/utils/log/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# -*- coding: utf-8 -*-
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
Loading