Skip to content

Commit

Permalink
make useOtelForInternalMetrics as beta (#9037)
Browse files Browse the repository at this point in the history
The metrics are now consistent with the metrics produced by OpenCensus.
We should move the featuregate forward.

Note that the OpenTelemetry generated metrics includes grpc
client/server metrics (for receivers/exporters that use grpc) and
`target_info` metrics

Fixes
#7454

---------

Signed-off-by: Alex Boten <[email protected]>
  • Loading branch information
Alex Boten committed Dec 12, 2023
1 parent 3495332 commit a158ff3
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 40 deletions.
29 changes: 29 additions & 0 deletions .chloggen/codeboten_make-otel-default.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: service

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: "Enable `telemetry.useOtelForInternalMetrics` by updating the flag to beta"

# One or more tracking issues or pull requests related to the change
issues: [7454]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext: |
The metrics generated should be consistent with the metrics generated
previously with OpenCensus. Users can disable the behaviour
by setting `--feature-gates -telemetry.useOtelForInternalMetrics` at
collector start.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
10 changes: 5 additions & 5 deletions exporter/exporterhelper/obsexporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,11 @@ type testParams struct {

func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt obsreporttest.TestTelemetry, useOtel bool)) {
t.Run("WithOC", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand All @@ -188,11 +193,6 @@ func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt
})

t.Run("WithOTel", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), true))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand Down
8 changes: 8 additions & 0 deletions exporter/exporterhelper/obsreport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/exporter"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/internal/obsreportconfig"
"go.opentelemetry.io/collector/obsreport/obsreporttest"
)

Expand All @@ -26,6 +28,12 @@ func TestExportEnqueueFailure(t *testing.T) {
})
require.NoError(t, err)

originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()

logRecords := int64(7)
obsrep.recordEnqueueFailureWithOC(context.Background(), component.DataTypeLogs, logRecords)
require.NoError(t, tt.CheckExporterEnqueueFailedLogs(logRecords))
Expand Down
5 changes: 2 additions & 3 deletions exporter/exporterhelper/queue_sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ func setFeatureGateForTest(t testing.TB, gate *featuregate.Gate, enabled bool) f
}

func TestQueuedRetry_QueueMetricsReported(t *testing.T) {
resetFlag := setFeatureGateForTest(t, obsreportconfig.UseOtelForInternalMetricsfeatureGate, false)
defer resetFlag()
tt, err := obsreporttest.SetupTelemetry(defaultID)
require.NoError(t, err)

Expand All @@ -166,9 +168,6 @@ func TestQueuedRetry_QueueMetricsReported(t *testing.T) {
}

func TestQueuedRetry_QueueMetricsReportedUsingOTel(t *testing.T) {
resetFlag := setFeatureGateForTest(t, obsreportconfig.UseOtelForInternalMetricsfeatureGate, true)
defer resetFlag()

tt, err := obsreporttest.SetupTelemetry(defaultID)
require.NoError(t, err)

Expand Down
2 changes: 1 addition & 1 deletion internal/obsreportconfig/obsreportconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (
// telemetrySettings for internal metrics.
var UseOtelForInternalMetricsfeatureGate = featuregate.GlobalRegistry().MustRegister(
"telemetry.useOtelForInternalMetrics",
featuregate.StageAlpha,
featuregate.StageBeta,
featuregate.WithRegisterDescription("controls whether the collector uses OpenTelemetry for internal metrics"))

// DisableHighCardinalityMetricsfeatureGate is the feature gate that controls whether the collector should enable
Expand Down
7 changes: 7 additions & 0 deletions internal/obsreportconfig/obsreportconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,18 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.opentelemetry.io/collector/config/configtelemetry"
"go.opentelemetry.io/collector/featuregate"
)

func TestConfigure(t *testing.T) {
originalValue := UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tests := []struct {
name string
level configtelemetry.Level
Expand Down
10 changes: 5 additions & 5 deletions processor/processorhelper/obsreport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ func TestProcessorLogRecords(t *testing.T) {

func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt obsreporttest.TestTelemetry, useOtel bool)) {
t.Run("WithOC", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand All @@ -109,11 +114,6 @@ func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt
})

t.Run("WithOTel", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), true))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand Down
31 changes: 15 additions & 16 deletions receiver/otlpreceiver/otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"go.opentelemetry.io/collector/config/configgrpc"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/config/configtelemetry"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/consumertest"
Expand Down Expand Up @@ -179,7 +178,7 @@ func TestJsonHttp(t *testing.T) {

// Set the buffer count to 1 to make it flush the test span immediately.
sink := &errOrSinkConsumer{TracesSink: new(consumertest.TracesSink)}
ocr := newHTTPReceiver(t, addr, tracesURLPath, metricsURLPath, logsURLPath, sink, nil)
ocr := newHTTPReceiver(t, componenttest.NewNopTelemetrySettings(), addr, tracesURLPath, metricsURLPath, logsURLPath, sink, nil)

require.NoError(t, ocr.Start(context.Background(), componenttest.NewNopHost()), "Failed to start trace receiver")
t.Cleanup(func() { require.NoError(t, ocr.Shutdown(context.Background())) })
Expand Down Expand Up @@ -469,7 +468,7 @@ func TestProtoHttp(t *testing.T) {

// Set the buffer count to 1 to make it flush the test span immediately.
tSink := &errOrSinkConsumer{TracesSink: new(consumertest.TracesSink)}
ocr := newHTTPReceiver(t, addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, tSink, consumertest.NewNop())
ocr := newHTTPReceiver(t, componenttest.NewNopTelemetrySettings(), addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, tSink, consumertest.NewNop())

require.NoError(t, ocr.Start(context.Background(), componenttest.NewNopHost()), "Failed to start trace receiver")
t.Cleanup(func() { require.NoError(t, ocr.Shutdown(context.Background())) })
Expand Down Expand Up @@ -617,7 +616,7 @@ func TestOTLPReceiverInvalidContentEncoding(t *testing.T) {
// Set the buffer count to 1 to make it flush the test span immediately.
tSink := new(consumertest.TracesSink)
mSink := new(consumertest.MetricsSink)
ocr := newHTTPReceiver(t, addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, tSink, mSink)
ocr := newHTTPReceiver(t, componenttest.NewNopTelemetrySettings(), addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, tSink, mSink)

require.NoError(t, ocr.Start(context.Background(), componenttest.NewNopHost()), "Failed to start trace receiver")
t.Cleanup(func() { require.NoError(t, ocr.Shutdown(context.Background())) })
Expand Down Expand Up @@ -662,7 +661,7 @@ func TestGRPCNewPortAlreadyUsed(t *testing.T) {
assert.NoError(t, ln.Close())
})

r := newGRPCReceiver(t, addr, consumertest.NewNop(), consumertest.NewNop())
r := newGRPCReceiver(t, componenttest.NewNopTelemetrySettings(), addr, consumertest.NewNop(), consumertest.NewNop())
require.NotNil(t, r)

require.Error(t, r.Start(context.Background(), componenttest.NewNopHost()))
Expand All @@ -676,7 +675,7 @@ func TestHTTPNewPortAlreadyUsed(t *testing.T) {
assert.NoError(t, ln.Close())
})

r := newHTTPReceiver(t, addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, consumertest.NewNop(), consumertest.NewNop())
r := newHTTPReceiver(t, componenttest.NewNopTelemetrySettings(), addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, consumertest.NewNop(), consumertest.NewNop())
require.NotNil(t, r)

require.Error(t, r.Start(context.Background(), componenttest.NewNopHost()))
Expand Down Expand Up @@ -720,7 +719,7 @@ func TestOTLPReceiverGRPCTracesIngestTest(t *testing.T) {

sink := &errOrSinkConsumer{TracesSink: new(consumertest.TracesSink)}

ocr := newGRPCReceiver(t, addr, sink, nil)
ocr := newGRPCReceiver(t, tt.TelemetrySettings, addr, sink, nil)
require.NotNil(t, ocr)
require.NoError(t, ocr.Start(context.Background(), componenttest.NewNopHost()))
t.Cleanup(func() { require.NoError(t, ocr.Shutdown(context.Background())) })
Expand Down Expand Up @@ -787,7 +786,7 @@ func TestOTLPReceiverHTTPTracesIngestTest(t *testing.T) {

sink := &errOrSinkConsumer{TracesSink: new(consumertest.TracesSink)}

ocr := newHTTPReceiver(t, addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, sink, nil)
ocr := newHTTPReceiver(t, tt.TelemetrySettings, addr, defaultTracesURLPath, defaultMetricsURLPath, defaultLogsURLPath, sink, nil)
require.NotNil(t, ocr)
require.NoError(t, ocr.Start(context.Background(), componenttest.NewNopHost()))
t.Cleanup(func() { require.NoError(t, ocr.Shutdown(context.Background())) })
Expand Down Expand Up @@ -865,7 +864,7 @@ func TestGRPCMaxRecvSize(t *testing.T) {
cfg := factory.CreateDefaultConfig().(*Config)
cfg.GRPC.NetAddr.Endpoint = addr
cfg.HTTP = nil
ocr := newReceiver(t, factory, cfg, otlpReceiverID, sink, nil)
ocr := newReceiver(t, componenttest.NewNopTelemetrySettings(), factory, cfg, otlpReceiverID, sink, nil)

require.NotNil(t, ocr)
require.NoError(t, ocr.Start(context.Background(), componenttest.NewNopHost()))
Expand All @@ -879,7 +878,7 @@ func TestGRPCMaxRecvSize(t *testing.T) {
require.NoError(t, ocr.Shutdown(context.Background()))

cfg.GRPC.MaxRecvMsgSizeMiB = 100
ocr = newReceiver(t, factory, cfg, otlpReceiverID, sink, nil)
ocr = newReceiver(t, componenttest.NewNopTelemetrySettings(), factory, cfg, otlpReceiverID, sink, nil)

require.NotNil(t, ocr)
require.NoError(t, ocr.Start(context.Background(), componenttest.NewNopHost()))
Expand Down Expand Up @@ -976,28 +975,28 @@ func TestHTTPMaxRequestBodySize_TooLarge(t *testing.T) {
testHTTPMaxRequestBodySizeJSON(t, traceJSON, len(traceJSON)-1, 400)
}

func newGRPCReceiver(t *testing.T, endpoint string, tc consumer.Traces, mc consumer.Metrics) component.Component {
func newGRPCReceiver(t *testing.T, settings component.TelemetrySettings, endpoint string, tc consumer.Traces, mc consumer.Metrics) component.Component {
factory := NewFactory()
cfg := factory.CreateDefaultConfig().(*Config)
cfg.GRPC.NetAddr.Endpoint = endpoint
cfg.HTTP = nil
return newReceiver(t, factory, cfg, otlpReceiverID, tc, mc)
return newReceiver(t, settings, factory, cfg, otlpReceiverID, tc, mc)
}

func newHTTPReceiver(t *testing.T, endpoint string, tracesURLPath string, metricsURLPath string, logsURLPath string, tc consumer.Traces, mc consumer.Metrics) component.Component {
func newHTTPReceiver(t *testing.T, settings component.TelemetrySettings, endpoint string, tracesURLPath string, metricsURLPath string, logsURLPath string, tc consumer.Traces, mc consumer.Metrics) component.Component {
factory := NewFactory()
cfg := factory.CreateDefaultConfig().(*Config)
cfg.HTTP.Endpoint = endpoint
cfg.HTTP.TracesURLPath = tracesURLPath
cfg.HTTP.MetricsURLPath = metricsURLPath
cfg.HTTP.LogsURLPath = logsURLPath
cfg.GRPC = nil
return newReceiver(t, factory, cfg, otlpReceiverID, tc, mc)
return newReceiver(t, settings, factory, cfg, otlpReceiverID, tc, mc)
}

func newReceiver(t *testing.T, factory receiver.Factory, cfg *Config, id component.ID, tc consumer.Traces, mc consumer.Metrics) component.Component {
func newReceiver(t *testing.T, settings component.TelemetrySettings, factory receiver.Factory, cfg *Config, id component.ID, tc consumer.Traces, mc consumer.Metrics) component.Component {
set := receivertest.NewNopCreateSettings()
set.TelemetrySettings.MetricsLevel = configtelemetry.LevelNormal
set.TelemetrySettings = settings
set.ID = id
var r component.Component
var err error
Expand Down
10 changes: 5 additions & 5 deletions receiver/receiverhelper/obsreport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,11 @@ func TestReceiveWithLongLivedCtx(t *testing.T) {

func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt obsreporttest.TestTelemetry, useOtel bool)) {
t.Run("WithOC", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand All @@ -245,11 +250,6 @@ func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt
})

t.Run("WithOTel", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), true))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand Down
10 changes: 5 additions & 5 deletions receiver/scraperhelper/obsreport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ func TestScrapeMetricsDataOp(t *testing.T) {

func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt obsreporttest.TestTelemetry, useOtel bool)) {
t.Run("WithOC", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), false))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand All @@ -102,11 +107,6 @@ func testTelemetry(t *testing.T, id component.ID, testFunc func(t *testing.T, tt
})

t.Run("WithOTel", func(t *testing.T) {
originalValue := obsreportconfig.UseOtelForInternalMetricsfeatureGate.IsEnabled()
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), true))
defer func() {
require.NoError(t, featuregate.GlobalRegistry().Set(obsreportconfig.UseOtelForInternalMetricsfeatureGate.ID(), originalValue))
}()
tt, err := obsreporttest.SetupTelemetry(id)
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })
Expand Down

0 comments on commit a158ff3

Please sign in to comment.