Batch Mutable State Metrics #6655
Conversation
|
|
njo
force-pushed
the
extend_metrics_handler
branch
from
2eb04bf to
22d7056
Compare
service/history/workflow/metrics.go
Outdated
| } | ||
|
|
||
| for category, taskCount := range stats.TaskCountByCategory { | ||
| // We use the metricsHandler rather than the batchHandler here because the same metric is repeatedly sent |
There was a problem hiding this comment.
Can't we support this in the batch handler? This limits the approach a bit.
There was a problem hiding this comment.
We could add it to the batch and leave it up to the handler implementation to decide what to do with duplicate metrics/labels. The main issue with this particular line is it also adds extra labels. We can't add those labels to a batch as is because they couldn't be correlated with the specific task count and would apply to the batch as a whole. We could change the metric name/label to be [categoryname]-task_count but that would break existing task_count metrics (unless the concatenation happened in the handler but that seems like a bit of a hack).
There was a problem hiding this comment.
Thinking about this some more I'm going to add it to the batch and we can decide what to do about it in the underlying implementation. I think I can make some kind of nested structure to handle it.
njo
force-pushed
the
extend_metrics_handler
branch
from
22d7056 to
7b74056
Compare
## What changed? Extend metrics.Handler interface to include a batching feature which enables implementations to send "wide events". Batch the mutable state metrics. ## Why? Send a single metrics "event" where possible rather than several. ## How did you test it? Unit tests. ## Potential risks None expected. ## Documentation Nothing in `docs/` to update. Release notes to come. ## Is hotfix candidate? No
What changed?
Extend metrics.Handler interface to include a batching feature which enables implementations to send "wide events". Batch the mutable state metrics.
Why?
Send a single metrics "event" where possible rather than several.
How did you test it?
Unit tests.
Potential risks
None expected.
Documentation
Nothing in
docs/to update. Release notes to come.Is hotfix candidate?
No