Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise internal cardinality metrics #241

Merged
merged 2 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions m3/example/m3_main.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2021 Uber Technologies, Inc.
// Copyright (c) 2024 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 @@ -87,7 +87,8 @@
}

scope, closer := tally.NewRootScope(tally.ScopeOptions{
CachedReporter: r,
CachedReporter: r,
CardinalityMetricsTags: cfg.M3.InternalTags,

Check warning on line 91 in m3/example/m3_main.go

View check run for this annotation

Codecov / codecov/patch

m3/example/m3_main.go#L90-L91

Added lines #L90 - L91 were not covered by tests
}, 1*time.Second)

defer closer.Close()
Expand Down
8 changes: 3 additions & 5 deletions m3/reporter.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2021 Uber Technologies, Inc.
// Copyright (c) 2024 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 @@ -66,8 +66,6 @@ const (
DefaultHistogramBucketIDName = "bucketid"
// DefaultHistogramBucketName is the default histogram bucket name tag name
DefaultHistogramBucketName = "bucket"
// DefaultTagRedactValue is the default tag value to use when redacting
DefaultTagRedactValue = "global"
// DefaultHistogramBucketTagPrecision is the default
// precision to use when formatting the metric tag
// with the histogram bucket bound values.
Expand Down Expand Up @@ -290,8 +288,8 @@ func NewReporter(opts Options) (Reporter, error) {

internalTags := map[string]string{
"version": tally.Version,
"host": DefaultTagRedactValue,
"instance": DefaultTagRedactValue,
"host": tally.DefaultTagRedactValue,
"instance": tally.DefaultTagRedactValue,
}

for k, v := range opts.InternalTags {
Expand Down
1 change: 1 addition & 0 deletions m3/reporter_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ func main() {
scope, closer := tally.NewRootScope(tally.ScopeOptions{
CachedReporter: r,
OmitCardinalityMetrics: true,
}, 5 * time.Second)
defer closer.Close()
Expand Down
6 changes: 3 additions & 3 deletions m3/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var (
var protocols = []Protocol{Compact, Binary}

const internalMetrics = 5 // Additional metrics the reporter sends in a batch - use this, not a magic number.
const cardinalityMetrics = 3 // Additional metrics emitted by the scope registry.
const cardinalityMetrics = 4 // Additional metrics emitted by the scope registry.

// TestReporter tests the reporter works as expected with both compact and binary protocols
func TestReporter(t *testing.T) {
Expand Down Expand Up @@ -601,8 +601,8 @@ func TestReporterCommmonTagsInternal(t *testing.T) {
}

// The following tags should be redacted.
require.True(t, tagEquals(metric.Tags, "host", DefaultTagRedactValue))
require.True(t, tagEquals(metric.Tags, "instance", DefaultTagRedactValue))
require.True(t, tagEquals(metric.Tags, "host", tally.DefaultTagRedactValue))
require.True(t, tagEquals(metric.Tags, "instance", tally.DefaultTagRedactValue))
} else {
require.Equal(t, "testCounter1", metric.Name)
require.False(t, tagIncluded(metric.Tags, "internal1"))
Expand Down
8 changes: 4 additions & 4 deletions m3/scope_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,10 @@ func newTestReporterScope(
require.NoError(t, err)

scope, closer := tally.NewRootScope(tally.ScopeOptions{
Prefix: scopePrefix,
Tags: scopeTags,
CachedReporter: r,
MetricsOption: tally.SendInternalMetrics,
Prefix: scopePrefix,
Tags: scopeTags,
CachedReporter: r,
OmitCardinalityMetrics: false,
}, shortInterval)

return r, scope, func() {
Expand Down
33 changes: 12 additions & 21 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) 2024 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,17 +28,7 @@ import (
"go.uber.org/atomic"
)

// InternalMetricOption is used to configure internal metrics.
type InternalMetricOption int

const (
// Unset is the "no-op" config, which turns off internal metrics.
Unset InternalMetricOption = iota
// SendInternalMetrics turns on internal metrics submission.
SendInternalMetrics
// OmitInternalMetrics turns off internal metrics submission.
OmitInternalMetrics

_defaultInitialSliceSize = 16
_defaultReportingInterval = 2 * time.Second
)
Expand Down Expand Up @@ -106,15 +96,16 @@ 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
MetricsOption InternalMetricOption
Tags map[string]string
Prefix string
Reporter StatsReporter
CachedReporter CachedStatsReporter
Separator string
DefaultBuckets Buckets
SanitizeOptions *SanitizeOptions
registryShardCount uint
OmitCardinalityMetrics bool
CardinalityMetricsTags map[string]string
}

// NewRootScope creates a new root Scope with a set of options and
Expand Down Expand Up @@ -190,7 +181,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.MetricsOption)
s.registry = newScopeRegistryWithShardCount(s, opts.registryShardCount, opts.OmitCardinalityMetrics, opts.CardinalityMetricsTags)

if interval > 0 {
s.wg.Add(1)
Expand Down
69 changes: 49 additions & 20 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) 2024 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 @@ -33,10 +33,15 @@ var (
scopeRegistryKey = keyForPrefixedStringMaps

// 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"
scopeCardinalityName = "tally.internal.scope_cardinality"
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

num-active-scopes would be a better name for this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


const (
// DefaultTagRedactValue is the default tag value to use when redacting
DefaultTagRedactValue = "global"
)

type scopeRegistry struct {
Expand All @@ -45,10 +50,16 @@ 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
// Internal metrics related.
internalMetricsOption InternalMetricOption
omitCardinalityMetrics bool
cardinalityMetricsTags map[string]string
sanitizedCounterCardinalityName string
sanitizedGaugeCardinalityName string
sanitizedHistogramCardinalityName string
sanitizedScopeCardinalityName string
cachedCounterCardinalityGauge CachedGauge
cachedGaugeCardinalityGauge CachedGauge
cachedHistogramCardinalityGauge CachedGauge
cachedScopeCardinalityGauge CachedGauge
}

type scopeBucket struct {
Expand All @@ -59,7 +70,8 @@ type scopeBucket struct {
func newScopeRegistryWithShardCount(
root *scope,
shardCount uint,
internalMetricsOption InternalMetricOption,
omitCardinalityMetrics bool,
cardinalityMetricsTags map[string]string,
) *scopeRegistry {
if shardCount == 0 {
shardCount = uint(runtime.GOMAXPROCS(-1))
Expand All @@ -69,17 +81,34 @@ func newScopeRegistryWithShardCount(
root: root,
subscopes: make([]*scopeBucket, shardCount),
seed: maphash.MakeSeed(),
internalMetricsOption: internalMetricsOption,
omitCardinalityMetrics: omitCardinalityMetrics,
sanitizedCounterCardinalityName: root.sanitizer.Name(counterCardinalityName),
sanitizedGaugeCardinalityName: root.sanitizer.Name(gaugeCardinalityName),
sanitizedHistogramCardinalityName: root.sanitizer.Name(histogramCardinalityName),
sanitizedScopeCardinalityName: root.sanitizer.Name(scopeCardinalityName),
cardinalityMetricsTags: map[string]string{
"version": Version,
"host": DefaultTagRedactValue,
"instance": DefaultTagRedactValue,
},
}

for k, v := range cardinalityMetricsTags {
r.cardinalityMetricsTags[root.sanitizer.Key(k)] = root.sanitizer.Value(v)
}

for i := uint(0); i < shardCount; i++ {
r.subscopes[i] = &scopeBucket{
s: make(map[string]*scope),
}
r.subscopes[i].s[scopeRegistryKey(root.prefix, root.tags)] = root
}
if r.root.cachedReporter != nil {
r.cachedCounterCardinalityGauge = r.root.cachedReporter.AllocateGauge(r.sanitizedCounterCardinalityName, r.cardinalityMetricsTags)
r.cachedGaugeCardinalityGauge = r.root.cachedReporter.AllocateGauge(r.sanitizedGaugeCardinalityName, r.cardinalityMetricsTags)
r.cachedHistogramCardinalityGauge = r.root.cachedReporter.AllocateGauge(r.sanitizedHistogramCardinalityName, r.cardinalityMetricsTags)
r.cachedScopeCardinalityGauge = r.root.cachedReporter.AllocateGauge(r.sanitizedScopeCardinalityName, r.cardinalityMetricsTags)
}
return r
}

Expand Down Expand Up @@ -237,12 +266,13 @@ func (r *scopeRegistry) removeWithRLock(subscopeBucket *scopeBucket, key string)

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

counters, gauges, histograms := atomic.Int64{}, atomic.Int64{}, atomic.Int64{}
counters, gauges, histograms, scopes := atomic.Int64{}, atomic.Int64{}, atomic.Int64{}, atomic.Int64{}
rootCounters, rootGauges, rootHistograms := atomic.Int64{}, atomic.Int64{}, atomic.Int64{}
scopes.Inc() // Account for root scope.
r.ForEachScope(
func(ss *scope) {
counterSliceLen, gaugeSliceLen, histogramSliceLen := int64(len(ss.countersSlice)), int64(len(ss.gaugesSlice)), int64(len(ss.histogramsSlice))
Expand All @@ -255,25 +285,24 @@ func (r *scopeRegistry) reportInternalMetrics() {
counters.Add(counterSliceLen)
gauges.Add(gaugeSliceLen)
histograms.Add(histogramSliceLen)
scopes.Inc()
},
)

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

brawndou marked this conversation as resolved.
Show resolved Hide resolved
if r.root.reporter != nil {
r.root.reporter.ReportCounter(r.sanitizedCounterCardinalityName, internalTags, counters.Load())
r.root.reporter.ReportCounter(r.sanitizedGaugeCardinalityName, internalTags, gauges.Load())
r.root.reporter.ReportCounter(r.sanitizedHistogramCardinalityName, internalTags, histograms.Load())
r.root.reporter.ReportGauge(r.sanitizedCounterCardinalityName, r.cardinalityMetricsTags, float64(counters.Load()))
r.root.reporter.ReportGauge(r.sanitizedGaugeCardinalityName, r.cardinalityMetricsTags, float64(gauges.Load()))
r.root.reporter.ReportGauge(r.sanitizedHistogramCardinalityName, r.cardinalityMetricsTags, float64(histograms.Load()))
r.root.reporter.ReportGauge(r.sanitizedScopeCardinalityName, r.cardinalityMetricsTags, float64(scopes.Load()))
}

if r.root.cachedReporter != nil {
numCounters := r.root.cachedReporter.AllocateCounter(r.sanitizedCounterCardinalityName, internalTags)
numGauges := r.root.cachedReporter.AllocateCounter(r.sanitizedGaugeCardinalityName, internalTags)
numHistograms := r.root.cachedReporter.AllocateCounter(r.sanitizedHistogramCardinalityName, internalTags)
numCounters.ReportCount(counters.Load())
numGauges.ReportCount(gauges.Load())
numHistograms.ReportCount(histograms.Load())
r.cachedCounterCardinalityGauge.ReportGauge(float64(counters.Load()))
r.cachedGaugeCardinalityGauge.ReportGauge(float64(gauges.Load()))
r.cachedHistogramCardinalityGauge.ReportGauge(float64(histograms.Load()))
r.cachedScopeCardinalityGauge.ReportGauge(float64(scopes.Load()))
}
}
Loading
Loading