You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "ferruzzi (via GitHub)" <gi...@apache.org> on 2024/06/17 21:36:20 UTC
[PR] Remove "experimental" banner for OTel Metrics [airflow]
ferruzzi opened a new pull request, #40286:
URL: https://github.com/apache/airflow/pull/40286
OpenTelemetry metrics have been live for a while now and there have been no major bugs except the ongoing discussions around timers reporting in seconds or milliseconds ([example](https://github.com/apache/airflow/pull/39908)). I propose we drop the "Experimental" banner for OTel Metrics and leave the new Experimental for OTel Traces.
@howardyoo
@potiuk
@romanzdk
@o-nikolas
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove "experimental" banner for OTel Metrics [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #40286:
URL: https://github.com/apache/airflow/pull/40286#issuecomment-2205854126
> #34405 , is that not a concern ?
>
> airflow that stop working cause otel-collector is down is not similar to what happen with statsd
It's independent from being "experimental" - it's just regular - potential - bug fix that @feruzzi was supposed to follow up on :D
It does require discussion. I think the current behaviour (failure when otel collector is down) is correct. Mostly because if you are using OTEL collector, it becomes your way to monitor some issues and silently skipping over metrics generation is not likely a good idea (if you have permanent issue with your otel collector - you can still disable it in configuration until it is solved, with generally the same result as if we skipped it).
But I think it requires a short discussion and asking what others think, but it's not blocking the experimental status changing (also removal of the banner is not equal with removing experimental status - it's just that the banner was a bad idea in the first place).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove "experimental" banner for OTel Metrics [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on PR #40286:
URL: https://github.com/apache/airflow/pull/40286#issuecomment-2176491601
I'll get the voting thread going and add the news fragment, thanks folks.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove "experimental" banner for OTel Metrics [airflow]
Posted by "raphaelauv (via GitHub)" <gi...@apache.org>.
raphaelauv commented on PR #40286:
URL: https://github.com/apache/airflow/pull/40286#issuecomment-2205748120
@potiuk @ferruzzi
https://github.com/apache/airflow/issues/34405 , is that not a concern ?
airflow that stop working cause otel-collector is down is not similar to what happen with statsd
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove "experimental" banner for OTel Metrics [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #40286:
URL: https://github.com/apache/airflow/pull/40286#discussion_r1643859452
##########
airflow/www/templates/airflow/dags.html:
##########
@@ -119,12 +119,6 @@
<a href={{ get_docs_url("executor/index.html") }} target="_blank" rel="noopener noreferrer"><b>Click here</b></a> for more information.
{% endcall %}
{% endif %}
- {% if otel_metrics_on | default(false) %}
Review Comment:
Add conversation about newsfragment here so that it's not merged accidentally.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove "experimental" banner for OTel Metrics [airflow]
Posted by "utkarsharma2 (via GitHub)" <gi...@apache.org>.
utkarsharma2 commented on PR #40286:
URL: https://github.com/apache/airflow/pull/40286#issuecomment-2205111178
> I think the change could be applied manually - this is a very simple change - we've done that in the past.
Sure, I'll manually apply them.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove "experimental" banner for OTel Metrics [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #40286:
URL: https://github.com/apache/airflow/pull/40286#issuecomment-2203207650
Yeah. I think it would be great to include that one. Really the banner was a mistake - it is enough to describe feature as experimental, showing a banner is a bad idea, because if someone turns it on, they do it deliberately and do not want to see the banner. So we can easily treat that one as "bugfix".
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove "experimental" banner for OTel Metrics [airflow]
Posted by "ferruzzi (via GitHub)" <gi...@apache.org>.
ferruzzi commented on code in PR #40286:
URL: https://github.com/apache/airflow/pull/40286#discussion_r1644947942
##########
airflow/www/templates/airflow/dags.html:
##########
@@ -119,12 +119,6 @@
<a href={{ get_docs_url("executor/index.html") }} target="_blank" rel="noopener noreferrer"><b>Click here</b></a> for more information.
{% endcall %}
{% endif %}
- {% if otel_metrics_on | default(false) %}
Review Comment:
Dev list email sent this morning. :+1:
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org