Skip to content

Conversation

Groxx
Copy link
Member

@Groxx Groxx commented Aug 27, 2025

tbd.

sample with the new linter:

❯ git diff | cat                  
service/history/replication/task_processor.go --- Go
511 511 
512 512         shardScope := p.metricsClient.Scope(scope, metrics.TargetClusterTag(p.sourceCluster), metrics.InstanceTag(strconv.Itoa(p.shard.GetShardID())))
513 513         shardScope.IncCounter(metrics.ReplicationTasksApplied)
... 514         p.metrics.Count("replication_tasks_applied", 1, p.metricTags.With("shard_id", strconv.Itoa(p.shard.GetShardID())))
514 515     }
515 516 
516 517     return err

❯ make .build/metrics-lint
linting metrics definitions...
Metric name "replication_tasks_applied" used in multiple places:
        /path/to/github.com/uber/cadence/common/metrics/defs.go:3413:66: success: replication_tasks_applied
        /path/to/github.com/uber/cadence/service/history/replication/task_processor.go:514:3: success: replication_tasks_applied
make: *** [.build/metrics-lint] Error 1

@Groxx Groxx force-pushed the simpler-histogram branch from 56257ad to 75d6f56 Compare September 3, 2025 23:52
@Groxx Groxx mentioned this pull request Sep 4, 2025
Groxx added a commit that referenced this pull request Sep 4, 2025
Part cleanup due to discoveries in e.g. #7210, part prep-work for migrating metrics to almost literally any new setup (to make that easier and safer, by allowing int -> name lookups).

I *believe* this is a safe thing to do, as we don't seem to do anything fancy with the metric-def indexes.
But it's quite hard to verify because we don't have a unique type (fixed in #7238) and all the code here is rather value-oriented and fallback-y so it's hard to be totally confident that everything is handled.
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.

1 participant