Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed different types of Detectors for Resources. #1810

Merged
merged 20 commits into from
Apr 29, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion exporters/metric/prometheus/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ func ExampleNewExportPipeline() {
// Create a resource, with builtin attributes plus R=V.
res, err := resource.New(
context.Background(),
resource.WithoutBuiltin(), // Test-only!
resource.WithAttributes(attribute.String("R", "V")),
)
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions sdk/resource/builtin.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ type (
}

defaultServiceNameDetector struct{}

MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
// NoOp is a Detector that only provides an empty resource. Used
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
// to disable automatic detection.
NoOp struct{}
)

var (
Expand Down Expand Up @@ -101,3 +105,8 @@ func (defaultServiceNameDetector) Detect(ctx context.Context) (*Resource, error)
},
).Detect(ctx)
}

// Detector that does nothing. Used to disable default Detector behavior.
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
func (NoOp) Detect(_ context.Context) (*Resource, error) {
return &emptyResource, nil
}
1 change: 0 additions & 1 deletion sdk/resource/builtin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ func TestStringDetectorErrors(t *testing.T) {
for _, test := range tests {
res, err := resource.New(
context.Background(),
resource.WithoutBuiltin(),
resource.WithAttributes(attribute.String("A", "B")),
resource.WithDetectors(test.s),
)
Expand Down
113 changes: 28 additions & 85 deletions sdk/resource/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,6 @@ import (
type config struct {
// detectors that will be evaluated.
detectors []Detector

// telemetrySDK is used to specify non-default
// `telemetry.sdk.*` attributes.
telemetrySDK Detector

// HostResource is used to specify non-default `host.*`
// attributes.
host Detector

// FromEnv is used to specify non-default OTEL_RESOURCE_ATTRIBUTES
// attributes.
fromEnv Detector
}

// Option is the interface that applies a configuration option.
Expand Down Expand Up @@ -67,7 +55,24 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) {
}

// WithDetectors adds detectors to be evaluated for the configured resource.
// Any use of WithDetectors disabled the default behavior, to reenable this
// inlcude a WithDetectors(BuiltinDetectors...),
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
// Examples:
// `New(ctx)`: Use builtin `Detector`s.
// `New(ctx, WithDetectors())`: Use no `Detector`s
// `New(ctx, WithDetectors(NoOp{}))`: Use no `Detector`s
// `New(ctx, WithDetectors(d1, d2))`: Use Detector `d1`, then overlay Detector `d2`
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
// ```
// New(ctx,
// WithDetectors(BuiltinDetectors...),
// WithDetectors(d1),
// )
// ```
// Use The `BuiltinDetectors`, then overlay Detector `d1`
func WithDetectors(detectors ...Detector) Option {
if len(detectors) == 0 {
return detectorsOption{detectors: []Detector{NoOp{}}}
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
}
return detectorsOption{detectors: detectors}
}

Expand All @@ -81,85 +86,23 @@ func (o detectorsOption) Apply(cfg *config) {
cfg.detectors = append(cfg.detectors, o.detectors...)
}

// WithTelemetrySDK overrides the builtin `telemetry.sdk.*`
// attributes. Use nil to disable these attributes entirely.
func WithTelemetrySDK(d Detector) Option {
return telemetrySDKOption{Detector: d}
}

type telemetrySDKOption struct {
option
Detector
}

// Apply implements Option.
func (o telemetrySDKOption) Apply(cfg *config) {
cfg.telemetrySDK = o.Detector
}

// WithHost overrides the builtin `host.*` attributes. Use nil to
// disable these attributes entirely.
func WithHost(d Detector) Option {
return hostOption{Detector: d}
}

type hostOption struct {
option
Detector
}

// Apply implements Option.
func (o hostOption) Apply(cfg *config) {
cfg.host = o.Detector
}

// WithFromEnv overrides the builtin detector for
// OTEL_RESOURCE_ATTRIBUTES. Use nil to disable environment checking.
func WithFromEnv(d Detector) Option {
return fromEnvOption{Detector: d}
}

type fromEnvOption struct {
option
Detector
}

// Apply implements Option.
func (o fromEnvOption) Apply(cfg *config) {
cfg.fromEnv = o.Detector
}

// WithoutBuiltin disables all the builtin detectors, including the
// telemetry.sdk.*, host.*, and the environment detector.
func WithoutBuiltin() Option {
return noBuiltinOption{}
}

type noBuiltinOption struct {
option
}

// Apply implements Option.
func (o noBuiltinOption) Apply(cfg *config) {
cfg.host = nil
cfg.telemetrySDK = nil
cfg.fromEnv = nil
var BuiltinDetectors = []Detector{
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
TelemetrySDK{},
Host{},
FromEnv{},
}

// New returns a Resource combined from the provided attributes,
// user-provided detectors and builtin detectors.
func New(ctx context.Context, opts ...Option) (*Resource, error) {
cfg := config{
telemetrySDK: TelemetrySDK{},
host: Host{},
fromEnv: FromEnv{},
}
cfg := config{}
for _, opt := range opts {
opt.Apply(&cfg)
}
detectors := append(
[]Detector{cfg.telemetrySDK, cfg.host, cfg.fromEnv},
cfg.detectors...,
)
return Detect(ctx, detectors...)

if cfg.detectors == nil {
cfg.detectors = BuiltinDetectors
}

return Detect(ctx, cfg.detectors...)
}
165 changes: 75 additions & 90 deletions sdk/resource/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,103 +23,88 @@ import (
"github.com/stretchr/testify/require"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
ottest "go.opentelemetry.io/otel/internal/internaltest"
"go.opentelemetry.io/otel/sdk/resource"
)

const envVar = "OTEL_RESOURCE_ATTRIBUTES"

func TestDefaultConfig(t *testing.T) {
store, err := ottest.SetEnvVariables(map[string]string{
envVar: "",
})
require.NoError(t, err)
defer func() { require.NoError(t, store.Restore()) }()

ctx := context.Background()
res, err := resource.New(ctx)
require.NoError(t, err)
require.EqualValues(t, map[string]string{
"host.name": hostname(),
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "go",
"telemetry.sdk.version": otel.Version(),
}, toMap(res))
}

func TestDefaultConfigNoHost(t *testing.T) {
store, err := ottest.SetEnvVariables(map[string]string{
envVar: "",
})
require.NoError(t, err)
defer func() { require.NoError(t, store.Restore()) }()

ctx := context.Background()
res, err := resource.New(ctx, resource.WithHost(nil))
require.NoError(t, err)
require.EqualValues(t, map[string]string{
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "go",
"telemetry.sdk.version": otel.Version(),
}, toMap(res))
}

func TestDefaultConfigNoEnv(t *testing.T) {
store, err := ottest.SetEnvVariables(map[string]string{
envVar: "from=here",
})
require.NoError(t, err)
defer func() { require.NoError(t, store.Restore()) }()

ctx := context.Background()
res, err := resource.New(ctx, resource.WithFromEnv(nil))
require.NoError(t, err)
require.EqualValues(t, map[string]string{
"host.name": hostname(),
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "go",
"telemetry.sdk.version": otel.Version(),
}, toMap(res))
}

func TestDefaultConfigWithEnv(t *testing.T) {
store, err := ottest.SetEnvVariables(map[string]string{
envVar: "key=value,other=attr",
})
require.NoError(t, err)
defer func() { require.NoError(t, store.Restore()) }()

ctx := context.Background()
res, err := resource.New(ctx)
require.NoError(t, err)
require.EqualValues(t, map[string]string{
"key": "value",
"other": "attr",
"host.name": hostname(),
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "go",
"telemetry.sdk.version": otel.Version(),
}, toMap(res))
}

func TestWithoutBuiltin(t *testing.T) {
store, err := ottest.SetEnvVariables(map[string]string{
envVar: "key=value,other=attr",
})
require.NoError(t, err)
defer func() { require.NoError(t, store.Restore()) }()

ctx := context.Background()
res, err := resource.New(
ctx,
resource.WithoutBuiltin(),
resource.WithAttributes(attribute.String("hello", "collector")),
)
require.NoError(t, err)
require.EqualValues(t, map[string]string{
"hello": "collector",
}, toMap(res))
tc := []struct {
name string
envars string
detectors []resource.Detector

resouceValues map[string]string
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
}{
{
name: "DefaultConfig",
envars: "",
resouceValues: map[string]string{
"host.name": hostname(),
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "go",
"telemetry.sdk.version": otel.Version(),
},
},
{
name: "Only Host",
envars: "from=here",
detectors: []resource.Detector{
resource.Host{},
},
resouceValues: map[string]string{
"host.name": hostname(),
},
},
{
name: "Only Env",
envars: "key=value,other=attr",
detectors: []resource.Detector{
resource.FromEnv{},
},
resouceValues: map[string]string{
"key": "value",
"other": "attr",
},
},
{
name: "Only TelemetrySDK",
envars: "",
detectors: []resource.Detector{
resource.TelemetrySDK{},
},
resouceValues: map[string]string{
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "go",
"telemetry.sdk.version": otel.Version(),
},
},
{
name: "Disable Detectors",
envars: "key=value,other=attr",
detectors: []resource.Detector{
resource.NoOp{},
},
resouceValues: map[string]string{},
},
}
for _, tt := range tc {
t.Run(tt.name, func(t *testing.T) {
store, err := ottest.SetEnvVariables(map[string]string{
envVar: tt.envars,
})
require.NoError(t, err)
defer func() { require.NoError(t, store.Restore()) }()

ctx := context.Background()
res, err := resource.New(ctx,
resource.WithDetectors(tt.detectors...),
)
require.NoError(t, err)
require.EqualValues(t, tt.resouceValues, toMap(res))
})
}
}

func toMap(res *resource.Resource) map[string]string {
Expand Down