-
Notifications
You must be signed in to change notification settings - Fork 16.4k
OpenTelemetry traces implementation cleanup #49180
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
Conversation
…orated function spans
|
The failure for the |
|
@ashb Can you help get this PR merged? |
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit - one comment, but it looks good.
|
@ashb -> are you ok with this one ? |
|
https://github.com/apache/airflow/actions/runs/16028112052/job/45221996545?pr=49180#step:6:2093 The error is The test passed for postgres and failed for mysql. I ran it locally for both backends and it works. It must have been an unfortunate moment where there was indeed a connection loss to the db causing a temporary error. I triggered the CI again. |
|
Every time, it's a different unrelated CI failure. I'll trigger the workflow once more. |
|
I think we've missed the cut off for 3.0.3, sorry. More generally I'm not sure this counts as a bugfix anyway, it is more of a new feature I think so 3.1.0 in a month or two is usually when we'd release this. |
Agree. This is more of "make it easier to maintain code" rather than bugfix - and except avoiding difficult to resolve conflicts (which I do not see as a big risk here) there is no particular reason to cherry-pick that one. |
This is a follow up PR for #43941. It's also related to #43789.
The patch is composed by the following changes
The
active_spansdict has been using theti.keyas a key for task instance spans.ti.keywill be replaced byti.id. The change is based on this comment Provide an alternative OpenTelemetry implementation for traces that follows standard otel practices #43941 (comment). The comment is referring totry_idbut it was removed by PR Ensure that TI.id is unique per try. #48749 in favor ofid.The previous PR added some integration tests which aren't running on the CI. The patch fixes that. The tests are using the
redisintegration. I've verified that the providers tests with theredisintegration, don't run on this new CI step.We are generating spans for a bunch of airflow's internal methods. The information that comes from the spans, might be relevant to developers but not to end users. Added a config flag to turn on/off the traces.
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.