Re: [PR] Re-enable dag_processing.last_duration metric [airflow]

2026-01-26 Thread via GitHub 


potiuk commented on PR #60363:
URL: https://github.com/apache/airflow/pull/60363#issuecomment-3800566602

   I am closing all your PRs @Arunodoy18 - ydespite earlier warnings, you are 
not looking at your PRs, submit multiple unrelated, AI generated changes 
without even looking at them. If this continues to happen, we will ask ASF 
infra to block your user for any ASF contributions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] Re-enable dag_processing.last_duration metric [airflow]

2026-01-26 Thread via GitHub 


potiuk closed pull request #60363: Re-enable dag_processing.last_duration metric
URL: https://github.com/apache/airflow/pull/60363


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] Re-enable dag_processing.last_duration metric [airflow]

2026-01-11 Thread via GitHub 


Arunodoy18 commented on PR #60363:
URL: https://github.com/apache/airflow/pull/60363#issuecomment-3734353711

   Yeaah sure , Apologising for the blunder , I will correct it within some
   time.
   
   On Sun, Jan 11, 2026, 3:36 PM Yeonguk Choo ***@***.***> wrote:
   
   > *choo121600* left a comment (apache/airflow#60363)
   > 
   >
   > It looks like this PR also includes commits addressing a different issue.
   > Could we split those changes into a separate PR to keep the scope clear?
   >
   > —
   > Reply to this email directly, view it on GitHub
   > ,
   > or unsubscribe
   > 

   > .
   > You are receiving this because you authored the thread.Message ID:
   > ***@***.***>
   >
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] Re-enable dag_processing.last_duration metric [airflow]

2026-01-11 Thread via GitHub 


choo121600 commented on PR #60363:
URL: https://github.com/apache/airflow/pull/60363#issuecomment-3734346164

   It looks like this PR also includes commits addressing a different issue.
   Could we split those changes into a separate PR to keep the scope clear?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] Re-enable dag_processing.last_duration metric [airflow]

2026-01-10 Thread via GitHub 


Arunodoy18 commented on code in PR #60363:
URL: https://github.com/apache/airflow/pull/60363#discussion_r2679271719


##
airflow-core/newsfragments/60325.bugfix.rst:
##


Review Comment:
   Thanks for the feedback, @choo121600.
   
   I've removed the unnecessary 60325.bugfix.rst newsfragment.
   
   I also found and removed a duplicate StatsD metric for 
[dag_processing.last_duration](vscode-file://vscode-app/c:/Users/aruno/AppData/Local/Programs/Microsoft%20VS%20Code/resources/app/out/vs/code/electron-browser/workbench/workbench.html)
 which was likely causing the CI failure.
   
   The CI should be passing now. Let me know if there is anything else.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Re: [PR] Re-enable dag_processing.last_duration metric [airflow]

2026-01-10 Thread via GitHub 


choo121600 commented on code in PR #60363:
URL: https://github.com/apache/airflow/pull/60363#discussion_r2678599749


##
airflow-core/newsfragments/60325.bugfix.rst:
##


Review Comment:
   It looks like the CI is failing.
   Also, this PR seems to include some unnecessary changes. Could you please 
take a look?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

[PR] Re-enable dag_processing.last_duration metric [airflow]

2026-01-09 Thread via GitHub 


Arunodoy18 opened a new pull request, #60363:
URL: https://github.com/apache/airflow/pull/60363

   # Re-enable dag_processing.last_duration metric
   
   Closes #60325
   
   ## Description
   
   This PR fixes the missing `dag_processing.last_duration.` metric 
that was accidentally disabled in Airflow 3.0. The metric is still documented 
but was not being emitted due to commented-out code with a "TODO: AIP-66 emit 
metrics" comment.
   
   The `dag_processing.last_run.seconds_ago.` metric continued to 
work fine, making this an inconsistency in metric emission.
   
   ## Root Cause
   
   In 
[manager.py](airflow-core/src/airflow/dag_processing/manager.py#L1191-L1192), 
the Stats calls for the `last_duration` metric were commented out:
   
   ```python
   # TODO: AIP-66 emit metrics
   # file_name = Path(dag_file.path).stem
   # Stats.timing(f"dag_processing.last_duration.{file_name}", 
stat.last_duration)
   # Stats.timing("dag_processing.last_duration", stat.last_duration, 
tags={"file_name": file_name})
   ```
   
   The corresponding test in 
[test_manager.py](airflow-core/tests/unit/dag_processing/test_manager.py#L722) 
was also skipped:
   
   ```python
   @pytest.mark.skip("AIP-66: stats are not implemented yet")
   def test_send_file_processing_statsd_timing(...):
   ```
   
   ## Changes
   
   ### Modified Files
   
   1. 
**[airflow-core/src/airflow/dag_processing/manager.py](airflow-core/src/airflow/dag_processing/manager.py)**:
  - Uncommented the `Stats.timing()` calls for 
`dag_processing.last_duration` metric
  - Updated to use the `relative_fileloc` parameter (already available in 
the function signature)
  - Added proper null checking for safety
   
   2. 
**[airflow-core/tests/unit/dag_processing/test_manager.py](airflow-core/tests/unit/dag_processing/test_manager.py)**:
  - Removed the `@pytest.mark.skip` decorator from 
`test_send_file_processing_statsd_timing`
  - Test now validates the metric is correctly emitted
   
   3. 
**[airflow-core/newsfragments/60325.bugfix.rst](airflow-core/newsfragments/60325.bugfix.rst)**:
  - Added changelog entry for this bugfix
   
   ## Testing Performed
   
   ### 1. Code Validation
   ```powershell
   python -m py_compile airflow-core\src\airflow\dag_processing\manager.py
   # Result: No syntax errors
   ```
   
   ### 2. Unit Test
   The previously skipped test `test_send_file_processing_statsd_timing` is now 
enabled and should pass. This test verifies:
   - The metric `dag_processing.last_duration.` is emitted
   - The tagged version `dag_processing.last_duration` with `tags={"file_name": 
...}` is emitted
   - Both metrics use the correct `last_duration` value
   
   ### 3. Expected Metrics After Fix
   
   When DAG files are parsed, these metrics will be emitted to 
StatsD/CloudWatch/Grafana:
   
   ```
   dag_processing.last_duration.: 
   dag_processing.last_duration (with tags: {file_name: }): 

   ```
   
   For example, for a file `example_dag.py` that took 1.5 seconds to parse:
   ```
   dag_processing.last_duration.example_dag: 1.5
   dag_processing.last_duration{file_name="example_dag"}: 1.5
   ```
   
   ## Impact
   
   - **Users**: Can now monitor DAG parsing performance per-file in MWAA 
CloudWatch and other metric backends
   - **Breaking Changes**: None - this re-enables a previously documented metric
   - **Performance**: Negligible - just emitting metrics that should have been 
there
   
   ## Verification in MWAA/CloudWatch
   
   After deploying this fix to MWAA 3.x:
   1. Upload a DAG file to your S3 bucket
   2. Wait for DAG parsing to occur
   3. Check CloudWatch metrics under namespace `AmazonMWAA`
   4. You should now see `dag_processing.last_duration.` metrics
   
   ## Additional Notes
   
   - The fix is minimal and surgical - only uncomments existing code
   - The test was already written and validates the expected behavior
   - Similar pattern to the working `dag_processing.last_run.seconds_ago` metric
   - Documentation already lists this metric as available


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]