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.

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

fix(web): node container log stream sharing #942

Merged
merged 8 commits into from
Apr 2, 2024

Conversation

m8vago
Copy link
Contributor

@m8vago m8vago commented Mar 27, 2024

  • Remove NodeGlobalContainerController
  • Add _ as global container namespace.
  • Refactor agent calls for log streaming
  • Add upgradeable log stream watcher
  • Automatically reattach to the log stream, when an agent reconnects

@m8vago m8vago requested a review from a team as a code owner March 27, 2024 12:21
@m8vago m8vago marked this pull request as draft March 27, 2024 12:22
@github-actions github-actions bot added source:web The scope of the issue or pull request is web. lang:typescript labels Mar 27, 2024
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch from 37ba383 to de7f190 Compare March 27, 2024 12:32
@m8vago m8vago changed the title fix(crux): node container log stream sharing fix(web): node container log stream sharing Mar 27, 2024
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch from 6f27ae0 to 9fe2907 Compare March 27, 2024 14:38
@github-actions github-actions bot added source:agent The scope of the issue or pull request is agent. lang:golang source:proto Protobuf related changes. labels Mar 28, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2024

Codecov Report

Attention: Patch coverage is 33.81148% with 323 lines in your changes are missing coverage. Please review.

Project coverage is 17.99%. Comparing base (b44a979) to head (b0520be).

Files Patch % Lines
golang/internal/grpc/grpc.go 0.00% 152 Missing ⚠️
web/crux/src/app/agent/agent.service.ts 0.00% 23 Missing ⚠️
golang/pkg/dagent/utils/dockerhelper.go 0.00% 17 Missing ⚠️
web/crux/src/grpc/protobuf/proto/common.ts 11.11% 16 Missing ⚠️
web/crux/src/shared/fixed-length-linked-list.ts 62.16% 14 Missing ⚠️
...rc/app/node/pipes/node.container-log-query.pipe.ts 0.00% 13 Missing ⚠️
web/crux/src/domain/agent.ts 79.62% 11 Missing ⚠️
web/crux/src/websockets/dyo.ws.adapter.ts 0.00% 11 Missing ⚠️
...rux/src/app/node/node.container.http.controller.ts 0.00% 10 Missing ⚠️
...c/app/node/interceptors/node.prefix.interceptor.ts 0.00% 8 Missing ⚠️
... and 14 more

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #942      +/-   ##
===========================================
+ Coverage    17.94%   17.99%   +0.05%     
===========================================
  Files          346      350       +4     
  Lines        17467    17606     +139     
  Branches      1027     1029       +2     
===========================================
+ Hits          3135     3169      +34     
- Misses       14143    14249     +106     
+ Partials       189      188       -1     

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

@m8vago m8vago force-pushed the fix/node-log-stream-issues branch 7 times, most recently from 8c0ac27 to 8dc76da Compare March 31, 2024 14:21
@m8vago m8vago marked this pull request as ready for review March 31, 2024 14:23
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch 6 times, most recently from 2730ec8 to 89ab1c5 Compare March 31, 2024 15:44
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch 4 times, most recently from ae07635 to 30eb79e Compare March 31, 2024 16:09
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch from 30eb79e to b2772bf Compare March 31, 2024 16:35
Copy link
Contributor

@nandor-magyar nandor-magyar left a comment

Choose a reason for hiding this comment

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

I feel like I have got a fresh pair of eyes 👀
I've checked the Go part, that looks good. 👍 My comments are mostly wording related.
I've skimmed the TS part too, but that would better be checked by some else.

golang/pkg/crane/crane.go Outdated Show resolved Hide resolved
golang/internal/grpc/grpc.go Outdated Show resolved Hide resolved
golang/internal/grpc/grpc.go Outdated Show resolved Hide resolved
web/crux/.env.example Outdated Show resolved Hide resolved
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch 3 times, most recently from 0e925d5 to 8eaf483 Compare April 2, 2024 12:19
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch from 8eaf483 to 4d027e8 Compare April 2, 2024 12:30
@m8vago m8vago force-pushed the fix/node-log-stream-issues branch from b0520be to 45bf005 Compare April 2, 2024 13:46
@m8vago m8vago merged commit ec5f567 into develop Apr 2, 2024
26 checks passed
@m8vago m8vago deleted the fix/node-log-stream-issues branch April 2, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang:golang lang:typescript pr:fix source:agent The scope of the issue or pull request is agent. source:proto Protobuf related changes. source:web The scope of the issue or pull request is web.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants