Skip to content

Conversation

@xBis7
Copy link
Contributor

@xBis7 xBis7 commented Apr 13, 2025

This is a follow up PR for #43941. It's also related to #43789.

The patch is composed by the following changes

  1. The active_spans dict has been using the ti.key as a key for task instance spans. ti.key will be replaced by ti.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 to try_id but it was removed by PR Ensure that TI.id is unique per try. #48749 in favor of id.

  2. The previous PR added some integration tests which aren't running on the CI. The patch fixes that. The tests are using the redis integration. I've verified that the providers tests with the redis integration, don't run on this new CI step.

  3. 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.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@xBis7
Copy link
Contributor Author

xBis7 commented May 7, 2025

The failure for the gremlin provider seems unrelated. The new CI tests passed.

@xBis7
Copy link
Contributor Author

xBis7 commented May 19, 2025

@ashb Can you help get this PR merged?

Copy link
Member

@potiuk potiuk left a 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.

@potiuk
Copy link
Member

potiuk commented Jun 23, 2025

@ashb -> are you ok with this one ?

@xBis7
Copy link
Contributor Author

xBis7 commented Jul 3, 2025

https://github.com/apache/airflow/actions/runs/16028112052/job/45221996545?pr=49180#step:6:2093

The error is

sqlalchemy.exc.OperationalError: (MySQLdb.OperationalError) (2013, 'Lost connection to server during query'

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.

@xBis7
Copy link
Contributor Author

xBis7 commented Jul 3, 2025

Every time, it's a different unrelated CI failure. I'll trigger the workflow once more.

@xBis7
Copy link
Contributor Author

xBis7 commented Jul 3, 2025

@potiuk @ashb Can we get this merged? It would be great to include it in release 3.0.3.

@ashb ashb merged commit 3fa68e6 into apache:main Jul 3, 2025
102 checks passed
@ashb
Copy link
Member

ashb commented Jul 3, 2025

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.

@xBis7
Copy link
Contributor Author

xBis7 commented Jul 3, 2025

@ashb @potiuk Thank you for the reviews and the help!

@ashb ashb modified the milestones: Airflow 3.1+, Airflow 3.1.0 Jul 3, 2025
@potiuk
Copy link
Member

potiuk commented Jul 3, 2025

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.

@xBis7 xBis7 deleted the otel_cleanup branch July 7, 2025 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:DAG-processing area:dev-tools area:Executors-core LocalExecutor & SequentialExecutor area:providers area:Scheduler including HA (high availability) scheduler area:Triggerer provider:edge Edge Executor / Worker (AIP-69) / edge3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants