Skip to content

Commit

Permalink
[service] Start refactor of GetExporters (#11249)
Browse files Browse the repository at this point in the history
#### Description
As part of
#11204 I
left `host`'s implementation of `GetExporters` alone, thinking I didn't
need to change a deprecated interface implementation.

But `GetExporters` depends on `component.DataType`, so we won't be able
to remove `component.DataType` unless `GetExporters` is updated to use
`pipeline.Signal` instead.

This PR deprecates the deprecated `GetExporters` interface with
`GetExportersWithSignal`, which uses `pipeline.Signal`. Then we'll
follow the process to rename `GetExportersWithSignal` back to
`GetExporters`.
  • Loading branch information
TylerHelmuth authored Sep 23, 2024
1 parent fe10446 commit 20f73e2
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 1 deletion.
25 changes: 25 additions & 0 deletions .chloggen/rename-getexporters.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Use this changelog template to create an entry for release notes.

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

# 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: Deprecates service's implementation of `GetExporters` interface. Use `GetExportersWithSignal` instead.

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

# (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:

# 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: [api]
2 changes: 1 addition & 1 deletion service/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ require (
go.opentelemetry.io/collector/extension/zpagesextension v0.109.0
go.opentelemetry.io/collector/featuregate v1.15.0
go.opentelemetry.io/collector/internal/globalgates v0.109.0
go.opentelemetry.io/collector/internal/globalsignal v0.0.0-20240923154032-388e56cdb156
go.opentelemetry.io/collector/pdata v1.15.0
go.opentelemetry.io/collector/pdata/pprofile v0.109.0
go.opentelemetry.io/collector/pdata/testdata v0.109.0
Expand Down Expand Up @@ -95,7 +96,6 @@ require (
go.opentelemetry.io/collector/config/configtls v1.15.0 // indirect
go.opentelemetry.io/collector/config/internal v0.109.1-0.20240916143658-74729e731d3b // indirect
go.opentelemetry.io/collector/extension/auth v0.109.0 // indirect
go.opentelemetry.io/collector/internal/globalsignal v0.0.0-20240923154032-388e56cdb156 // indirect
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.55.0 // indirect
go.opentelemetry.io/contrib/zpages v0.55.0 // indirect
go.opentelemetry.io/otel/exporters/otlp/otlplog/otlploghttp v0.6.0 // indirect
Expand Down
20 changes: 20 additions & 0 deletions service/internal/graph/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"go.opentelemetry.io/collector/component/componentstatus"
"go.opentelemetry.io/collector/extension"
"go.opentelemetry.io/collector/featuregate"
"go.opentelemetry.io/collector/internal/globalsignal"
"go.opentelemetry.io/collector/pipeline"
"go.opentelemetry.io/collector/service/extensions"
"go.opentelemetry.io/collector/service/internal/builders"
Expand All @@ -28,7 +29,15 @@ type getExporters interface {
GetExporters() map[component.DataType]map[component.ID]component.Component
}

// TODO: remove as part of https://github.com/open-telemetry/opentelemetry-collector/issues/7370 for service 1.0
//
// nolint
type getExportersWithSignal interface {
GetExportersWithSignal() map[globalsignal.Signal]map[component.ID]component.Component
}

var _ getExporters = (*Host)(nil)
var _ getExportersWithSignal = (*Host)(nil)
var _ component.Host = (*Host)(nil)

type Host struct {
Expand Down Expand Up @@ -74,6 +83,7 @@ func (host *Host) GetExtensions() map[component.ID]component.Component {
// connector. See https://github.com/open-telemetry/opentelemetry-collector/issues/7370 and
// https://github.com/open-telemetry/opentelemetry-collector/pull/7390#issuecomment-1483710184
// for additional information.
// If you still need this, use GetExportersWithSignal instead.
// nolint
func (host *Host) GetExporters() map[component.DataType]map[component.ID]component.Component {
exporters := host.Pipelines.GetExporters()
Expand All @@ -84,6 +94,16 @@ func (host *Host) GetExporters() map[component.DataType]map[component.ID]compone
return exportersMap
}

// Deprecated: [0.79.0] This function will be removed in the future.
// Several components in the contrib repository use this function so it cannot be removed
// before those cases are removed. In most cases, use of this function can be replaced by a
// connector. See https://github.com/open-telemetry/opentelemetry-collector/issues/7370 and
// https://github.com/open-telemetry/opentelemetry-collector/pull/7390#issuecomment-1483710184
// for additional information.
func (host *Host) GetExportersWithSignal() map[pipeline.Signal]map[component.ID]component.Component {
return host.Pipelines.GetExporters()
}

// nolint
func convertSignalToDataType(signal pipeline.Signal) component.DataType {
switch signal {
Expand Down
27 changes: 27 additions & 0 deletions service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,33 @@ func TestServiceGetExporters(t *testing.T) {
assert.Contains(t, expMap[componentprofiles.DataTypeProfiles], component.NewID(nopType))
}

// nolint
func TestServiceGetExportersWithSignal(t *testing.T) {
srv, err := New(context.Background(), newNopSettings(), newNopConfig())
require.NoError(t, err)

assert.NoError(t, srv.Start(context.Background()))
t.Cleanup(func() {
assert.NoError(t, srv.Shutdown(context.Background()))
})

expMap := srv.host.GetExportersWithSignal()

v, ok := expMap[pipeline.SignalTraces]
assert.True(t, ok)
assert.NotNil(t, v)

assert.Len(t, expMap, 4)
assert.Len(t, expMap[pipeline.SignalTraces], 1)
assert.Contains(t, expMap[pipeline.SignalTraces], component.NewID(nopType))
assert.Len(t, expMap[pipeline.SignalMetrics], 1)
assert.Contains(t, expMap[pipeline.SignalMetrics], component.NewID(nopType))
assert.Len(t, expMap[pipeline.SignalLogs], 1)
assert.Contains(t, expMap[pipeline.SignalLogs], component.NewID(nopType))
assert.Len(t, expMap[componentprofiles.SignalProfiles], 1)
assert.Contains(t, expMap[componentprofiles.SignalProfiles], component.NewID(nopType))
}

// TestServiceTelemetryCleanupOnError tests that if newService errors due to an invalid config telemetry is cleaned up
// and another service with a valid config can be started right after.
func TestServiceTelemetryCleanupOnError(t *testing.T) {
Expand Down

0 comments on commit 20f73e2

Please sign in to comment.