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

Fix dag duration metric #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jward-bw
Copy link

@jward-bw jward-bw commented Apr 1, 2020

The filter was causing an implicit cartesian join on TaskInstance in the outer query, making this metric appear multiple times for a single dag. I have moved the filter into the sub-query which is already joined with TaskInstance to prevent this cartesian join.

@abhishekray07 I hope you can take a look at this soon.

@jward-bw
Copy link
Author

jward-bw commented Apr 1, 2020

This PR addresses this issue: #24

@jward-bw jward-bw changed the title DI-1793: Patch prometheus exporter Fix dag duration metric Apr 1, 2020
@mshalak-nix
Copy link

mshalak-nix commented May 7, 2020

+1 for this fix, we have ~28K task instances, and "dag_run_duration" metric gets duplicated 28K times.
@ayush-san @abhishekray07

@drboyer
Copy link

drboyer commented May 28, 2020

Any idea when these changes will be merged and released @ayush-san @abhishekray07?

@ayush-san
Copy link
Contributor

@drboyer I don't have merging rights. Waiting for @abhishekray07 to merge and release these changes

@dusty73
Copy link

dusty73 commented Jul 9, 2020

We're also experiencing performance issues, after a week the metrics endpoint takes something like 20 seconds to respond. When do you think this fix will be merged?

@jward-bw
Copy link
Author

jward-bw commented Jul 9, 2020

The owner of this repo seems to be inactive, so you'd do better just forking and applying this patch yourself.

GBradleyBridge added a commit to oneaudience/airflow-prometheus-exporter that referenced this pull request Jul 14, 2020
Apply fix from robinhood#25 of upstream repository
@azwjp
Copy link

azwjp commented Oct 8, 2020

FYI, issue #26 is also related to this PR.

@jward-bw jward-bw mentioned this pull request Oct 8, 2020
@ring-pete
Copy link

Hello just bumping this PR to see if any upstream maintainers will be able to merge this soon :)

@jonathonbattista
Copy link

For anyone having performance or query response time issues with this exporter, this change will fix those.

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.

8 participants