Skip to content

Commit

Permalink
Revert "fix broken internal metrics names and make them optional (#203)"
Browse files Browse the repository at this point in the history
This reverts commit 24b844b.
  • Loading branch information
brawndou authored Jan 27, 2023
1 parent 24b844b commit 9ad201e
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 115 deletions.
28 changes: 11 additions & 17 deletions scope.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2023 Uber Technologies, Inc.
// Copyright (c) 2022 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -28,13 +28,7 @@ import (
"go.uber.org/atomic"
)

type InternalMetricOption int

const (
Unset InternalMetricOption = iota
SendInternalMetrics
OmitInternalMetrics

_defaultInitialSliceSize = 16
)

Expand Down Expand Up @@ -101,15 +95,15 @@ type scope struct {

// ScopeOptions is a set of options to construct a scope.
type ScopeOptions struct {
Tags map[string]string
Prefix string
Reporter StatsReporter
CachedReporter CachedStatsReporter
Separator string
DefaultBuckets Buckets
SanitizeOptions *SanitizeOptions
registryShardCount uint
internalMetricsOption InternalMetricOption
Tags map[string]string
Prefix string
Reporter StatsReporter
CachedReporter CachedStatsReporter
Separator string
DefaultBuckets Buckets
SanitizeOptions *SanitizeOptions
registryShardCount uint
skipInternalMetrics bool
}

// NewRootScope creates a new root Scope with a set of options and
Expand Down Expand Up @@ -179,7 +173,7 @@ func newRootScope(opts ScopeOptions, interval time.Duration) *scope {
s.tags = s.copyAndSanitizeMap(opts.Tags)

// Register the root scope
s.registry = newScopeRegistryWithShardCount(s, opts.registryShardCount, opts.internalMetricsOption)
s.registry = newScopeRegistryWithShardCount(s, opts.registryShardCount, opts.skipInternalMetrics)

if interval > 0 {
s.wg.Add(1)
Expand Down
70 changes: 29 additions & 41 deletions scope_registry.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2023 Uber Technologies, Inc.
// Copyright (c) 2022 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -34,9 +34,9 @@ var (

// Metrics related.
internalTags = map[string]string{"version": Version}
counterCardinalityName = "tally_internal_counter_cardinality"
gaugeCardinalityName = "tally_internal_gauge_cardinality"
histogramCardinalityName = "tally_internal_histogram_cardinality"
counterCardinalityName = "tally.internal.counter-cardinality"
gaugeCardinalityName = "tally.internal.gauge-cardinality"
histogramCardinalityName = "tally.internal.histogram-cardinality"
)

type scopeRegistry struct {
Expand All @@ -45,28 +45,24 @@ type scopeRegistry struct {
// We need a subscope per GOPROC so that we can take advantage of all the cpu available to the application.
subscopes []*scopeBucket
// Toggles internal metrics reporting.
internalMetricsOption InternalMetricOption
skipInternalMetrics bool
}

type scopeBucket struct {
mu sync.RWMutex
s map[string]*scope
}

func newScopeRegistryWithShardCount(
root *scope,
shardCount uint,
internalMetricsOption InternalMetricOption,
) *scopeRegistry {
func newScopeRegistryWithShardCount(root *scope, shardCount uint, skipInternalMetrics bool) *scopeRegistry {
if shardCount == 0 {
shardCount = uint(runtime.GOMAXPROCS(-1))
}

r := &scopeRegistry{
root: root,
subscopes: make([]*scopeBucket, shardCount),
seed: maphash.MakeSeed(),
internalMetricsOption: internalMetricsOption,
root: root,
subscopes: make([]*scopeBucket, shardCount),
seed: maphash.MakeSeed(),
skipInternalMetrics: skipInternalMetrics,
}
for i := uint(0); i < shardCount; i++ {
r.subscopes[i] = &scopeBucket{
Expand Down Expand Up @@ -231,47 +227,39 @@ func (r *scopeRegistry) removeWithRLock(subscopeBucket *scopeBucket, key string)

// Records internal Metrics' cardinalities.
func (r *scopeRegistry) reportInternalMetrics() {
if r.internalMetricsOption != SendInternalMetrics {
if r.skipInternalMetrics {
return
}

counters, gauges, histograms := atomic.Int64{}, atomic.Int64{}, atomic.Int64{}
rootCounters, rootGauges, rootHistograms := atomic.Int64{}, atomic.Int64{}, atomic.Int64{}
r.ForEachScope(
func(ss *scope) {
counterSliceLen, gaugeSliceLen, histogramSliceLen := int64(len(ss.countersSlice)), int64(len(ss.gaugesSlice)), int64(len(ss.histogramsSlice))
if ss.root { // Root scope is referenced across all buckets.
rootCounters.Store(counterSliceLen)
rootGauges.Store(gaugeSliceLen)
rootHistograms.Store(histogramSliceLen)
return
}
counters.Add(counterSliceLen)
gauges.Add(gaugeSliceLen)
histograms.Add(histogramSliceLen)
},
)
r.ForEachScope(func(ss *scope) {
counterSliceLen, gaugeSliceLen, histogramSliceLen := int64(len(ss.countersSlice)), int64(len(ss.gaugesSlice)), int64(len(ss.histogramsSlice))
if ss.root { // Root scope is referenced across all buckets.
rootCounters.Store(counterSliceLen)
rootGauges.Store(gaugeSliceLen)
rootHistograms.Store(histogramSliceLen)
return
}
counters.Add(counterSliceLen)
gauges.Add(gaugeSliceLen)
histograms.Add(histogramSliceLen)
})

counters.Add(rootCounters.Load())
gauges.Add(rootGauges.Load())
histograms.Add(rootHistograms.Load())

if r.root.reporter != nil {
r.root.reporter.ReportCounter(r.root.sanitizer.Name(counterCardinalityName), internalTags, counters.Load())
r.root.reporter.ReportCounter(r.root.sanitizer.Name(gaugeCardinalityName), internalTags, gauges.Load())
r.root.reporter.ReportCounter(r.root.sanitizer.Name(histogramCardinalityName), internalTags, histograms.Load())
r.root.reporter.ReportCounter(counterCardinalityName, internalTags, counters.Load())
r.root.reporter.ReportCounter(gaugeCardinalityName, internalTags, gauges.Load())
r.root.reporter.ReportCounter(histogramCardinalityName, internalTags, histograms.Load())
}

if r.root.cachedReporter != nil {
numCounters := r.root.cachedReporter.AllocateCounter(
r.root.sanitizer.Name(counterCardinalityName), internalTags,
)
numGauges := r.root.cachedReporter.AllocateCounter(
r.root.sanitizer.Name(gaugeCardinalityName), internalTags,
)
numHistograms := r.root.cachedReporter.AllocateCounter(
r.root.sanitizer.Name(histogramCardinalityName), internalTags,
)
numCounters := r.root.cachedReporter.AllocateCounter(counterCardinalityName, internalTags)
numGauges := r.root.cachedReporter.AllocateCounter(gaugeCardinalityName, internalTags)
numHistograms := r.root.cachedReporter.AllocateCounter(histogramCardinalityName, internalTags)
numCounters.ReportCount(counters.Load())
numGauges.ReportCount(gauges.Load())
numHistograms.ReportCount(histograms.Load())
Expand Down
6 changes: 3 additions & 3 deletions scope_registry_test.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2023 Uber Technologies, Inc.
// Copyright (c) 2022 Uber Technologies, Inc.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -55,7 +55,7 @@ func TestVerifyCachedTaggedScopesAlloc(t *testing.T) {

func TestNewTestStatsReporterOneScope(t *testing.T) {
r := newTestStatsReporter()
root, closer := NewRootScope(ScopeOptions{Reporter: r, internalMetricsOption: SendInternalMetrics}, 0)
root, closer := NewRootScope(ScopeOptions{Reporter: r, skipInternalMetrics: false}, 0)
s := root.(*scope)

numFakeCounters := 3
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestNewTestStatsReporterOneScope(t *testing.T) {

func TestNewTestStatsReporterManyScopes(t *testing.T) {
r := newTestStatsReporter()
root, closer := NewRootScope(ScopeOptions{Reporter: r, internalMetricsOption: SendInternalMetrics}, 0)
root, closer := NewRootScope(ScopeOptions{Reporter: r, skipInternalMetrics: false}, 0)
wantCounters, wantGauges, wantHistograms := int64(3), int64(2), int64(1)

s := root.(*scope)
Expand Down
Loading

0 comments on commit 9ad201e

Please sign in to comment.