Skip to content

Add log service addon#5507

Merged
xoxys merged 22 commits into
woodpecker-ci:mainfrom
qwerty287:addon-logs
Oct 21, 2025
Merged

Add log service addon#5507
xoxys merged 22 commits into
woodpecker-ci:mainfrom
qwerty287:addon-logs

Conversation

@qwerty287
Copy link
Copy Markdown
Contributor

@qwerty287 qwerty287 commented Sep 14, 2025

provide a more flexible approach to #5482
Works just like the forge addons.

@qwerty287 qwerty287 added enhancement improve existing features addon labels Sep 14, 2025
@xoxys
Copy link
Copy Markdown
Member

xoxys commented Sep 15, 2025

but is it also possible to load more than one file at a time?

IMO that's a must for an addon system.

@Levy-Tal
Copy link
Copy Markdown
Contributor

Cool, this still doesn’t solve the problem that the addon can’t know if the step is finished from the log chunk or from the *model.Step when the last logAppend is called.
Would it make sense to add a log lifecycle, like logStart or logFinished?
Or do you think it would be better to create a separate pull request for this?

@Levy-Tal
Copy link
Copy Markdown
Contributor

Also, I really like the addon approach!
Just throwing out an idea — this could also be extended to a “secret store”
For example, someone could securely save secrets in Vault, AWS Secrets Manager, etc.

@qwerty287
Copy link
Copy Markdown
Contributor Author

IMO that's a must for an addon system.

The system supports having multiple plugins in one binary. But that would require everyone who uses multiple plugins to compile their own correct "mix" of plugins.

this could also be extended to a “secret store”

For secrets, I'd rather use an http-based approach because then, this can also be added on a repo-level and not only globally.

@qwerty287
Copy link
Copy Markdown
Contributor Author

Cool, this still doesn’t solve the problem that the addon can’t know if the step is finished from the log chunk or from the *model.Step when the last logAppend is called.
Would it make sense to add a log lifecycle, like logStart or logFinished?
Or do you think it would be better to create a separate pull request for this?

Should be done in a separate PR.

My idea would be to add a new function to the interface like StepFinished.

@xoxys
Copy link
Copy Markdown
Member

xoxys commented Sep 15, 2025

The system supports having multiple plugins in one binary. But that would require everyone who uses multiple plugins to compile their own correct "mix" of plugins.

No way around it? That's a pretty bad UX.

@qwerty287
Copy link
Copy Markdown
Contributor Author

No, I don't know. Didn't try it. Maybe (I hope so) it also works with more than one plugin binary.

@anbraten
Copy link
Copy Markdown
Member

No, I don't know. Didn't try it. Maybe (I hope so) it also works with more than one plugin binary.

It should be possible to load multiple binaries, otherwise that would be to difficult for 99% of our users to run such plugins and no-one would really share a helpful plugin, resulting in the feature not being used a lot.

In general I am still not 100% happy with the addon / plugin mechanisms we have atm:

  • HTTP extensions (config service):
    • (+) pretty easy to implement
    • (+) can be easily shared (container image, public hosted api, some binary you ship, ... endless ways to share such a service)
    • (-) hosting them requires additional containers etc or external hosting (needs to be allowed by the admin)
  • Binary addons (as used by the addon forge):
    • (+) can be quite powerful and are probably a bit more performant than http plugins
    • (-) difficult to share as they need to be compiled with same version of the server / architecture
    • (-) need to be installed by admins, can not be installed by users as necessary for Allow to configure a config extension per repo #3349

Some notes of my thoughts on the plugin topic (no concrete ideas / directions yet):

We had a look at https://github.com/knqyf263/go-plugin some time ago, there seems to be sth similar in istio: https://istio.io/latest/docs/reference/config/proxy_extensions/wasm-plugin
There was also the idea to use agents to host containers that would run them like pipeline services that never stop (but that would be quite some journey as well).

@qwerty287
Copy link
Copy Markdown
Contributor Author

(-) difficult to share as they need to be compiled with same version of the server / architecture

No. This affefcts Go's internal plugins system. This one uses RPC and as long as this does not change there should be no incompatibiltiy.

@qwerty287
Copy link
Copy Markdown
Contributor Author

Tested it, it is working with both log and forge addon configured. So no problem there. Also added some more docs.

What we need to think about is where we put the user docs for addons as that's quite an advanced feature. Right now it was below the forges but that doesn't fit anymore after this.

@woodpecker-bot
Copy link
Copy Markdown
Contributor

woodpecker-bot commented Sep 20, 2025

Surge PR preview deployment was removed

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 20, 2025

Codecov Report

❌ Patch coverage is 0% with 148 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.22%. Comparing base (045a222) to head (974bc54).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
server/services/log/addon/client.go 0.00% 67 Missing ⚠️
server/services/log/addon/server.go 0.00% 44 Missing ⚠️
shared/logger/addon_logger.go 0.00% 29 Missing ⚠️
server/services/log/addon/plugin.go 0.00% 4 Missing ⚠️
cmd/server/setup.go 0.00% 2 Missing ⚠️
server/forge/addon/client.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5507      +/-   ##
==========================================
- Coverage   21.30%   21.22%   -0.08%     
==========================================
  Files         420      423       +3     
  Lines       38163    38280     +117     
==========================================
- Hits         8130     8125       -5     
- Misses      29280    29402     +122     
  Partials      753      753              

☔ 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.

This was referenced Sep 20, 2025
@xoxys
Copy link
Copy Markdown
Member

xoxys commented Sep 24, 2025

What we need to think about is where we put the user docs for addons as that's quite an advanced feature. Right now it was below the forges but that doesn't fit anymore after this.

I would move it to Configuration > Addons

@qwerty287 qwerty287 requested a review from a team September 25, 2025 16:05
Copy link
Copy Markdown
Member

@xoxys xoxys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nits, the rest LGTM

Comment thread docs/docs/30-administration/10-configuration/10-server.md Outdated
Comment thread docs/docs/92-development/100-addons.md Outdated
Comment thread server/services/log/addon/client.go
qwerty287 and others added 3 commits October 12, 2025 09:09
@xoxys xoxys enabled auto-merge (squash) October 13, 2025 06:05
@xoxys xoxys merged commit 1019d85 into woodpecker-ci:main Oct 21, 2025
7 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Oct 21, 2025
1 task
@qwerty287 qwerty287 deleted the addon-logs branch October 21, 2025 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

addon enhancement improve existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants