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 7 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
It no longer is a conglomerate of itself, events, and link attributes that have been dropped. (#1771)
- Make `ExportSpans` in Jaeger Exporter honor context deadline. (#1773)
- The `go.opentelemetry.io/otel/sdk/export/trace` package is merged into the `go.opentelemetry.io/otel/sdk/trace` package. (#1778)
- Resources default detectors are overwritten by using `WithDetectors()` (#1810)

### Removed

Expand All @@ -89,6 +90,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- The `trace.FlagsDebug` and `trace.FlagsDeferred` constants have been removed and will be localized to the B3 propagator. (#1770)
- Remove `Process` configuration, `WithProcessFromEnv` and `ProcessFromEnv`, from the Jaeger exporter package.
The information that could be configured in the `Process` struct should be configured in a `Resource` instead. (#1776)
- Remove `resource.WithoutBuiltin()`. Any use of `resource.WithDetecors()` will have the same effect (#1810)
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
- Remove `resource.WithHost()`, `resource.WithTelemetrySDK()`, and `resource.WithFromEnv()`. To replace these effects use `resource.WithDetectors()` (#1810)

## [0.19.0] - 2021-03-18

Expand Down
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
// 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
115 changes: 29 additions & 86 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.
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved

// `New(ctx, WithDetectors(d1, d2))`: Use Detector `d1`, then overlay Detector `d2`.
// ```
// 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{}}}
}
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.
// user-provided detectors or builtin detectors.
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
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...)
}
164 changes: 89 additions & 75 deletions sdk/resource/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,103 +23,117 @@ 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()) }()
func TestConfig(t *testing.T) {
tc := []struct {
name string
envars string
detectors []resource.Detector

resouceValues map[string]string
MadVikingGod marked this conversation as resolved.
Show resolved Hide resolved
}{
{
name: "No detectors disables detection",
envars: "key=value,other=attr",
detectors: []resource.Detector{},
resouceValues: map[string]string{},
},
{
name: "Nil detectors disables detection",
envars: "key=value,other=attr",
detectors: nil,
resouceValues: map[string]string{},
},
{
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: "Builtins",
envars: "key=value,other=attr",
detectors: resource.BuiltinDetectors,
resouceValues: map[string]string{
"host.name": hostname(),
"telemetry.sdk.name": "opentelemetry",
"telemetry.sdk.language": "go",
"telemetry.sdk.version": otel.Version(),
"key": "value",
"other": "attr",
},
},
}
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))
})
}
}

ctx := context.Background()
res, err := resource.New(ctx)
require.NoError(t, err)
require.EqualValues(t, map[string]string{
func TestDefaultConfig(t *testing.T) {
resouceValues := 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))
require.EqualValues(t, resouceValues, toMap(res))
}

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