chore: Add flag to skip legacy duplicate telemetry #630
Conversation
Currently we have some legacy metrics with `peer_id` in the metrics suffix (in addition to same metrics with `peer_id`d as label) 1. `raft_replication_appendEntries_rpc_peer0` 2. `raft_replication_appendEntries_logs_peer0` 3. `raft_replication_heartbeat_peer0` 4. `raft_replication_installSnapshot_peer0` These metrics may have additional `_count` or `_sum` metrics. And each metrics are multiplicative. Meaning if I have 10 peers, these metrics will be 10x. This PR adds a flag `noLegacyTelemetry` (default: false) which by setting to `true` you can skip those duplicate metrics. Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
|
|
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
There was a problem hiding this comment.
Thanks @kavirajk this is a useful addition and something we have not gotten around to doing for way to long 😅 .
I made one super minor request that doesn't impact behavior. I think @pkazmierczak also requested a changelog update which is hopefully simple.
If you're able to do those I think we'd be good to merge this. If not we can probably take care of that in a spare minute, but I'm a fan of commits/attribution staying with the person who did the hard work if possible!
| // NoLegacyTelemetry allow to skip the legacy metrics to avoid duplicates. | ||
| // legacy metrics are which has `_peer_name` as metric suffix instead as labels. | ||
| // e.g: raft_replication_heartbeat_peer0 | ||
| NoLegacyTelemetry bool |
There was a problem hiding this comment.
Minor thing, but we'd prefer to move this above the private internal-only skipStartup option. We typically keep these internal-only details grouped at the end of the struct just to make it clearer at a glance which configs are public.
Signed-off-by: Kaviraj <kavirajkanagaraj@gmail.com>
|
@banks @pkazmierczak thanks for the review :) Addressed the comments. |
Co-authored-by: Piotr Kazmierczak <470696+pkazmierczak@users.noreply.github.com>
Currently we have some legacy metrics with
peer_idin the metrics suffix (in addition to same metrics withpeer_idd as label)raft_replication_appendEntries_rpc_peer0raft_replication_appendEntries_logs_peer0raft_replication_heartbeat_peer0raft_replication_installSnapshot_peer0These metrics may have additional
_countor_summetrics. And each metrics are multiplicative. Meaning if I have 10 peers, these metrics will be 10x.This PR adds a flag
noLegacyTelemetry(default: false) which by setting totrueyou can skip those duplicate metrics.Before (
weaviate_internalis some custom prefix)After