[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-07-13 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1635171569 @conorbev "plans" may be a strong word, but I do intend to do more work on the OTel stuff as I get time. The reason they still have the embedded values is because not all metrics

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-17 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1511761292 @uranusjr @vincbeck @howardyoo - Are we good? Can I get a review/approval on this one when you get time? I think I've covered all comments. -- This is an automated message from th

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-13 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1507566256 Looks like it is all passing. Going to rebase from main, run it all again locally to make sure that doesn't break anything, then I think I've addressed any current concerns. -- Thi

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-13 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1507478512 Alright, the last two commits should remove the email and run_id tags and update the unit tests accordingly. -- This is an automated message from the Apache Git Service. To respond t

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-12 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1506234387 Sorry for the churn. Post-vacation brain melt, I guess :P All tests are currently passing. Still pending discussion/decisions: 1) Do we want emails in the tags (discuss

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-12 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1505698125 In [this test](https://github.com/apache/airflow/blob/main/tests/models/test_dagrun.py#L891) we are specifically checking to make sure that there are no tags in the `dagrun.{dag.dag_i

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1504650113 Alright, a whole slew of new ones failing not that that one is out of the way. I have to update a handful of unit tests that were checking for called_with(). I'll worry about that to

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1504526453 > Nice, the easiest of fixes! smiley And among the more embarrassing ones :P Running static checks and double-checking it was only the one place I made that mistake and I'll have

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1504522661 It's a typo. stats_tags vs stat_tags -- 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 t

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1504519841 Nah, I'm just a dough-head and linked the wrong place. Fixed the link. It's in both places. -- This is an automated message from the Apache Git Service. To respond to the message,

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1504509474 @o-nikolas mentioned that it was likely because the stats_tags wasn't be stored in the db, so I thought making it a `property` and assembling it there was a clever solution. I may hav

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1504460915 Rebased and made a handful of adjustments based on feedback above. Still pending a decision on including the email address(es); I'll adjust the test and/or the method accordingly onc

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1503885277 > Congrats! That must have been a celerbration 15 years! I can't believe anyone would put up with me that long! -- This is an automated message from the Apache Git Service. T

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-11 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1503851784 Sorry for the delay, I was out of town celebrating my anniversary :PI will look at the failing tests and get to the comments today. Thanks for your patience! -- This is an auto

[GitHub] [airflow] ferruzzi commented on pull request #30496: [OTel Integration] Add tagging to existing stats

2023-04-05 Thread via GitHub
ferruzzi commented on PR #30496: URL: https://github.com/apache/airflow/pull/30496#issuecomment-1498376028 Interesting. I'll fix the tests -- 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 spe