Skip to content

Conversation

@grzuy
Copy link
Contributor

@grzuy grzuy commented Oct 31, 2025

As stated in #436 (comment) there are quite a bit of pending needed improvements for opentelemetry_oban.

Trying to help with that in going in small, easy to review steps.

First would be to make opentelemetry_oban tests green I guess, so that follow PRs can be expected to continue having green checks.

Just that here.

@grzuy
Copy link
Contributor Author

grzuy commented Oct 31, 2025

Not sure why "Opentelemetry Oban" job was skipped...

@grzuy
Copy link
Contributor Author

grzuy commented Oct 31, 2025

maybe because it was skipped before the labeler was able to label the PR?

@grzuy
Copy link
Contributor Author

grzuy commented Oct 31, 2025

I can't hit "Re-run" button for the checks. Maybe someone else can 🙏

@tsloughter
Copy link
Member

I don't know why they aren't running.. the labels look right https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/.github/workflows/elixir.yml#L354

@grzuy
Copy link
Contributor Author

grzuy commented Nov 3, 2025

I don't know why they aren't running.. the labels look right https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/.github/workflows/elixir.yml#L354

It seems the if condition is working when the target branch is from this upstream repo (open-telemetry), but not for pull requests from forked repos?

Upstream repo PRs:

Forked repo PRs:

@grzuy
Copy link
Contributor Author

grzuy commented Nov 3, 2025

Nudging GitHub with a new commit

$ git commit --allow-empty -m "empty commit 1"
$ git push

did the trick ✔️

image

I guess it made GitHub re-evaluate those if conditions with the labels present?
Not sure.

@tsloughter
Copy link
Member

Weird... I reran the test manually after you commented. That is frustrating if rerunning them doesn't reevaluate the rules, that means there is no way but to push new commits...

@grzuy
Copy link
Contributor Author

grzuy commented Nov 3, 2025

I don't know why they aren't running.. the labels look right https://github.com/open-telemetry/opentelemetry-erlang-contrib/blob/main/.github/workflows/elixir.yml#L354

It seems the if condition is working when the target branch is from this upstream repo (open-telemetry), but not for pull requests from forked repos?

Upstream repo PRs:

* [chore(deps): update dependency finch to ~> 0.20 #547](https://github.com/open-telemetry/opentelemetry-erlang-contrib/pull/547) did run opentelemetry_finch tests

* [chore(deps): update dependency opentelemetry_bandit to ~> 0.3.0 #548](https://github.com/open-telemetry/opentelemetry-erlang-contrib/pull/548) did run opentelemetry_bandit tests

Forked repo PRs:

* [feat(phoenix): trace LiveView `render` #529](https://github.com/open-telemetry/opentelemetry-erlang-contrib/pull/529) did NOT run opentelemetry_phoenix tests

* [Widen allowed semantic_conventions for tesla and oban #514](https://github.com/open-telemetry/opentelemetry-erlang-contrib/pull/514) did NOT run opentelemetry_oban tests

* [Update semantic conventions in opentelemetry_oban #528](https://github.com/open-telemetry/opentelemetry-erlang-contrib/pull/528) DID run opentelemetry_oban tests (not sure why in this case - it seems the 2nd commit is after github was able to label the PR, maybe worked because of that?)

Scartch all my speculation here about upstream vs fork repo.

It seems it is because of the chronological order of the commit vs the bot adding the labels.

In the case of renovate PRs I linked to it worked because the last commit is after the labels were set.

@grzuy
Copy link
Contributor Author

grzuy commented Nov 3, 2025

Weird... I reran the test manually after you commented. That is frustrating if rerunning them doesn't reevaluate the rules, that means there is no way but to push new commits...

Apparently a known limitation of reading labels from the github.event that triggered the first attempt: https://github.com/orgs/community/discussions/39062.
Bummer.

@grzuy
Copy link
Contributor Author

grzuy commented Nov 3, 2025

As far as this oban test fix goes, it's ready and apparently with green tests.

The label issue broader issue affecting any PR for any instrumentation library I guess.

@tsloughter
Copy link
Member

Thanks! And will merge this.

@tsloughter tsloughter enabled auto-merge November 4, 2025 11:05
@grzuy
Copy link
Contributor Author

grzuy commented Nov 4, 2025

That is frustrating if rerunning them doesn't reevaluate the rules, that means there is no way but to push new commits...

I think the problem is that the workflow event we are "re-running" is the pull_request.opened event.
The problem seems to be that the pull_request.labeled event, which is already listed in .github/workflows/elixir.yml event is NOT triggering.

It seems the cause could be the .config/workflows/labeler.yml using the GITHUB_TOKEN

repo-token: "${{ secrets.GITHUB_TOKEN }}"

to create the labels on the pull requests is not triggering the pull_request.labeled event in .github/workflows/elixir.yml

types: [opened, reopened, synchronize, labeled]
.

It needs to be a github app token or a personal access token apparently.

sources:

@grzuy
Copy link
Contributor Author

grzuy commented Nov 4, 2025

Thanks! And will merge this.

Thanks.

I think an approval still pending for auto-merge to kick in.

@tsloughter tsloughter added this pull request to the merge queue Nov 4, 2025
Merged via the queue into open-telemetry:main with commit b22c6cc Nov 4, 2025
28 checks passed
@grzuy grzuy deleted the fix-oban-test branch November 4, 2025 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants