uber-go / cadence-client

Framework for authoring workflows and activities running on top of the Cadence orchestration engine.
https://cadenceworkflow.io
MIT License
339 stars 128 forks source link

Create separate worker usage data collection and move hardware emit there #1293

Open timl3136 opened 7 months ago

timl3136 commented 7 months ago

What changed? Create a dedicated worker usage collector Move hardware usage emitting functionality from base worker to the worker usage collector

Why? We want to create a separate component responsible for collecting worker usage rather than a huge code block in the base worker.

How did you test it? Tested locally as well as tested in staging env to ensure metrics consistency and no goroutine leak.

Potential risks Instead of using Sync.Once to ensure the goroutine is run once per host, we move the hardware emitting to decision worker only as the Sync.Once might cause test timeout as it would keep other goroutine wait until the current one returns. So if a host does not have decision worker (impossible at the moment), it's hardware metrics won't be emitted.

codecov[bot] commented 2 months ago

Codecov Report

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

Project coverage is 73.23%. Comparing base (7f81710) to head (822564b).

Additional details and impacted files | [Files](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) | Coverage Δ | | |---|---|---| | [internal/internal\_worker.go](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?src=pr&el=tree&filepath=internal%2Finternal_worker.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-aW50ZXJuYWwvaW50ZXJuYWxfd29ya2VyLmdv) | `79.65% <100.00%> (+0.08%)` | :arrow_up: | | [internal/worker.go](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?src=pr&el=tree&filepath=internal%2Fworker.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-aW50ZXJuYWwvd29ya2VyLmdv) | `14.28% <ø> (ø)` | | | [internal/internal\_worker\_base.go](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?src=pr&el=tree&filepath=internal%2Finternal_worker_base.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-aW50ZXJuYWwvaW50ZXJuYWxfd29ya2VyX2Jhc2UuZ28=) | `75.54% <89.65%> (+9.04%)` | :arrow_up: | | [internal/internal\_worker\_usage\_collector.go](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?src=pr&el=tree&filepath=internal%2Finternal_worker_usage_collector.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go#diff-aW50ZXJuYWwvaW50ZXJuYWxfd29ya2VyX3VzYWdlX2NvbGxlY3Rvci5nbw==) | `53.22% <53.22%> (ø)` | | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). Last update [7f81710...822564b](https://app.codecov.io/gh/uber-go/cadence-client/pull/1293?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=uber-go).