Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Rework log streaming and related functions #1802

Merged
merged 49 commits into from
Jun 6, 2023
Merged

Rework log streaming and related functions #1802

merged 49 commits into from
Jun 6, 2023

Conversation

anbraten
Copy link
Member

@anbraten anbraten commented Jun 2, 2023 •
edited by 6543
Loading

closes #1801
closes #1815
closes #1144
closes #983
closes #1322
closes #374
closes #557
closes #1827
fix a regression of #1791 too

TODO

  • adjust log model
  • add migration for logs
  • send log line via grpc using step-id
  • save log-line to db
  • stream log-lines to UI
  • use less structs for log-data
  • make web UI work
    • display logs loaded from db
    • display streaming logs
  • make migration work -> dedicated pull (Migrate old logs to new database schema #1828)

TESTED

  • new logs are stored in database
  • log retrieval via cli (of new logs) works
  • log streaming works (tested via curl & webui)
  • log retrieval via web (of new logs) works

@anbraten anbraten marked this pull request as draft June 2, 2023 05:31
@anbraten
Copy link
Member Author

anbraten commented Jun 2, 2023

The logging is still pretty weird.

Current state:

  • logs are saved per step id using a json dump with ()
  • logs are streamed to UI per ?
  • logs are send to server using the workflow id and step-name => should probably be step-id

Expected:

  • logs are saved per step (maybe line by line / chunk by chunk and on get they would be concatenated)
  • logs are accessed per step-id
  • logs are send to agent per step-id
  • UI is listening on a stream per step-id

CC @6543

@qwerty287 qwerty287 added the bug Something isn't working label Jun 2, 2023
@6543 6543 added the regression fix a bug that was not released yet label Jun 2, 2023
@6543 6543 added this to the 1.0.0 milestone Jun 2, 2023
@codecov-commenter

This comment was marked as resolved.

Sign in to view
@6543 6543 added the important label Jun 3, 2023
@6543 6543 mentioned this pull request Jun 4, 2023
Closed
5 tasks
@6543
Copy link
Member

6543 commented Jun 4, 2023

@anbraten if you dont have time just push what we did discussed and I'll try to finish it

@anbraten
Copy link
Member Author

anbraten commented Jun 4, 2023

@6543 Would be nice if you cloud have a look. I've pushed all of my changes. Most things should be fine already. Missing is:

  • getting the id of a step (not workflow) in: agent/logger.go
  • testing streaming and loading of logs in UI

6543
6543 reviewed Jun 4, 2023
View reviewed changes
server/api/pipeline.go Outdated Show resolved Hide resolved
@6543 @anbraten
Update server/store/datastore/log.go
dab2e5d 
Co-authored-by: Anbraten <anton@ju60.de>
6543
6543 reviewed Jun 5, 2023
View reviewed changes
server/logging/logging.go Outdated Show resolved Hide resolved
@6543 6543 mentioned this pull request Jun 5, 2023
Closed
6543 added a commit to 6543-forks/woodpecker that referenced this pull request Jun 5, 2023
@6543
migration extracted from woodpecker-ci#1802
7c42d9e
@6543 6543 mentioned this pull request Jun 5, 2023
Merged
@6543 6543 marked this pull request as ready for review June 5, 2023 16:03
@6543
Copy link
Member

6543 commented Jun 5, 2023

ok to test this: -> old logs wont be displayed so just rerun and you have new logs to check against

6543
6543 approved these changes Jun 5, 2023
View reviewed changes
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

it has my OK ... but well somebody else might check it too :D

@lafriks
Copy link
Contributor

lafriks commented Jun 5, 2023

As this is in RPC spec can we add also log entry type Command?

@6543
Copy link
Member

6543 commented Jun 5, 2023

As this is in RPC spec can we add also log entry type Command?

there is no such thing in the proto3 file ... the only thing reverence the command is an TODO comment to add it later.
I can remove the code-comment for now if you like?

@6543 6543 requested a review from lafriks June 5, 2023 23:50
@6543 6543 mentioned this pull request Jun 6, 2023
Closed
5 tasks
@6543 6543 enabled auto-merge (squash) June 6, 2023 07:24
@6543
Copy link
Member

6543 commented Jun 6, 2023

Thankst to the basework @anbraten ;)

@6543 6543 merged commit 556607b into woodpecker-ci:master Jun 6, 2023
This was referenced Jun 6, 2023
Closed
Closed
6543 added a commit that referenced this pull request Jun 12, 2023
@6543
Migrate old logs to new database schema (#1828)
5139624 
address new models based on #1802
@Sebclem Sebclem mentioned this pull request Jun 21, 2023
Closed
5 tasks
@6543 6543 mentioned this pull request Jul 16, 2023
Merged
lafriks pushed a commit that referenced this pull request Jul 16, 2023
@6543
Index stepID for logEntries so loading time decrease significantly (#…
1b646a2 
…2006)

speedup from 2min loading to 0.01sec :D

got missed by  #1802
@6543 6543 mentioned this pull request Jul 26, 2023
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@6543 6543 6543 approved these changes

@lafriks lafriks lafriks approved these changes

Assignees
No one assigned
Labels
bug Something isn't working important refactor delete or replace old code regression fix a bug that was not released yet
Projects
None yet
Milestone
1.0.0
6 participants
@anbraten @codecov-commenter @6543 @woodpecker-bot @lafriks @qwerty287