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

[agent-smith] Enable egress monitoring #8742

Merged
merged 2 commits into from
Mar 15, 2022
Merged

[agent-smith] Enable egress monitoring #8742

merged 2 commits into from
Mar 15, 2022

Conversation

princerachit
Copy link
Contributor

@princerachit princerachit commented Mar 11, 2022

Description

Related Issue(s)

Fixes https://github.com/gitpod-io/ops/issues/1383

How to test

  • Enable debug logs for your deployment in core-dev using gpctl debug logs agent-smith
  • (optional) modify agent-smith config map and update the reduce the thresholds of egress in the cm. This will allow you to see right logs even when you push small files. The default config has 3G of threshold, so you need to push around 3G from your workspace
  • Start a workspace A
  • Start another workspace B
  • Generate egress in your workspace A. You can copy a file to gcs bucket or push a docker image.
  • Wait for 2 minutes (that is the ticker time)
  • Stop workspace A and B
  • Check the logs. You should see something similar to below:

Logs

  • Adding a workspace to a map
{"level":"debug","message":"adding workspace with pid 3495247 and workspaceId princerachit-atheoscommu-mowawkn4gso to workspaces","serviceContext":{"service":"agent-smith","version":"commit-17abe734cb4252becd7a3447bb0a2b5b4ff21de9"},"severity":"DEBUG","time":"2022-03-11T11:39:37Z"}
  • Infringing workspace
{"instanceId":"82c305b5-4c8a-4637-aeb9-4bcbd2f71df1","level":"info","message":"found egress infringing workspace","serviceContext":{"service":"agent-smith","version":"commit-c5295379290c4f83bb2fe5fb271c6ea8fba8104b"},"severity":"INFO","time":"2022-03-15T05:30:35Z","userId":"","workspaceId":"princerachit-atheoscommu-wqfinhsmx5f"}
  • Logs from callback function reporting egress total bytes
{"level":"debug","message":"total egress bytes 486787600","serviceContext":{"service":"agent-smith","version":"commit-17abe734cb4252becd7a3447bb0a2b5b4ff21de9"},"severity":"DEBUG","time":"2022-03-11T11:48:37Z"}
  • Deletion of workspace from map post it is stopped
{"level":"debug","message":"deleting workspace with pid 3495247 and workspaceId princerachit-atheoscommu-mowawkn4gso from workspaces","serviceContext":{"service":"agent-smith","version":"commit-17abe734cb4252becd7a3447bb0a2b5b4ff21de9"},"severity":"DEBUG","time":"2022-03-11T11:46:37Z"}

Release Notes

Enable egress metrics for agent-smith

Documentation

@codecov
Copy link

codecov bot commented Mar 11, 2022

Codecov Report

Merging #8742 (c529537) into main (b10bdc6) will increase coverage by 6.00%.
The diff coverage is 18.51%.

@@            Coverage Diff             @@
##             main    #8742      +/-   ##
==========================================
+ Coverage   12.31%   18.31%   +6.00%     
==========================================
  Files          20       39      +19     
  Lines        1161     3401    +2240     
==========================================
+ Hits          143      623     +480     
- Misses       1014     2740    +1726     
- Partials        4       38      +34     
Flag Coverage Δ
components-ee-agent-smith-app 39.33% <18.51%> (?)
components-ee-agent-smith-lib 39.33% <18.51%> (?)
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
install-installer-raw-app 4.27% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/ee/agent-smith/pkg/agent/agent.go 29.65% <2.22%> (ø)
components/ee/agent-smith/pkg/agent/metrics.go 50.00% <100.00%> (ø)
components/local-app/pkg/auth/auth.go
components/local-app/pkg/auth/pkce.go
...onents/ee/agent-smith/pkg/classifier/classifier.go 19.73% <0.00%> (ø)
install/installer/pkg/common/common.go 4.45% <0.00%> (ø)
components/ee/agent-smith/pkg/agent/egress.go 0.00% <0.00%> (ø)
install/installer/pkg/common/display.go 0.00% <0.00%> (ø)
install/installer/pkg/common/storage.go 0.00% <0.00%> (ø)
...staller/pkg/components/ws-manager/networkpolicy.go 0.00% <0.00%> (ø)
... and 15 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@princerachit princerachit marked this pull request as ready for review March 11, 2022 12:03
@princerachit princerachit requested a review from a team March 11, 2022 12:03
@github-actions github-actions bot added the team: workspace Issue belongs to the Workspace team label Mar 11, 2022
@princerachit
Copy link
Contributor Author

princerachit commented Mar 11, 2022

/werft run

👍 started the job as gitpod-build-prs-agent-smith.8

@kylos101
Copy link
Contributor

Hey @princerachit , really cool to see this, thank you!

Can you think if we'll need to update any configuration for the related deploy of workspace-clusters to support this?

@princerachit
Copy link
Contributor Author

Can you think if we'll need to update any configuration for the related deploy of workspace-clusters to support this?

No. The config already exists and should work.

// the workspaces map. This is because we do not lock before reading from the map.
// So, if multiple processes written to the cli chan are
// read in parallel, all of them will get inside the below if check.
if _, ok := workspaces[i.Workspace.PID]; !ok {
Copy link
Member

Choose a reason for hiding this comment

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

How about acquiring the lock before reading from the map? Currently it would be possible that one thread is reading from the map while another is currently modifying the internal data structures of the map.

Copy link
Contributor Author

@princerachit princerachit Mar 14, 2022

Choose a reason for hiding this comment

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

It would not make any difference as the if block will only overwrite the existing entry with the same content. i.e. even if the data in the map is updated we do not care as we just want to make sure that the key exists in the map.

Introducing a lock for reads will block the subsequent classification steps as the number of reads are going to be too many.

components/ee/agent-smith/pkg/agent/agent.go Outdated Show resolved Hide resolved
@princerachit
Copy link
Contributor Author

princerachit commented Mar 15, 2022

/werft run

👍 started the job as gitpod-build-prs-agent-smith.11
with-vm=true

@princerachit
Copy link
Contributor Author

princerachit commented Mar 15, 2022

/werft run

👍 started the job as gitpod-build-prs-agent-smith.14

@roboquat roboquat merged commit 5a9c920 into main Mar 15, 2022
@roboquat roboquat deleted the prs/agent-smith branch March 15, 2022 11:36
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note size/M team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants