Skip to content

[AIRFLOW-1325] Use Elasticsearch as logging backend - #2380

Closed
allisonwang wants to merge 1 commit into
apache:masterfrom
allisonwang:allison--es
Closed

[AIRFLOW-1325] Use Elasticsearch as logging backend#2380
allisonwang wants to merge 1 commit into
apache:masterfrom
allisonwang:allison--es

Conversation

@allisonwang

@allisonwang allisonwang commented Jun 19, 2017

Copy link
Copy Markdown
Contributor

Dear Airflow maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

  • My PR addresses the following Airflow 1325 issues and references them in the PR title.

Description

  • Here are some details about my PR, including screenshots of any UI changes:
    Please refer to Airflow 1325 for more details.

This PR depends on #2383

This PR is made the following changes:

Frontend

  • ti_logs.html This adds JS to fetch from flask endpoint get_log.

Flask Endpoint

  • get_log The endpoint for fetching logs from logging backend.

Logging Backend (Most Important)

  • airflow/logging_backends folder. The general idea here is to dynamically instantiate logging backend class based on user configuration logging_backend_url. The current logic with S3/GCS won't be changed if logging_backend_url is not configured.
    • BaseLoggingBackend: an interface exposed to query logs. ElasticsearchLoggingBackend is an implementation of this base class.
    • imports.py: a util file to get class reference by name.
    • logging_backend.py: it acts like cached_app except it returns a cached logging backend.

Test

  • test_elasticsearch_logging_backend.py: A ES integration test.
  • test_elasticsearch.sh: A script to run integration test with Elasticsearch logging backend using Docker. This is not added to Travis-CI because Elasticesarch requires > 30s to install, initialize and execute the test.
  • test_logging_backend.py: Test getting logging backend correctly.
  • test_imports.py: Test importing class by its full name.

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
  • Unit test for logging_backend.py and imports.py
  • Integration test for ElasticsearchLogginBackend using Docker.

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
    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"

@aoen @amaliujia

@mention-bot

Copy link
Copy Markdown

@allisonwang, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bolkedebruin, @mistercrunch and @aoen to be potential reviewers.

@allisonwang
allisonwang force-pushed the allison--es branch 5 times, most recently from 7f6e554 to 4d152b4 Compare June 21, 2017 05:25
@codecov-io

codecov-io commented Jun 21, 2017

Copy link
Copy Markdown

Codecov Report

Merging #2380 into master will decrease coverage by 0.43%.
The diff coverage is 35.38%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2380      +/-   ##
==========================================
- Coverage   69.26%   68.83%   -0.44%     
==========================================
  Files         146      150       +4     
  Lines       11231    11354     +123     
==========================================
+ Hits         7779     7815      +36     
- Misses       3452     3539      +87
Impacted Files Coverage Δ
.../logging_backends/elasticsearch_logging_backend.py 0% <0%> (ø)
airflow/www/views.py 67.12% <43.47%> (-0.5%) ⬇️
airflow/bin/cli.py 51.7% <50%> (ø) ⬆️
airflow/logging_backends/base_logging_backend.py 55% <55%> (ø)
airflow/logging_backend.py 66.66% <66.66%> (ø)
airflow/utils/imports.py 88.88% <88.88%> (ø)
airflow/jobs.py 74.87% <0%> (-0.41%) ⬇️
... and 2 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 e870a8e...e52dc3f. Read the comment docs.

@allisonwang allisonwang changed the title [AIRFLOW-1325][WIP] Use Elasticsearch as logging backend [AIRFLOW-1325] Use Elasticsearch as logging backend Jun 21, 2017
@allisonwang allisonwang changed the title [AIRFLOW-1325] Use Elasticsearch as logging backend [AIRFLOW-1325][WIP] Use Elasticsearch as logging backend Jun 22, 2017
@allisonwang
allisonwang force-pushed the allison--es branch 4 times, most recently from d299b5f to e669621 Compare June 22, 2017 22:13
@allisonwang allisonwang changed the title [AIRFLOW-1325][WIP] Use Elasticsearch as logging backend [AIRFLOW-1325] Use Elasticsearch as logging backend Jun 22, 2017
@allisonwang
allisonwang force-pushed the allison--es branch 8 times, most recently from cbf2fbf to 012538a Compare June 23, 2017 05:13
Comment thread airflow/bin/cli.py Outdated

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.

better to check is not None

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.

UNless it's an empty str

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.

we want logging backend url to be neither None nor empty in this case.

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.

There's probably a better way than sleeping

@@ -0,0 +1,70 @@
{#

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.

Note: this file is WIP and will be slightly changed in the future.

@allisonwang allisonwang changed the title [AIRFLOW-1325] Use Elasticsearch as logging backend [AIRFLOW-1325] Use Elasticsearch as logging backend and add streaming log Jun 28, 2017
@allisonwang allisonwang changed the title [AIRFLOW-1325] Use Elasticsearch as logging backend and add streaming log [AIRFLOW-1325] Use Elasticsearch as logging backend Jun 29, 2017
@allisonwang

Copy link
Copy Markdown
Contributor Author

Close this PR in favor of #2469.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants