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

[Airflow] Revert metrics field definition to the format used before i… #7469

Merged
merged 3 commits into from
Aug 22, 2023
Merged
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
5 changes: 5 additions & 0 deletions packages/airflow/changelog.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
# newer versions go on top
- version: "0.3.0"
changes:
- description: Revert metrics field definition to the format used before introducing metric_type.
type: enhancement
link: https://github.com/elastic/integrations/pull/7469
- version: "0.2.0"
changes:
- description: Add metric_type mapping for the fields of `statsd` datastream.
Expand Down
8 changes: 6 additions & 2 deletions packages/airflow/data_stream/statsd/fields/fields.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
type: group
fields:
- name: '*.count'
type: double
type: object
object_type: double
object_type_mapping_type: "*"
metric_type: counter
description: Airflow counters
- name: '*.max'
Expand Down Expand Up @@ -36,7 +38,9 @@
object_type_mapping_type: "*"
description: Airflow standard deviation timers metric
- name: '*.value'
type: double
type: object
object_type: double
object_type_mapping_type: "*"
metric_type: gauge
description: Airflow gauges
- name: 'dag_file'
Expand Down
4 changes: 2 additions & 2 deletions packages/airflow/docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@ statsd_prefix =
|---|---|---|---|
| @timestamp | Event timestamp. | date | |
| agent.id | | keyword | |
| airflow.\*.count | Airflow counters | double | counter |
| airflow.\*.count | Airflow counters | object | counter |
| airflow.\*.max | Airflow max timers metric | object | |
| airflow.\*.mean | Airflow mean timers metric | object | |
| airflow.\*.mean_rate | Airflow mean rate timers metric | object | |
| airflow.\*.median | Airflow median timers metric | object | |
| airflow.\*.min | Airflow min timers metric | object | |
| airflow.\*.stddev | Airflow standard deviation timers metric | object | |
| airflow.\*.value | Airflow gauges | double | gauge |
| airflow.\*.value | Airflow gauges | object | gauge |
| airflow.dag_file | Airflow dag file metadata | keyword | |
| airflow.dag_id | Airflow dag id metadata | keyword | |
| airflow.job_name | Airflow job name metadata | keyword | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,7 +435,7 @@
"dataType": "number",
"isBucketed": false,
"label": "Scheduler Heartbeat",
"operationType": "sum",
"operationType": "max",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ritalwar, Was this an aggregation change to fix some issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was intended for the dashboard, as the sum aggregator isn't compatible with counters.

Copy link
Contributor

Choose a reason for hiding this comment

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

avg() replaced by max() is understandable. But, sum() replaced by a max(), might change what value is shown by the dashboard panel. Can you re-assess the correctness of the dashboard value following this change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please create an issue for tracking purpose, if needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree, we can summarise the change in a top level issue (existing Airlfow TSDB issue also works)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created this issue.

"params": {
"emptyAsNull": true,
"format": {
Expand Down
2 changes: 1 addition & 1 deletion packages/airflow/manifest.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name: airflow
title: Airflow
version: "0.2.0"
version: "0.3.0"
description: Airflow Integration.
type: integration
format_version: 1.0.0
Expand Down