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

Iter14 #14

Merged
merged 17 commits into from
Sep 5, 2024
Merged

Iter14 #14

merged 17 commits into from
Sep 5, 2024

Conversation

xoxloviwan
Copy link
Owner

No description provided.

Copy link
Collaborator

@vokinneberg vokinneberg left a comment

Choose a reason for hiding this comment

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

Поздравляю с успешным завершением четвертого спринта! 🎉 В этом спринте мы коснулись, наверное, самых сложных тем: многопоточности и криптографии. Ты реализовал доработки в соответствии с постановкой задачи. Отличная работа! 🚀 Желаю удачи на выпускном проекте! 🍀

Сильные стороны твоего PR:

  1. Код функционален и корректен! 🚀
  2. Твоя реализация достаточно локоничная и хорошо встроилась в структуру приложения. 💪

На что я бы рекомендовал обратить внимание и что можно улучшить:

  1. Было бы здорово реализовать тесты новых сценариев.

cmd/agent/main.go Outdated Show resolved Hide resolved
internal/metrics/metrics.go Show resolved Hide resolved
cmd/agent/main.go Outdated Show resolved Hide resolved
cmd/agent/main.go Outdated Show resolved Hide resolved
Comment on lines +28 to +45
go func() {
runtime.ReadMemStats(&MemStats)
wg.Done()
}()
go func() {
cpuUtilization, err = cpu.Percent(0, true) // вернет слайс с нагрузкой каждого ядра
if err != nil {
slog.Error("Getting cpu utilization failed:", slog.Any("error", err))
}
wg.Done()
}()
go func() {
vMem, err = mem.VirtualMemory()
if err != nil {
slog.Error("Getting virtual memory failed:", slog.Any("error", err))
}
wg.Done()
}()
Copy link
Collaborator

Choose a reason for hiding this comment

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

В целом ОК, но как мне кажется особого бенефита тут от распаралелливания не будет, при этом это добавляет дополнительную сложность.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Согласен, просто вроде бы так было нужно по ТЗ, чтобы опрос метрик был тоже в отдельных goрутинах, ну или я так понял...

@xoxloviwan xoxloviwan merged commit 7bd06e2 into main Sep 5, 2024
3 checks passed
@xoxloviwan xoxloviwan deleted the iter14 branch November 22, 2024 21:20
xoxloviwan added a commit that referenced this pull request Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants