Skip to content

Commit

Permalink
Use same root tags for system scope and user scope (#2784)
Browse files Browse the repository at this point in the history
  • Loading branch information
yux0 authored Apr 29, 2022
1 parent 08d0232 commit 4faf1f4
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 17 deletions.
8 changes: 8 additions & 0 deletions common/metrics/metric_client_tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,3 +127,11 @@ func (s *MetricTestSuiteBase) TestScopeReportHistogram() {
assert.NoError(s.T(), s.metricTestUtility.ContainsHistogram(testDef.metricName, map[string]string{namespace: namespaceAllValue, OperationTagName: ScopeDefs[UnitTestService][TestScope1].operation, serviceName: primitives.UnitTestService}, 66))
assert.Equal(s.T(), 1, s.metricTestUtility.CollectionSize())
}

func (s *MetricTestSuiteBase) TestUserScopeReportHistogram() {
userScopeMetrics := "test_user_scope_metrics"
tags := map[string]string{OperationTagName: "user_scope_operation"}
s.testClient.UserScope().Tagged(tags).RecordDistribution(userScopeMetrics, Dimensionless, 66)
assert.NoError(s.T(), s.metricTestUtility.ContainsHistogram(MetricName(userScopeMetrics), map[string]string{namespace: namespaceAllValue, OperationTagName: "user_scope_operation", serviceName: primitives.UnitTestService}, 66))
assert.Equal(s.T(), 1, s.metricTestUtility.CollectionSize())
}
10 changes: 5 additions & 5 deletions common/metrics/opentelemetry_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,20 @@ func NewOpenTelemetryClient(clientConfig *ClientConfig, serviceIdx ServiceIdx, r
return NewTagFilteringScope(tagsFilterConfig, impl)
}

globalRootScope := newOpenTelemetryScope(serviceIdx, reporter.GetMeter(), nil, clientConfig.Tags, getMetricDefs(serviceIdx), false, gaugeCache, false)
rootScope := newOpenTelemetryScope(serviceIdx, reporter.GetMeter(), nil, clientConfig.Tags, getMetricDefs(serviceIdx), false, gaugeCache, false)

serviceTypeTagValue, err := MetricsServiceIdxToServiceName(serviceIdx)
if err != nil {
return nil, fmt.Errorf("failed to initialize metrics client: %w", err)
}

rootTags := make(map[string]string, len(clientConfig.Tags)+1)
rootTags := make(map[string]string, len(clientConfig.Tags)+2)
for k, v := range clientConfig.Tags {
rootTags[k] = v
}
rootTags[serviceName] = serviceTypeTagValue
rootTags[namespace] = namespaceAllValue
globalRootScope := rootScope.taggedString(rootTags, true)

totalScopes := len(ScopeDefs[Common]) + len(ScopeDefs[serviceIdx])
metricsClient := &openTelemetryClient{
Expand All @@ -75,13 +77,12 @@ func NewOpenTelemetryClient(clientConfig *ClientConfig, serviceIdx ServiceIdx, r
serviceIdx: serviceIdx,
scopeWrapper: scopeWrapper,
gaugeCache: gaugeCache,
userScope: reporter.UserScope(),
userScope: reporter.UserScope().Tagged(rootTags),
}

for idx, def := range ScopeDefs[Common] {
scopeTags := map[string]string{
OperationTagName: def.operation,
namespace: namespaceAllValue,
}
mergeMapToRight(def.tags, scopeTags)
metricsClient.childScopes[idx] = scopeWrapper(globalRootScope.taggedString(scopeTags, true))
Expand All @@ -90,7 +91,6 @@ func NewOpenTelemetryClient(clientConfig *ClientConfig, serviceIdx ServiceIdx, r
for idx, def := range ScopeDefs[serviceIdx] {
scopeTags := map[string]string{
OperationTagName: def.operation,
namespace: namespaceAllValue,
}
mergeMapToRight(def.tags, scopeTags)
metricsClient.childScopes[idx] = scopeWrapper(globalRootScope.taggedString(scopeTags, true))
Expand Down
21 changes: 13 additions & 8 deletions common/metrics/otel_metric_test_utility.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ type (
TestOtelReporter struct {
// TODO: https://github.com/open-telemetry/opentelemetry-go/issues/2722
controller *controller.Controller
userScope UserScope
}

OtelMetricTestUtility struct {
Expand Down Expand Up @@ -231,14 +232,18 @@ func (t *OtelMetricTestUtility) labelsMatch(left []attribute.KeyValue, right map
}

func NewTestOtelReporter() *TestOtelReporter {
return &TestOtelReporter{
controller: controller.New(
processor.NewFactory(
selector.NewWithHistogramDistribution(),
aggregation.CumulativeTemporalitySelector(),
),
controller.WithCollectPeriod(0),
ctr := controller.New(
processor.NewFactory(
selector.NewWithHistogramDistribution(),
aggregation.CumulativeTemporalitySelector(),
),
controller.WithCollectPeriod(0),
)
meter := ctr.Meter("")
gaugeCache := NewOtelGaugeCache(meter)
return &TestOtelReporter{
controller: ctr,
userScope: NewOpenTelemetryUserScope(meter, nil, gaugeCache),
}
}

Expand All @@ -253,5 +258,5 @@ func (t TestOtelReporter) NewClient(logger log.Logger, serviceIdx ServiceIdx) (C
func (t TestOtelReporter) Stop(logger log.Logger) {}

func (t TestOtelReporter) UserScope() UserScope {
return NoopUserScope
return t.userScope
}
6 changes: 2 additions & 4 deletions common/metrics/tally_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewClient(clientConfig *ClientConfig, scope tally.Scope, serviceIdx Service
return nil, fmt.Errorf("failed to initialize metrics client: %w", err)
}

rootScope := scope.Tagged(map[string]string{serviceName: serviceTypeTagValue})
rootScope := scope.Tagged(map[string]string{serviceName: serviceTypeTagValue, namespace: namespaceAllValue})

totalScopes := len(ScopeDefs[Common]) + len(ScopeDefs[serviceIdx])
metricsClient := &TallyClient{
Expand All @@ -76,13 +76,12 @@ func NewClient(clientConfig *ClientConfig, scope tally.Scope, serviceIdx Service
serviceIdx: serviceIdx,
scopeWrapper: scopeWrapper,
perUnitBuckets: perUnitBuckets,
userScope: newTallyUserScope(clientConfig, scope),
userScope: newTallyUserScope(clientConfig, rootScope),
}

for idx, def := range ScopeDefs[Common] {
scopeTags := map[string]string{
OperationTagName: def.operation,
namespace: namespaceAllValue,
}
mergeMapToRight(def.tags, scopeTags)
metricsClient.childScopes[idx] = rootScope.Tagged(scopeTags)
Expand All @@ -91,7 +90,6 @@ func NewClient(clientConfig *ClientConfig, scope tally.Scope, serviceIdx Service
for idx, def := range ScopeDefs[serviceIdx] {
scopeTags := map[string]string{
OperationTagName: def.operation,
namespace: namespaceAllValue,
}
mergeMapToRight(def.tags, scopeTags)
metricsClient.childScopes[idx] = rootScope.Tagged(scopeTags)
Expand Down

0 comments on commit 4faf1f4

Please sign in to comment.