Skip to content

Commit

Permalink
fixes after review
Browse files Browse the repository at this point in the history
  • Loading branch information
@povilasv
povilasv committed Jul 13, 2023
1 parent 431fbfc commit 93fb6a3
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 38 deletions.
2 changes: 1 addition & 1 deletion connector/spanmetricsconnector/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ connectors:
default: GET
- name: http.status_code
exemplars:
enabled: false
enabled: true
exclude_dimensions: ['status.code']
dimensions_cache_size: 1000
aggregation_temporality: "AGGREGATION_TEMPORALITY_CUMULATIVE"
Expand Down
6 changes: 3 additions & 3 deletions connector/spanmetricsconnector/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ type Config struct {
// Namespace is the namespace of the metrics emitted by the connector.
Namespace string `mapstructure:"namespace"`

// ExemplarConfig defines the configuration for exemplars.
ExemplarConfig ExemplarConfig `mapstructure:"exemplar"`
// Exemplars defines the configuration for exemplars.
Exemplars ExemplarsConfig `mapstructure:"exemplars"`
}

type HistogramConfig struct {
Expand All @@ -67,7 +67,7 @@ type HistogramConfig struct {
Explicit *ExplicitHistogramConfig `mapstructure:"explicit"`
}

type ExemplarConfig struct {
type ExemplarsConfig struct {
Enabled bool `mapstructure:"enabled"`
}

Expand Down
12 changes: 11 additions & 1 deletion connector/spanmetricsconnector/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestLoadConfig(t *testing.T) {
},
DimensionsCacheSize: 1500,
MetricsFlushInterval: 30 * time.Second,
ExemplarConfig: ExemplarConfig{
Exemplars: ExemplarsConfig{
Enabled: true,
},
Histogram: HistogramConfig{
Expand Down Expand Up @@ -85,6 +85,16 @@ func TestLoadConfig(t *testing.T) {
id: component.NewIDWithName(metadata.Type, "invalid_histogram_unit"),
errorMessage: "unknown Unit \"h\"",
},
{
id: component.NewIDWithName(metadata.Type, "exemplars_enabled"),
expected: &Config{
AggregationTemporality: "AGGREGATION_TEMPORALITY_CUMULATIVE",
DimensionsCacheSize: defaultDimensionsCacheSize,
MetricsFlushInterval: 15 * time.Second,
Histogram: HistogramConfig{Disable: false, Unit: defaultUnit},
Exemplars: ExemplarsConfig{Enabled: true},
},
},
}

for _, tt := range tests {
Expand Down
2 changes: 1 addition & 1 deletion connector/spanmetricsconnector/connector.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ func (p *connectorImp) aggregateMetrics(traces ptrace.Traces) {
}

func (p *connectorImp) addExemplar(span ptrace.Span, duration float64, h metrics.Histogram) {
if !p.config.ExemplarConfig.Enabled {
if !p.config.Exemplars.Enabled {
return
}
if span.TraceID().IsEmpty() {
Expand Down
50 changes: 25 additions & 25 deletions connector/spanmetricsconnector/connector_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -398,14 +398,14 @@ func initSpan(span span, s ptrace.Span) {
s.SetSpanID(pcommon.SpanID(span.spanID))
}

func disabledExemplarConfig() ExemplarConfig {
return ExemplarConfig{
func disabledExemplarsConfig() ExemplarsConfig {
return ExemplarsConfig{
Enabled: false,
}
}

func enabledExemplarConfig() ExemplarConfig {
return ExemplarConfig{
func enabledExemplarsConfig() ExemplarsConfig {
return ExemplarsConfig{
Enabled: true,
}
}
Expand Down Expand Up @@ -583,7 +583,7 @@ func TestConcurrentShutdown(t *testing.T) {
ticker := mockClock.NewTicker(time.Nanosecond)

// Test
p := newConnectorImp(t, new(consumertest.MetricsSink), nil, explicitHistogramsConfig, disabledExemplarConfig, cumulative, logger, ticker)
p := newConnectorImp(t, new(consumertest.MetricsSink), nil, explicitHistogramsConfig, disabledExemplarsConfig, cumulative, logger, ticker)
err := p.Start(ctx, componenttest.NewNopHost())
require.NoError(t, err)

Expand Down Expand Up @@ -651,7 +651,7 @@ func TestConsumeMetricsErrors(t *testing.T) {

mockClock := clock.NewMock(time.Now())
ticker := mockClock.NewTicker(time.Nanosecond)
p := newConnectorImp(t, mcon, nil, explicitHistogramsConfig, disabledExemplarConfig, cumulative, logger, ticker)
p := newConnectorImp(t, mcon, nil, explicitHistogramsConfig, disabledExemplarsConfig, cumulative, logger, ticker)

ctx := metadata.NewIncomingContext(context.Background(), nil)
err := p.Start(ctx, componenttest.NewNopHost())
Expand Down Expand Up @@ -688,7 +688,7 @@ func TestConsumeTraces(t *testing.T) {
name string
aggregationTemporality string
histogramConfig func() HistogramConfig
exemplarConfig func() ExemplarConfig
exemplarConfig func() ExemplarsConfig
verifier func(t testing.TB, input pmetric.Metrics) bool
traces []ptrace.Traces
}{
Expand All @@ -697,7 +697,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test histogram metrics are disabled",
aggregationTemporality: cumulative,
histogramConfig: disabledHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyDisabledHistogram,
traces: []ptrace.Traces{buildSampleTrace()},
},
Expand All @@ -706,15 +706,15 @@ func TestConsumeTraces(t *testing.T) {
name: "Test single consumption, three spans (Cumulative), using exp. histogram",
aggregationTemporality: cumulative,
histogramConfig: exponentialHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyConsumeMetricsInputCumulative,
traces: []ptrace.Traces{buildSampleTrace()},
},
{
name: "Test single consumption, three spans (Delta), using exp. histogram",
aggregationTemporality: delta,
histogramConfig: exponentialHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyConsumeMetricsInputDelta,
traces: []ptrace.Traces{buildSampleTrace()},
},
Expand All @@ -723,7 +723,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test two consumptions (Cumulative), using exp. histogram",
aggregationTemporality: cumulative,
histogramConfig: exponentialHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyMultipleCumulativeConsumptions(),
traces: []ptrace.Traces{buildSampleTrace(), buildSampleTrace()},
},
Expand All @@ -732,7 +732,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test two consumptions (Delta), using exp. histogram",
aggregationTemporality: delta,
histogramConfig: exponentialHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyConsumeMetricsInputDelta,
traces: []ptrace.Traces{buildSampleTrace(), buildSampleTrace()},
},
Expand All @@ -741,7 +741,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test bad consumptions (Delta), using exp. histogram",
aggregationTemporality: cumulative,
histogramConfig: exponentialHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyBadMetricsOkay,
traces: []ptrace.Traces{buildBadSampleTrace()},
},
Expand All @@ -751,15 +751,15 @@ func TestConsumeTraces(t *testing.T) {
name: "Test single consumption, three spans (Cumulative).",
aggregationTemporality: cumulative,
histogramConfig: explicitHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyConsumeMetricsInputCumulative,
traces: []ptrace.Traces{buildSampleTrace()},
},
{
name: "Test single consumption, three spans (Delta).",
aggregationTemporality: delta,
histogramConfig: explicitHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyConsumeMetricsInputDelta,
traces: []ptrace.Traces{buildSampleTrace()},
},
Expand All @@ -768,7 +768,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test two consumptions (Cumulative).",
aggregationTemporality: cumulative,
histogramConfig: explicitHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyMultipleCumulativeConsumptions(),
traces: []ptrace.Traces{buildSampleTrace(), buildSampleTrace()},
},
Expand All @@ -777,7 +777,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test two consumptions (Delta).",
aggregationTemporality: delta,
histogramConfig: explicitHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyConsumeMetricsInputDelta,
traces: []ptrace.Traces{buildSampleTrace(), buildSampleTrace()},
},
Expand All @@ -786,7 +786,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test bad consumptions (Delta).",
aggregationTemporality: cumulative,
histogramConfig: explicitHistogramsConfig,
exemplarConfig: disabledExemplarConfig,
exemplarConfig: disabledExemplarsConfig,
verifier: verifyBadMetricsOkay,
traces: []ptrace.Traces{buildBadSampleTrace()},
},
Expand All @@ -795,7 +795,7 @@ func TestConsumeTraces(t *testing.T) {
name: "Test Exemplars are enabled",
aggregationTemporality: cumulative,
histogramConfig: explicitHistogramsConfig,
exemplarConfig: enabledExemplarConfig,
exemplarConfig: enabledExemplarsConfig,
verifier: verifyExemplarsExist,
traces: []ptrace.Traces{buildSampleTrace()},
},
Expand Down Expand Up @@ -846,7 +846,7 @@ func TestMetricKeyCache(t *testing.T) {
mcon := &mocks.MetricsConsumer{}
mcon.On("ConsumeMetrics", mock.Anything, mock.Anything).Return(nil)

p := newConnectorImp(t, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarConfig, cumulative, zaptest.NewLogger(t), nil)
p := newConnectorImp(t, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarsConfig, cumulative, zaptest.NewLogger(t), nil)
traces := buildSampleTrace()

// Test
Expand Down Expand Up @@ -878,7 +878,7 @@ func BenchmarkConnectorConsumeTraces(b *testing.B) {
mcon := &mocks.MetricsConsumer{}
mcon.On("ConsumeMetrics", mock.Anything, mock.Anything).Return(nil)

conn := newConnectorImp(nil, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarConfig, cumulative, zaptest.NewLogger(b), nil)
conn := newConnectorImp(nil, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarsConfig, cumulative, zaptest.NewLogger(b), nil)

traces := buildSampleTrace()

Expand All @@ -893,7 +893,7 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) {
mcon := &mocks.MetricsConsumer{}
mcon.On("ConsumeMetrics", mock.Anything, mock.Anything).Return(nil)
excludeDimensions := []string{"span.kind", "span.name", "totallyWrongNameDoesNotAffectAnything"}
p := newConnectorImp(t, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarConfig, cumulative, zaptest.NewLogger(t), nil, excludeDimensions...)
p := newConnectorImp(t, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarsConfig, cumulative, zaptest.NewLogger(t), nil, excludeDimensions...)
traces := buildSampleTrace()

// Test
Expand Down Expand Up @@ -942,12 +942,12 @@ func TestExcludeDimensionsConsumeTraces(t *testing.T) {

}

func newConnectorImp(t *testing.T, mcon consumer.Metrics, defaultNullValue *string, histogramConfig func() HistogramConfig, exemplarConfig func() ExemplarConfig, temporality string, logger *zap.Logger, ticker *clock.Ticker, excludedDimensions ...string) *connectorImp {
func newConnectorImp(t *testing.T, mcon consumer.Metrics, defaultNullValue *string, histogramConfig func() HistogramConfig, exemplarsConfig func() ExemplarsConfig, temporality string, logger *zap.Logger, ticker *clock.Ticker, excludedDimensions ...string) *connectorImp {

cfg := &Config{
AggregationTemporality: temporality,
Histogram: histogramConfig(),
ExemplarConfig: exemplarConfig(),
Exemplars: exemplarsConfig(),
ExcludeDimensions: excludedDimensions,
DimensionsCacheSize: DimensionsCacheSize,
Dimensions: []Dimension{
Expand Down Expand Up @@ -1077,7 +1077,7 @@ func TestConnectorConsumeTracesEvictedCacheKey(t *testing.T) {
ticker := mockClock.NewTicker(time.Nanosecond)

// Note: default dimension key cache size is 2.
p := newConnectorImp(t, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarConfig, cumulative, zaptest.NewLogger(t), ticker)
p := newConnectorImp(t, mcon, stringp("defaultNullValue"), explicitHistogramsConfig, disabledExemplarsConfig, cumulative, zaptest.NewLogger(t), ticker)

ctx := metadata.NewIncomingContext(context.Background(), nil)
err := p.Start(ctx, componenttest.NewNopHost())
Expand Down
10 changes: 3 additions & 7 deletions connector/spanmetricsconnector/testdata/config.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ spanmetrics/full:
unit: "s"
explicit:
buckets: [ 10ms, 100ms, 250ms ]
exemplar:
exemplars:
enabled: true
dimensions_cache_size: 1500

Expand Down Expand Up @@ -57,11 +57,7 @@ spanmetrics/invalid_histogram_unit:
histogram:
unit: "h"

# exemplars disabled
spanmetrics/exemplars_disabled:
# exemplars enabled
spanmetrics/exemplars_enabled:
exemplars:
enabled: true
histogram:
exponential:
max_size: 10

0 comments on commit 93fb6a3

Please sign in to comment.