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

WIP: add configuration management and authentication features #57

Closed
wants to merge 3 commits into from

Conversation

greenpau
Copy link

@greenpau greenpau commented Aug 3, 2019

Resolves: #54, #55, #56

More info:

This commit adds the support for the configuration of the exporter via
prometheus_exporter section of Airflow configuration.

The keys with expose_ prefix enable or disable the exposure of certain class
of metrics. For example, Airflow variables-related metrics are not exposed by
default. However, with expose_variables set to True, the exporter will start
exposing the metrics. Similarly, by default, the exporter exposes scheduler-related
metrics. However, with expose_scheduler set to False, the exporter will
not export them.

Additionally, when expose_config is being enabled, the exporter will expose
a subset of core and prometheus_exporter configuration settings.

[prometheus_exporter]
auth_enabled = True
auth_token = 4e1dba1c-2b66-4275-b8ae-292ee9665fa1
expose_variables = True
expose_config = False
expose_scheduler = False

It is possible to disable the exporter:

[prometheus_exporter]
disabled = True

When authentication is enabled, the metrics are accessible via:

curl -v -H 'Authorization: Bearer 4e1dba1c-2b66-4275-b8ae-292ee9665fa1' https://localhost:8443/admin/metrics/

Also, when the authentication is enabled, Prometheus scrape job
might look like this:

  - job_name: 'airflow_exporters'
    metrics_path: /admin/metrics/
    scheme: https
    tls_config:
      insecure_skip_verify: true
    bearer_token: '4e1dba1c-2b66-4275-b8ae-292ee9665fa1'
    scrape_interval: 5m
    static_configs:
    - targets:
      - '127.0.0.1:8443'
      labels:
        environment: 'dev'
    relabel_configs:
    - source_labels: [__address__]
      regex: "^(.*):(.*)$"
      target_label: instance
      replacement: ${1}
    - source_labels: [instance]
      regex: "^(127.0.0.1)$"
      target_label: instance
      replacement: "airflow"

Resolves: epoch8#54, epoch8#55, epoch8#56

More info:

This commit adds the support for the configuration of the exporter via
`prometheus_exporter` section of Airflow configuration.

The keys with `expose_` prefix enable or disable the exposure of certain class
of metrics. For example, Airflow variables-related metrics are not exposed by
default. However, with `expose_variables` set to `True`, the exporter will start
exposing the metrics. Similarly, by default, the exporter exposes scheduler-related
metrics. However, with `expose_scheduler` set to `False`, the exporter will
not export them.

Additionally, when `expose_config` is being enabled, the exporter will expose
a subset of `core` and `prometheus_exporter` configuration settings.

```
[prometheus_exporter]
auth_enabled = True
auth_token = 4e1dba1c-2b66-4275-b8ae-292ee9665fa1
expose_variables = True
expose_config = False
expose_scheduler = False
```

It is possible to disable the exporter:

```
[prometheus_exporter]
disabled = True
```

When authentication is enabled, the metrics are accessible via:

```bash
curl -v -H 'Authorization: Bearer 4e1dba1c-2b66-4275-b8ae-292ee9665fa1' https://localhost:8443/admin/metrics/
```

Also, when the authentication is enabled, Prometheus scrape job
might look like this:

```yaml
  - job_name: 'airflow_exporters'
    metrics_path: /admin/metrics/
    scheme: https
    tls_config:
      insecure_skip_verify: true
    bearer_token: '4e1dba1c-2b66-4275-b8ae-292ee9665fa1'
    scrape_interval: 5m
    static_configs:
    - targets:
      - '127.0.0.1:8443'
      labels:
        environment: 'dev'
    relabel_configs:
    - source_labels: [__address__]
      regex: "^(.*):(.*)$"
      target_label: instance
      replacement: ${1}
    - source_labels: [instance]
      regex: "^(127.0.0.1)$"
      target_label: instance
      replacement: "airflow"
```
@elephantum
Copy link
Contributor

@greenpau nice job!

Is it ready for review?

@greenpau
Copy link
Author

greenpau commented Aug 6, 2019

@elephantum , not yet. I will ping you when it is done.

@greenpau
Copy link
Author

@elephantum , conceptually, you could try reviewing the exporter.

Also, do you think it is the code or the test itself causing the below?

image

@elephantum
Copy link
Contributor

It is difficult to review this PR because you did so many changes at once + renamed files.

Let’s discuss if it is possible to split this PR into several that are incremental and change only small parts of code at a time.

@greenpau
Copy link
Author

@elephantum, unfortunately, I don’t have much time to work on splitting it. By and large, it has these major changes:

  • introduce exporter configuration
  • introduce token based authentication (and a bypass if existing authenticated session)
  • add scheduler_up metric

Additionally, it:

  • it only runs the plugin for webserver process

@elephantum
Copy link
Contributor

I see. In current state this PR can not be accepted: tests are broken, diff is 100%

We will use it as a reference to fix #54, #55, #56 separately.

@greenpau greenpau closed this Aug 19, 2019
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.

feature: exporter does not require authentication
2 participants