Skip to content

Move wait for log uploads logic out of logger and tracer into pipeline runtime#6471

Merged
6543 merged 20 commits into
woodpecker-ci:mainfrom
6543-forks:refactor-pipeline-uploadWait
Apr 25, 2026
Merged

Move wait for log uploads logic out of logger and tracer into pipeline runtime#6471
6543 merged 20 commits into
woodpecker-ci:mainfrom
6543-forks:refactor-pipeline-uploadWait

Conversation

@6543
Copy link
Copy Markdown
Member

@6543 6543 commented Apr 19, 2026

fix https://ci.woodpecker-ci.org/repos/3780/pipeline/33635/39 as the dummy backend just returns to fast and so sometimes the uploads wait group was just to slow, now we remove the wait from the logger and tracer implementation into the runtime where it belongs

PS: this race also ocures in real world usage, where the agent operates on ram disks

this is also the successor to #6263

@6543 6543 added the refactor delete or replace old code label Apr 19, 2026
@6543 6543 changed the title move upload waitgroup logic out of logger and tracer into pipeline runtime Move wait for log uploads logic out of logger and tracer into pipeline runtime Apr 19, 2026
@6543 6543 added enhancement improve existing features bug Something isn't working and removed enhancement improve existing features labels Apr 19, 2026
Comment thread agent/runner.go
@6543 6543 added the wip label Apr 19, 2026
@6543 6543 marked this pull request as draft April 19, 2026 23:22
@6543

This comment was marked as resolved.

@6543 6543 mentioned this pull request Apr 19, 2026
1 task
@6543 6543 marked this pull request as ready for review April 19, 2026 23:43
Comment thread pipeline/runtime/step.go
@6543 6543 removed the wip label Apr 19, 2026
@6543
Copy link
Copy Markdown
Member Author

6543 commented Apr 19, 2026

we get rid of all the hacks more and more 🎉

Comment thread pipeline/runtime/step_test.go
@6543 6543 mentioned this pull request Apr 20, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 20, 2026

Codecov Report

❌ Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.19%. Comparing base (8e6084c) to head (7817902).

Files with missing lines Patch % Lines
pipeline/runtime/step.go 80.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6471      +/-   ##
==========================================
- Coverage   41.25%   41.19%   -0.06%     
==========================================
  Files         431      431              
  Lines       28792    28779      -13     
==========================================
- Hits        11878    11856      -22     
- Misses      15847    15853       +6     
- Partials     1067     1070       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@6543
Copy link
Copy Markdown
Member Author

6543 commented Apr 20, 2026

this is also the successor to #6263

@6543
Copy link
Copy Markdown
Member Author

6543 commented Apr 20, 2026

PS: if you want to verify non flakyness, just runn this on main and this pull head:

go test -count=300 -race -tags test -coverprofile=coverage.out ./e2e/...
go test -count=300 -race -tags test -coverprofile=coverage.out ./pipeline/runtime/...

(run can take a few minutes...(depending on how insane you go with count))

@6543 6543 requested a review from a team April 20, 2026 00:29
@6543 6543 mentioned this pull request Apr 20, 2026
16 tasks
@6543
Copy link
Copy Markdown
Member Author

6543 commented Apr 23, 2026

@6543 6543 force-pushed the refactor-pipeline-uploadWait branch from 8c5b01a to 8df3d7b Compare April 23, 2026 09:22
@woodpecker-bot
Copy link
Copy Markdown
Contributor

woodpecker-bot commented Apr 23, 2026

Surge PR preview deployment was removed

@6543 6543 mentioned this pull request Apr 23, 2026
9 tasks
@6543 6543 merged commit 4dd3be7 into woodpecker-ci:main Apr 25, 2026
6 of 7 checks passed
@6543 6543 deleted the refactor-pipeline-uploadWait branch April 25, 2026 14:36
@woodpecker-bot woodpecker-bot mentioned this pull request Apr 25, 2026
1 task
@6543
Copy link
Copy Markdown
Member Author

6543 commented Apr 26, 2026

sorry i tested this as hard as i could .. by running manual real pipelines in local and docker backend, rerun tests suite a lot and catch all imposible bugs etc ... still I should have just tested "services" in a real pipeline. i did not :/

-> #6507

this here make sure traces are uploaded before we return, but we depend on return to fully stop the workflow ATM ...

the mentioned pull makes workflow shutdown explizite and still has defer for cases we have an error etc...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working refactor delete or replace old code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants