Skip to content

Commit

Permalink
Remove caching objects to prevent leak (#2966)
Browse files Browse the repository at this point in the history
  • Loading branch information
jbreiding committed Jun 7, 2022
1 parent e4918d9 commit d3dc17e
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 45 deletions.
4 changes: 2 additions & 2 deletions common/metrics/events_scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,10 @@ func (e eventsScope) AddCounter(counter int, delta int64) {
// StartTimer starts a timer for the given metric name.
// Time will be recorded when stopwatch is stopped.
func (e eventsScope) StartTimer(timer int) Stopwatch {
m := getDefinition(e.serviceIdx, timer)

return &eventsStopwatch{
recordFunc: func(d time.Duration) {
m := getDefinition(e.serviceIdx, timer)

e.provider.Timer(m.metricName.String(), nil).Record(d, e.scopeTags...)

if !m.metricRollupName.Empty() {
Expand Down
30 changes: 6 additions & 24 deletions common/metrics/otel_event_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ package metrics
import (
"context"
"fmt"
"sync"

"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/metric/instrument"
Expand All @@ -41,13 +40,8 @@ import (
// MetricHandler is an event.Handler for OpenTelemetry metrics.
// Its Event method handles Metric events and ignores all others.
type OtelMetricHandler struct {
provider OpenTelemetryProvider
mu sync.Mutex
l log.Logger
// A map from event.Metrics to, effectively, otel Meters.
// But since the only thing we need from the Meter is recording a value, we
// use a function for that that closes over the Meter itself.
recordFuncs map[event.Metric]recordFunc
provider OpenTelemetryProvider
l log.Logger
excludeTags map[string]map[string]struct{}
}

Expand All @@ -60,12 +54,11 @@ func NewOtelMetricHandler(l log.Logger, o OpenTelemetryProvider, cfg ClientConfi
return &OtelMetricHandler{
provider: o,
l: l,
recordFuncs: map[event.Metric]recordFunc{},
excludeTags: configExcludeTags(cfg),
}
}

func (m *OtelMetricHandler) Event(ctx context.Context, e *event.Event) context.Context {
func (m OtelMetricHandler) Event(ctx context.Context, e *event.Event) context.Context {
if e.Kind != event.MetricKind {
return ctx
}
Expand All @@ -90,18 +83,7 @@ func (m *OtelMetricHandler) Event(ctx context.Context, e *event.Event) context.C
return ctx
}

func (m *OtelMetricHandler) getRecordFunc(em event.Metric) recordFunc {
m.mu.Lock()
defer m.mu.Unlock()
if f, ok := m.recordFuncs[em]; ok {
return f
}
f := m.newRecordFunc(em)
m.recordFuncs[em] = f
return f
}

func (m *OtelMetricHandler) newRecordFunc(em event.Metric) recordFunc {
func (m OtelMetricHandler) getRecordFunc(em event.Metric) recordFunc {
opts := em.Options()
name := em.Name()
otelOpts := []instrument.Option{
Expand Down Expand Up @@ -157,7 +139,7 @@ func (m *OtelMetricHandler) newRecordFunc(em event.Metric) recordFunc {
}
}

func (m *OtelMetricHandler) labelsToAttributes(ls []event.Label) []attribute.KeyValue {
func (m OtelMetricHandler) labelsToAttributes(ls []event.Label) []attribute.KeyValue {
var attrs []attribute.KeyValue
for _, l := range ls {
if vals, ok := m.excludeTags[l.Name]; ok {
Expand Down Expand Up @@ -191,7 +173,7 @@ func labelToAttribute(l event.Label) attribute.KeyValue {
}
}

func (m *OtelMetricHandler) Stop(logger log.Logger) {
func (m OtelMetricHandler) Stop(logger log.Logger) {
m.provider.Stop(logger)
}

Expand Down
23 changes: 4 additions & 19 deletions common/metrics/tally_event_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ package metrics

import (
"context"
"sync"

"github.com/uber-go/tally/v4"
"golang.org/x/exp/event"
Expand All @@ -37,9 +36,7 @@ import (

type TallyMetricHandler struct {
scope tally.Scope
mu sync.Mutex
l log.Logger
recordFuncs map[event.Metric]tallyRecordFunc
excludeTags map[string]map[string]struct{}
perUnitBuckets map[MetricUnit]tally.Buckets
}
Expand All @@ -59,12 +56,11 @@ func NewTallyMetricHandler(log log.Logger, scope tally.Scope, cfg ClientConfig,
l: log,
scope: scope,
perUnitBuckets: perUnitBuckets,
recordFuncs: make(map[event.Metric]tallyRecordFunc),
excludeTags: configExcludeTags(cfg),
}
}

func (t *TallyMetricHandler) Event(ctx context.Context, e *event.Event) context.Context {
func (t TallyMetricHandler) Event(ctx context.Context, e *event.Event) context.Context {
if e.Kind != event.MetricKind {
return ctx
}
Expand All @@ -89,18 +85,7 @@ func (t *TallyMetricHandler) Event(ctx context.Context, e *event.Event) context.
return ctx
}

func (t *TallyMetricHandler) getRecordFunc(em event.Metric, tags map[string]string) tallyRecordFunc {
t.mu.Lock()
defer t.mu.Unlock()
if f, ok := t.recordFuncs[em]; ok {
return f
}
f := t.newRecordFunc(em, tags)
t.recordFuncs[em] = f
return f
}

func (t *TallyMetricHandler) newRecordFunc(em event.Metric, tags map[string]string) tallyRecordFunc {
func (t TallyMetricHandler) getRecordFunc(em event.Metric, tags map[string]string) tallyRecordFunc {
name := em.Name()
unit := em.Options().Unit
switch em.(type) {
Expand Down Expand Up @@ -133,7 +118,7 @@ func (t *TallyMetricHandler) newRecordFunc(em event.Metric, tags map[string]stri
}
}

func (t *TallyMetricHandler) labelsToMap(attrs []event.Label) map[string]string {
func (t TallyMetricHandler) labelsToMap(attrs []event.Label) map[string]string {
tags := make(map[string]string)
for _, l := range attrs {
if vals, ok := t.excludeTags[l.Name]; ok {
Expand All @@ -151,6 +136,6 @@ func (t *TallyMetricHandler) labelsToMap(attrs []event.Label) map[string]string
return tags
}

func (t *TallyMetricHandler) Stop(logger log.Logger) {
func (t TallyMetricHandler) Stop(logger log.Logger) {
// noop
}

0 comments on commit d3dc17e

Please sign in to comment.