From 8b2edde4d54282aa33fb46675448b63dac0d5967 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 14 Apr 2021 02:44:31 +0000 Subject: [PATCH 01/15] Removed different types of Detectors for Resources. This change simplifies different types of collectors into one list. The order of this list determines how they are applied. Defaults are applied when the user does not supply any detectors. To achieve default behavior and additional behavior a DefaultDetectors struct has been created --- sdk/resource/builtin.go | 10 +++ sdk/resource/builtin_test.go | 1 - sdk/resource/config.go | 98 +++------------------ sdk/resource/config_test.go | 165 ++++++++++++++++------------------- 4 files changed, 98 insertions(+), 176 deletions(-) diff --git a/sdk/resource/builtin.go b/sdk/resource/builtin.go index c80ab697c6f..cb8fb5c1bf1 100644 --- a/sdk/resource/builtin.go +++ b/sdk/resource/builtin.go @@ -46,6 +46,10 @@ type ( } defaultServiceNameDetector struct{} + + // NoOp is a Detector that only provides an empty resource. Used + // to disable automatic detection. + NoOp struct{} ) var ( @@ -101,3 +105,9 @@ func (defaultServiceNameDetector) Detect(ctx context.Context) (*Resource, error) }, ).Detect(ctx) } + +// Detector that does nothing. Used to disable other builtin Detectors. +// Detect implements Detector +func (NoOp) Detect(_ context.Context) (*Resource, error) { + return &emptyResource, nil +} diff --git a/sdk/resource/builtin_test.go b/sdk/resource/builtin_test.go index 0e93aa17828..223bd0df800 100644 --- a/sdk/resource/builtin_test.go +++ b/sdk/resource/builtin_test.go @@ -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), ) diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 0365caa26a8..317cc7c4b00 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -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. @@ -67,6 +55,8 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) { } // WithDetectors adds detectors to be evaluated for the configured resource. +// If no detectors are passed, a resource will default to use BuiltinDetectors. +// To disable this behavior use a NoOp Detector func WithDetectors(detectors ...Detector) Option { return detectorsOption{detectors: detectors} } @@ -81,85 +71,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{ + 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...) } diff --git a/sdk/resource/config_test.go b/sdk/resource/config_test.go index 046a39bd3fa..35ac6f06b3f 100644 --- a/sdk/resource/config_test.go +++ b/sdk/resource/config_test.go @@ -23,7 +23,6 @@ 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" ) @@ -31,95 +30,81 @@ import ( 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 + }{ + { + 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 { From 7ab5d43a2ab05b0fce9cd37f42eb1e67c7811f32 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 14 Apr 2021 14:10:04 +0000 Subject: [PATCH 02/15] missed prometheus test. --- exporters/metric/prometheus/example_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/exporters/metric/prometheus/example_test.go b/exporters/metric/prometheus/example_test.go index d5259b98ca5..d672a4e2ae7 100644 --- a/exporters/metric/prometheus/example_test.go +++ b/exporters/metric/prometheus/example_test.go @@ -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 { From 1b5330d5c2563b381fc9d631632d82dc72757638 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Thu, 15 Apr 2021 14:52:45 +0000 Subject: [PATCH 03/15] Changed behavior around WithDetectors(nil) Added examples of use of WithDetectors. --- sdk/resource/builtin.go | 3 +-- sdk/resource/config.go | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/sdk/resource/builtin.go b/sdk/resource/builtin.go index cb8fb5c1bf1..eed3e86b3a9 100644 --- a/sdk/resource/builtin.go +++ b/sdk/resource/builtin.go @@ -106,8 +106,7 @@ func (defaultServiceNameDetector) Detect(ctx context.Context) (*Resource, error) ).Detect(ctx) } -// Detector that does nothing. Used to disable other builtin Detectors. -// Detect implements Detector +// Detector that does nothing. Used to disable default Detector behavior. func (NoOp) Detect(_ context.Context) (*Resource, error) { return &emptyResource, nil } diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 317cc7c4b00..c88e10806b0 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -55,9 +55,23 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) { } // WithDetectors adds detectors to be evaluated for the configured resource. -// If no detectors are passed, a resource will default to use BuiltinDetectors. -// To disable this behavior use a NoOp Detector +// Any use of WithDetectors disabled the default behavior, to reenable this +// inlcude a WithDetectors(BuiltinDetectors...), +// Examples: +// `New(ctx)`: Use builtin `Detector`s. +// `New(ctx, WithDetectors())`: Use no `Detector`s +// `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} } From b48a6c32c89bd72c01accdf69fb161e5fc36cd24 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Thu, 15 Apr 2021 14:53:52 +0000 Subject: [PATCH 04/15] Added NoOp example --- sdk/resource/config.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/resource/config.go b/sdk/resource/config.go index c88e10806b0..8ba59d30526 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -60,6 +60,7 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) { // 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` // ``` // New(ctx, From 449a733e48dc91d033974c339bc584172920af62 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Thu, 15 Apr 2021 20:46:30 +0000 Subject: [PATCH 05/15] Changed test to reflect acutal default case This changes because WithDetector() no longer is the same as not using WithDetector() --- sdk/resource/config_test.go | 42 +++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/sdk/resource/config_test.go b/sdk/resource/config_test.go index 35ac6f06b3f..47be01e4353 100644 --- a/sdk/resource/config_test.go +++ b/sdk/resource/config_test.go @@ -29,7 +29,7 @@ import ( const envVar = "OTEL_RESOURCE_ATTRIBUTES" -func TestDefaultConfig(t *testing.T) { +func TestConfig(t *testing.T) { tc := []struct { name string envars string @@ -38,14 +38,16 @@ func TestDefaultConfig(t *testing.T) { resouceValues map[string]string }{ { - name: "DefaultConfig", - envars: "", - resouceValues: map[string]string{ - "host.name": hostname(), - "telemetry.sdk.name": "opentelemetry", - "telemetry.sdk.language": "go", - "telemetry.sdk.version": otel.Version(), - }, + name: "No detectors", + envars: "", + detectors: []resource.Detector{}, + resouceValues: map[string]string{}, + }, + { + name: "Nil detectors", + envars: "", + detectors: nil, + resouceValues: map[string]string{}, }, { name: "Only Host", @@ -101,12 +103,34 @@ func TestDefaultConfig(t *testing.T) { res, err := resource.New(ctx, resource.WithDetectors(tt.detectors...), ) + require.NoError(t, err) require.EqualValues(t, tt.resouceValues, toMap(res)) }) } } +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(), + } + 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, resouceValues, toMap(res)) +} + func toMap(res *resource.Resource) map[string]string { m := map[string]string{} for _, attr := range res.Attributes() { From 7ee5d540ecef794a124a342c9f2dc3108596f4e8 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Fri, 16 Apr 2021 15:31:22 +0000 Subject: [PATCH 06/15] Unexports the noOp detector --- sdk/resource/builtin.go | 6 +++--- sdk/resource/config.go | 6 +++--- sdk/resource/config_test.go | 23 ++++++++++++++--------- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/sdk/resource/builtin.go b/sdk/resource/builtin.go index eed3e86b3a9..d021b8e201f 100644 --- a/sdk/resource/builtin.go +++ b/sdk/resource/builtin.go @@ -47,9 +47,9 @@ type ( defaultServiceNameDetector struct{} - // NoOp is a Detector that only provides an empty resource. Used + // noOp is a Detector that only provides an empty resource. Used // to disable automatic detection. - NoOp struct{} + noOp struct{} ) var ( @@ -107,6 +107,6 @@ func (defaultServiceNameDetector) Detect(ctx context.Context) (*Resource, error) } // Detector that does nothing. Used to disable default Detector behavior. -func (NoOp) Detect(_ context.Context) (*Resource, error) { +func (noOp) Detect(_ context.Context) (*Resource, error) { return &emptyResource, nil } diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 8ba59d30526..35a00d382e6 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -60,7 +60,7 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) { // 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` // ``` // New(ctx, @@ -71,7 +71,7 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) { // Use The `BuiltinDetectors`, then overlay Detector `d1` func WithDetectors(detectors ...Detector) Option { if len(detectors) == 0 { - return detectorsOption{detectors: []Detector{NoOp{}}} + return detectorsOption{detectors: []Detector{noOp{}}} } return detectorsOption{detectors: detectors} } @@ -93,7 +93,7 @@ var BuiltinDetectors = []Detector{ } // New returns a Resource combined from the provided attributes, -// user-provided detectors and builtin detectors. +// user-provided detectors or builtin detectors. func New(ctx context.Context, opts ...Option) (*Resource, error) { cfg := config{} for _, opt := range opts { diff --git a/sdk/resource/config_test.go b/sdk/resource/config_test.go index 47be01e4353..e1e2e4e639f 100644 --- a/sdk/resource/config_test.go +++ b/sdk/resource/config_test.go @@ -38,14 +38,14 @@ func TestConfig(t *testing.T) { resouceValues map[string]string }{ { - name: "No detectors", - envars: "", + name: "No detectors disables detection", + envars: "key=value,other=attr", detectors: []resource.Detector{}, resouceValues: map[string]string{}, }, { - name: "Nil detectors", - envars: "", + name: "Nil detectors disables detection", + envars: "key=value,other=attr", detectors: nil, resouceValues: map[string]string{}, }, @@ -83,12 +83,17 @@ func TestConfig(t *testing.T) { }, }, { - name: "Disable Detectors", - envars: "key=value,other=attr", - detectors: []resource.Detector{ - resource.NoOp{}, + 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", }, - resouceValues: map[string]string{}, }, } for _, tt := range tc { From 5eb993e64b54feaa1d5482a8e997cf54c22d95df Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Mon, 19 Apr 2021 14:59:45 +0000 Subject: [PATCH 07/15] Updated changelog --- CHANGELOG.md | 3 +++ sdk/resource/config.go | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 576b6297f89..19e4c583e21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 @@ -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) +- Remove `resource.WithHost()`, `resource.WithTelemetrySDK()`, and `resource.WithFromEnv()`. To replace these effects use `resource.WithDetectors()` (#1810) ## [0.19.0] - 2021-03-18 diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 35a00d382e6..3f94cd7c3d6 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -59,16 +59,16 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) { // inlcude a WithDetectors(BuiltinDetectors...), // Examples: // `New(ctx)`: Use builtin `Detector`s. -// `New(ctx, WithDetectors())`: Use no `Detector`s +// `New(ctx, WithDetectors())`: Use no `Detector`s. -// `New(ctx, WithDetectors(d1, d2))`: Use Detector `d1`, then overlay Detector `d2` +// `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` +// Use The `BuiltinDetectors`, then overlay Detector `d1`. func WithDetectors(detectors ...Detector) Option { if len(detectors) == 0 { return detectorsOption{detectors: []Detector{noOp{}}} From 5d59981a3763bdff79ca3a325398a37d3c514246 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Tue, 20 Apr 2021 15:27:01 +0000 Subject: [PATCH 08/15] Fixes to spelling mistakes. --- CHANGELOG.md | 2 +- sdk/resource/builtin.go | 2 +- sdk/resource/config.go | 4 ++-- sdk/resource/config_test.go | 28 ++++++++++++++-------------- 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19e4c583e21..6917f8588c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,7 +90,7 @@ 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) +- Remove `resource.WithoutBuiltin()`. Any use of `resource.WithDetecors()` will have the same effect (#1810) - Remove `resource.WithHost()`, `resource.WithTelemetrySDK()`, and `resource.WithFromEnv()`. To replace these effects use `resource.WithDetectors()` (#1810) ## [0.19.0] - 2021-03-18 diff --git a/sdk/resource/builtin.go b/sdk/resource/builtin.go index d021b8e201f..8031a7d2e67 100644 --- a/sdk/resource/builtin.go +++ b/sdk/resource/builtin.go @@ -106,7 +106,7 @@ func (defaultServiceNameDetector) Detect(ctx context.Context) (*Resource, error) ).Detect(ctx) } -// Detector that does nothing. Used to disable default Detector behavior. +// Detect that does nothing. Used to disable default Detector behavior. func (noOp) Detect(_ context.Context) (*Resource, error) { return &emptyResource, nil } diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 3f94cd7c3d6..29123b01f70 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -55,8 +55,8 @@ 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...), +// Any use of WithDetectors disables the default detectors. Use +// WithDetectors(BuiltinDetectors...) to reenable the default detectors. // Examples: // `New(ctx)`: Use builtin `Detector`s. // `New(ctx, WithDetectors())`: Use no `Detector`s. diff --git a/sdk/resource/config_test.go b/sdk/resource/config_test.go index e1e2e4e639f..4b735c38bc6 100644 --- a/sdk/resource/config_test.go +++ b/sdk/resource/config_test.go @@ -35,19 +35,19 @@ func TestConfig(t *testing.T) { envars string detectors []resource.Detector - resouceValues map[string]string + resourceValues map[string]string }{ { - name: "No detectors disables detection", - envars: "key=value,other=attr", - detectors: []resource.Detector{}, - resouceValues: map[string]string{}, + name: "No detectors disables detection", + envars: "key=value,other=attr", + detectors: []resource.Detector{}, + resourceValues: map[string]string{}, }, { - name: "Nil detectors disables detection", - envars: "key=value,other=attr", - detectors: nil, - resouceValues: map[string]string{}, + name: "Nil detectors disables detection", + envars: "key=value,other=attr", + detectors: nil, + resourceValues: map[string]string{}, }, { name: "Only Host", @@ -55,7 +55,7 @@ func TestConfig(t *testing.T) { detectors: []resource.Detector{ resource.Host{}, }, - resouceValues: map[string]string{ + resourceValues: map[string]string{ "host.name": hostname(), }, }, @@ -65,7 +65,7 @@ func TestConfig(t *testing.T) { detectors: []resource.Detector{ resource.FromEnv{}, }, - resouceValues: map[string]string{ + resourceValues: map[string]string{ "key": "value", "other": "attr", }, @@ -76,7 +76,7 @@ func TestConfig(t *testing.T) { detectors: []resource.Detector{ resource.TelemetrySDK{}, }, - resouceValues: map[string]string{ + resourceValues: map[string]string{ "telemetry.sdk.name": "opentelemetry", "telemetry.sdk.language": "go", "telemetry.sdk.version": otel.Version(), @@ -86,7 +86,7 @@ func TestConfig(t *testing.T) { name: "Builtins", envars: "key=value,other=attr", detectors: resource.BuiltinDetectors, - resouceValues: map[string]string{ + resourceValues: map[string]string{ "host.name": hostname(), "telemetry.sdk.name": "opentelemetry", "telemetry.sdk.language": "go", @@ -110,7 +110,7 @@ func TestConfig(t *testing.T) { ) require.NoError(t, err) - require.EqualValues(t, tt.resouceValues, toMap(res)) + require.EqualValues(t, tt.resourceValues, toMap(res)) }) } } From 0bb0f87516384995ef2218678778c7e32813e84f Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 21 Apr 2021 18:39:19 +0000 Subject: [PATCH 09/15] Added NewEmptyResouce and unexported builtin detectors --- CHANGELOG.md | 6 +- sdk/resource/builtin.go | 25 ++--- sdk/resource/builtin_test.go | 2 +- sdk/resource/config.go | 48 +++------ sdk/resource/config_test.go | 153 ----------------------------- sdk/resource/env.go | 8 +- sdk/resource/env_test.go | 8 +- sdk/resource/resource.go | 18 +++- sdk/resource/resource_test.go | 178 +++++++++++++++++++++++++++++++++- 9 files changed, 230 insertions(+), 216 deletions(-) delete mode 100644 sdk/resource/config_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cd66e8abd7d..9a6352f7460 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `Event` and `Link` struct types from the `go.opentelemetry.io/otel` package now include a `DroppedAttributeCount` field to record the number of attributes that were not recorded due to configured limits being reached. (#1771) - The Jaeger exporter now reports dropped attributes for a Span event in the exported log. (#1771) - Adds `k8s.node.name` and `k8s.node.uid` attribute keys to the `semconv` package. (#1789) +- Adds `resource.NewEmptyResouce()` for creating a Resouce without builtin detectors (#1810) - Adds `otlpgrpc.WithTimeout` option for configuring timeout to the otlp/gRPC exporter. (#1821) ### Fixed @@ -74,7 +75,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `go.opentelemetry.io/otel/sdk/export/trace` package is merged into the `go.opentelemetry.io/otel/sdk/trace` package. (#1778) - The prometheus.InstallNewPipeline example is moved from comment to example test (#1796) - The convenience functions for the stdout exporter have been updated to return the `TracerProvider` implementation and enable the shutdown of the exporter. (#1800) -- Resources default detectors are overwritten by using `WithDetectors()` (#1810) - Replace the flush function returned from the Jaeger exporter's convenience creation functions (`InstallNewPipeline` and `NewExportPipeline`) with the `TracerProvider` implementation they create. This enables the caller to shutdown and flush using the related `TracerProvider` methods. (#1822) @@ -97,8 +97,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm The information that could be configured in the `Process` struct should be configured in a `Resource` instead. (#1776, #1804) - Remove the `WithDisabled` option from the Jaeger exporter. To disable the exporter unregister it from the `TracerProvider` or use a no-operation `TracerProvider`. (#1806) -- Remove `resource.WithoutBuiltin()`. Any use of `resource.WithDetecors()` will have the same effect (#1810) -- Remove `resource.WithHost()`, `resource.WithTelemetrySDK()`, and `resource.WithFromEnv()`. To replace these effects use `resource.WithDetectors()` (#1810) +- Remove `resource.WithoutBuiltin()`. Use `resource.NewEmptyResource()`. (#1810) +- Unexported types `resouce.FromEnv`, `resource.Host`, and `resource.TelemetrySDK`, Use the corresponding `With*()` to use individually. (#1810) - Removed the Jaeger exporter `WithSDKOptions` `Option`. This option was used to set SDK options for the exporter creation convenience functions. These functions are provided as a way to easily setup or install the exporter with what are deemed reasonable SDK settings for common use cases. diff --git a/sdk/resource/builtin.go b/sdk/resource/builtin.go index 8031a7d2e67..9ad790883f4 100644 --- a/sdk/resource/builtin.go +++ b/sdk/resource/builtin.go @@ -26,19 +26,19 @@ import ( ) type ( - // TelemetrySDK is a Detector that provides information about + // telemetrySDK is a Detector that provides information about // the OpenTelemetry SDK used. This Detector is included as a // builtin. If these resource attributes are not wanted, use // the WithTelemetrySDK(nil) or WithoutBuiltin() options to // explicitly disable them. - TelemetrySDK struct{} + telemetrySDK struct{} - // Host is a Detector that provides information about the host + // host is a Detector that provides information about the host // being run on. This Detector is included as a builtin. If // these resource attributes are not wanted, use the // WithHost(nil) or WithoutBuiltin() options to explicitly // disable them. - Host struct{} + host struct{} stringDetector struct { K attribute.Key @@ -46,21 +46,17 @@ type ( } defaultServiceNameDetector struct{} - - // noOp is a Detector that only provides an empty resource. Used - // to disable automatic detection. - noOp struct{} ) var ( - _ Detector = TelemetrySDK{} - _ Detector = Host{} + _ Detector = telemetrySDK{} + _ Detector = host{} _ Detector = stringDetector{} _ Detector = defaultServiceNameDetector{} ) // Detect returns a *Resource that describes the OpenTelemetry SDK used. -func (TelemetrySDK) Detect(context.Context) (*Resource, error) { +func (telemetrySDK) Detect(context.Context) (*Resource, error) { return NewWithAttributes( semconv.TelemetrySDKNameKey.String("opentelemetry"), semconv.TelemetrySDKLanguageKey.String("go"), @@ -69,7 +65,7 @@ func (TelemetrySDK) Detect(context.Context) (*Resource, error) { } // Detect returns a *Resource that describes the host being run on. -func (Host) Detect(ctx context.Context) (*Resource, error) { +func (host) Detect(ctx context.Context) (*Resource, error) { return StringDetector(semconv.HostNameKey, os.Hostname).Detect(ctx) } @@ -105,8 +101,3 @@ func (defaultServiceNameDetector) Detect(ctx context.Context) (*Resource, error) }, ).Detect(ctx) } - -// Detect that does nothing. Used to disable default Detector behavior. -func (noOp) Detect(_ context.Context) (*Resource, error) { - return &emptyResource, nil -} diff --git a/sdk/resource/builtin_test.go b/sdk/resource/builtin_test.go index 223bd0df800..3fd26536110 100644 --- a/sdk/resource/builtin_test.go +++ b/sdk/resource/builtin_test.go @@ -59,7 +59,7 @@ func TestStringDetectorErrors(t *testing.T) { } for _, test := range tests { - res, err := resource.New( + res, err := resource.NewEmptyResouce( context.Background(), resource.WithAttributes(attribute.String("A", "B")), resource.WithDetectors(test.s), diff --git a/sdk/resource/config.go b/sdk/resource/config.go index 29123b01f70..eb657cd41eb 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -55,24 +55,7 @@ func (d detectAttributes) Detect(context.Context) (*Resource, error) { } // WithDetectors adds detectors to be evaluated for the configured resource. -// Any use of WithDetectors disables the default detectors. Use -// WithDetectors(BuiltinDetectors...) to reenable the default detectors. -// Examples: -// `New(ctx)`: Use builtin `Detector`s. -// `New(ctx, WithDetectors())`: Use no `Detector`s. - -// `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} } @@ -86,23 +69,24 @@ func (o detectorsOption) Apply(cfg *config) { cfg.detectors = append(cfg.detectors, o.detectors...) } -var BuiltinDetectors = []Detector{ - TelemetrySDK{}, - Host{}, - FromEnv{}, +// WithBuiltinDetectors adds the built detectors to the configured resoruce. +func WithBuiltinDetectors() Option { + return WithDetectors(telemetrySDK{}, + host{}, + fromEnv{}) } -// New returns a Resource combined from the provided attributes, -// user-provided detectors or builtin detectors. -func New(ctx context.Context, opts ...Option) (*Resource, error) { - cfg := config{} - for _, opt := range opts { - opt.Apply(&cfg) - } +// WithFromEnv adds attributes from environment variables to the configured resoruce. +func WithFromEnv() Option { + return WithDetectors(fromEnv{}) +} - if cfg.detectors == nil { - cfg.detectors = BuiltinDetectors - } +// WithHost adds attributes from the host to the configured resoruce. +func WithHost() Option { + return WithDetectors(host{}) +} - return Detect(ctx, cfg.detectors...) +// WithTelemetrySDK adds TelemetrySDK version info to the configured resoruce. +func WithTelemetrySDK() Option { + return WithDetectors(telemetrySDK{}) } diff --git a/sdk/resource/config_test.go b/sdk/resource/config_test.go deleted file mode 100644 index 4b735c38bc6..00000000000 --- a/sdk/resource/config_test.go +++ /dev/null @@ -1,153 +0,0 @@ -// Copyright The OpenTelemetry Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package resource_test - -import ( - "context" - "fmt" - "os" - "testing" - - "github.com/stretchr/testify/require" - - "go.opentelemetry.io/otel" - ottest "go.opentelemetry.io/otel/internal/internaltest" - "go.opentelemetry.io/otel/sdk/resource" -) - -const envVar = "OTEL_RESOURCE_ATTRIBUTES" - -func TestConfig(t *testing.T) { - tc := []struct { - name string - envars string - detectors []resource.Detector - - resourceValues map[string]string - }{ - { - name: "No detectors disables detection", - envars: "key=value,other=attr", - detectors: []resource.Detector{}, - resourceValues: map[string]string{}, - }, - { - name: "Nil detectors disables detection", - envars: "key=value,other=attr", - detectors: nil, - resourceValues: map[string]string{}, - }, - { - name: "Only Host", - envars: "from=here", - detectors: []resource.Detector{ - resource.Host{}, - }, - resourceValues: map[string]string{ - "host.name": hostname(), - }, - }, - { - name: "Only Env", - envars: "key=value,other=attr", - detectors: []resource.Detector{ - resource.FromEnv{}, - }, - resourceValues: map[string]string{ - "key": "value", - "other": "attr", - }, - }, - { - name: "Only TelemetrySDK", - envars: "", - detectors: []resource.Detector{ - resource.TelemetrySDK{}, - }, - resourceValues: 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, - resourceValues: 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.resourceValues, toMap(res)) - }) - } -} - -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(), - } - 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, resouceValues, toMap(res)) -} - -func toMap(res *resource.Resource) map[string]string { - m := map[string]string{} - for _, attr := range res.Attributes() { - m[string(attr.Key)] = attr.Value.Emit() - } - return m -} - -func hostname() string { - hn, err := os.Hostname() - if err != nil { - return fmt.Sprintf("hostname(%s)", err) - } - return hn -} diff --git a/sdk/resource/env.go b/sdk/resource/env.go index defb455382b..35b99c129e2 100644 --- a/sdk/resource/env.go +++ b/sdk/resource/env.go @@ -31,18 +31,18 @@ var ( errMissingValue = fmt.Errorf("%w: missing value", ErrPartialResource) ) -// FromEnv is a Detector that implements the Detector and collects +// fromEnv is a Detector that implements the Detector and collects // resources from environment. This Detector is included as a // builtin. If these resource attributes are not wanted, use the // WithFromEnv(nil) or WithoutBuiltin() options to explicitly disable // them. -type FromEnv struct{} +type fromEnv struct{} // compile time assertion that FromEnv implements Detector interface -var _ Detector = FromEnv{} +var _ Detector = fromEnv{} // Detect collects resources from environment -func (FromEnv) Detect(context.Context) (*Resource, error) { +func (fromEnv) Detect(context.Context) (*Resource, error) { attrs := strings.TrimSpace(os.Getenv(envVar)) if attrs == "" { diff --git a/sdk/resource/env_test.go b/sdk/resource/env_test.go index 50b788c2ff3..78bd68346b2 100644 --- a/sdk/resource/env_test.go +++ b/sdk/resource/env_test.go @@ -33,7 +33,7 @@ func TestDetectOnePair(t *testing.T) { require.NoError(t, err) defer func() { require.NoError(t, store.Restore()) }() - detector := &FromEnv{} + detector := &fromEnv{} res, err := detector.Detect(context.Background()) require.NoError(t, err) assert.Equal(t, NewWithAttributes(attribute.String("key", "value")), res) @@ -47,7 +47,7 @@ func TestDetectMultiPairs(t *testing.T) { require.NoError(t, err) defer func() { require.NoError(t, store.Restore()) }() - detector := &FromEnv{} + detector := &fromEnv{} res, err := detector.Detect(context.Background()) require.NoError(t, err) assert.Equal(t, res, NewWithAttributes( @@ -65,7 +65,7 @@ func TestEmpty(t *testing.T) { require.NoError(t, err) defer func() { require.NoError(t, store.Restore()) }() - detector := &FromEnv{} + detector := &fromEnv{} res, err := detector.Detect(context.Background()) require.NoError(t, err) assert.Equal(t, Empty(), res) @@ -78,7 +78,7 @@ func TestMissingKeyError(t *testing.T) { require.NoError(t, err) defer func() { require.NoError(t, store.Restore()) }() - detector := &FromEnv{} + detector := &fromEnv{} res, err := detector.Detect(context.Background()) assert.Error(t, err) assert.Equal(t, err, fmt.Errorf("%w: %v", errMissingValue, "[key]")) diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index a2db66cf3b5..610f4cb65ac 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -40,9 +40,25 @@ var ( otel.Handle(err) } return r - }(Detect(context.Background(), defaultServiceNameDetector{}, TelemetrySDK{})) + }(Detect(context.Background(), defaultServiceNameDetector{}, telemetrySDK{})) ) +// New returns a Resource combined from the builtin detectors and user-provided detectors. +func New(ctx context.Context, opts ...Option) (*Resource, error) { + opts = append([]Option{WithBuiltinDetectors()}, opts...) + return NewEmptyResouce(ctx, opts...) +} + +// NewEmptyResouce returns a Resource combined from the user-provided detectors. +func NewEmptyResouce(ctx context.Context, opts ...Option) (*Resource, error) { + cfg := config{} + for _, opt := range opts { + opt.Apply(&cfg) + } + + return Detect(ctx, cfg.detectors...) +} + // NewWithAttributes creates a resource from attrs. If attrs contains // duplicate keys, the last value will be used. If attrs contains any invalid // items those items will be dropped. diff --git a/sdk/resource/resource_test.go b/sdk/resource/resource_test.go index 00f0c6cca18..4846860e80a 100644 --- a/sdk/resource/resource_test.go +++ b/sdk/resource/resource_test.go @@ -15,8 +15,10 @@ package resource_test import ( + "context" "encoding/json" "fmt" + "os" "strings" "testing" @@ -25,6 +27,7 @@ import ( "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" + ottest "go.opentelemetry.io/otel/internal/internaltest" "go.opentelemetry.io/otel/sdk/resource" "go.opentelemetry.io/otel/semconv" ) @@ -38,7 +41,7 @@ var ( kv42 = attribute.String("k4", "") ) -func TestNew(t *testing.T) { +func TestNewWithAttributes(t *testing.T) { cases := []struct { name string in []attribute.KeyValue @@ -252,6 +255,8 @@ func TestString(t *testing.T) { } } +const envVar = "OTEL_RESOURCE_ATTRIBUTES" + func TestMarshalJSON(t *testing.T) { r := resource.NewWithAttributes(attribute.Int64("A", 1), attribute.String("C", "D")) data, err := json.Marshal(r) @@ -260,3 +265,174 @@ func TestMarshalJSON(t *testing.T) { `[{"Key":"A","Value":{"Type":"INT64","Value":1}},{"Key":"C","Value":{"Type":"STRING","Value":"D"}}]`, string(data)) } + +func TestNewRawResource(t *testing.T) { + tc := []struct { + name string + envars string + detectors []resource.Detector + options []resource.Option + + resourceValues map[string]string + }{ + { + name: "No Options returns empty resrouce", + envars: "key=value,other=attr", + options: nil, + resourceValues: map[string]string{}, + }, + { + name: "Nil Detectors works", + envars: "key=value,other=attr", + options: []resource.Option{ + resource.WithDetectors(), + }, + resourceValues: map[string]string{}, + }, + { + name: "Only Host", + envars: "from=here", + options: []resource.Option{ + resource.WithHost(), + }, + resourceValues: map[string]string{ + "host.name": hostname(), + }, + }, + { + name: "Only Env", + envars: "key=value,other=attr", + options: []resource.Option{ + resource.WithFromEnv(), + }, + resourceValues: map[string]string{ + "key": "value", + "other": "attr", + }, + }, + { + name: "Only TelemetrySDK", + envars: "", + options: []resource.Option{ + resource.WithTelemetrySDK(), + }, + resourceValues: map[string]string{ + "telemetry.sdk.name": "opentelemetry", + "telemetry.sdk.language": "go", + "telemetry.sdk.version": otel.Version(), + }, + }, + { + name: "WithAttributes", + envars: "key=value,other=attr", + options: []resource.Option{ + resource.WithAttributes(attribute.String("A", "B")), + }, + resourceValues: map[string]string{ + "A": "B", + }, + }, + { + name: "Builtins", + envars: "key=value,other=attr", + options: []resource.Option{ + resource.WithBuiltinDetectors(), + }, + resourceValues: 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.NewEmptyResouce(ctx, tt.options...) + + require.NoError(t, err) + require.EqualValues(t, tt.resourceValues, toMap(res)) + }) + } +} + +func TestNew(t *testing.T) { + tc := []struct { + name string + envars string + detectors []resource.Detector + options []resource.Option + + resourceValues map[string]string + }{ + { + name: "No Options returns builtin", + envars: "key=value,other=attr", + options: nil, + resourceValues: map[string]string{ + "host.name": hostname(), + "telemetry.sdk.name": "opentelemetry", + "telemetry.sdk.language": "go", + "telemetry.sdk.version": otel.Version(), + "key": "value", + "other": "attr", + }, + }, + { + name: "WithAttributes", + envars: "key=value,other=attr", + options: []resource.Option{ + resource.WithAttributes(attribute.String("A", "B")), + }, + resourceValues: map[string]string{ + "host.name": hostname(), + "telemetry.sdk.name": "opentelemetry", + "telemetry.sdk.language": "go", + "telemetry.sdk.version": otel.Version(), + "key": "value", + "other": "attr", + "A": "B", + }, + }, + } + 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, tt.options...) + + require.NoError(t, err) + require.EqualValues(t, tt.resourceValues, toMap(res)) + }) + } +} + +func toMap(res *resource.Resource) map[string]string { + m := map[string]string{} + for _, attr := range res.Attributes() { + m[string(attr.Key)] = attr.Value.Emit() + } + return m +} + +func hostname() string { + hn, err := os.Hostname() + if err != nil { + return fmt.Sprintf("hostname(%s)", err) + } + return hn +} From e403bd53704ff8ead8134e4927755403a6fd998d Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 21 Apr 2021 19:47:56 +0000 Subject: [PATCH 10/15] Fix for prometheus example --- exporters/metric/prometheus/example_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exporters/metric/prometheus/example_test.go b/exporters/metric/prometheus/example_test.go index e34de6a2c1e..8bf342256be 100644 --- a/exporters/metric/prometheus/example_test.go +++ b/exporters/metric/prometheus/example_test.go @@ -38,7 +38,7 @@ import ( func ExampleNewExportPipeline() { // Create a resource, with builtin attributes plus R=V. - res, err := resource.New( + res, err := resource.NewEmptyResouce( context.Background(), resource.WithAttributes(attribute.String("R", "V")), ) From f531472ad783d15612bdede52f4e8f31ccea5973 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Thu, 22 Apr 2021 15:49:18 -0500 Subject: [PATCH 11/15] Resource has two Rs I need to get a new R key it seems. Co-authored-by: Sam Xie --- CHANGELOG.md | 4 ++-- exporters/metric/prometheus/example_test.go | 2 +- sdk/resource/builtin_test.go | 2 +- sdk/resource/resource.go | 6 +++--- sdk/resource/resource_test.go | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c87d236273d..33b7e532bad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - The `Event` and `Link` struct types from the `go.opentelemetry.io/otel` package now include a `DroppedAttributeCount` field to record the number of attributes that were not recorded due to configured limits being reached. (#1771) - The Jaeger exporter now reports dropped attributes for a Span event in the exported log. (#1771) - Adds `k8s.node.name` and `k8s.node.uid` attribute keys to the `semconv` package. (#1789) -- Adds `resource.NewEmptyResouce()` for creating a Resouce without builtin detectors (#1810) +- Adds `resource.NewEmptyResource()` for creating a Resource without builtin detectors (#1810) - Adds `otlpgrpc.WithTimeout` option for configuring timeout to the otlp/gRPC exporter. (#1821) ### Fixed @@ -107,7 +107,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Remove the `WithDisabled` option from the Jaeger exporter. To disable the exporter unregister it from the `TracerProvider` or use a no-operation `TracerProvider`. (#1806) - Remove `resource.WithoutBuiltin()`. Use `resource.NewEmptyResource()`. (#1810) -- Unexported types `resouce.FromEnv`, `resource.Host`, and `resource.TelemetrySDK`, Use the corresponding `With*()` to use individually. (#1810) +- Unexported types `resource.FromEnv`, `resource.Host`, and `resource.TelemetrySDK`, Use the corresponding `With*()` to use individually. (#1810) - Removed the Jaeger exporter `WithSDKOptions` `Option`. This option was used to set SDK options for the exporter creation convenience functions. These functions are provided as a way to easily setup or install the exporter with what are deemed reasonable SDK settings for common use cases. diff --git a/exporters/metric/prometheus/example_test.go b/exporters/metric/prometheus/example_test.go index 8bf342256be..cc738ab928e 100644 --- a/exporters/metric/prometheus/example_test.go +++ b/exporters/metric/prometheus/example_test.go @@ -38,7 +38,7 @@ import ( func ExampleNewExportPipeline() { // Create a resource, with builtin attributes plus R=V. - res, err := resource.NewEmptyResouce( + res, err := resource.NewEmptyResource( context.Background(), resource.WithAttributes(attribute.String("R", "V")), ) diff --git a/sdk/resource/builtin_test.go b/sdk/resource/builtin_test.go index 3fd26536110..69f74ccc900 100644 --- a/sdk/resource/builtin_test.go +++ b/sdk/resource/builtin_test.go @@ -59,7 +59,7 @@ func TestStringDetectorErrors(t *testing.T) { } for _, test := range tests { - res, err := resource.NewEmptyResouce( + res, err := resource.NewEmptyResource( context.Background(), resource.WithAttributes(attribute.String("A", "B")), resource.WithDetectors(test.s), diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index b42a8b67aa3..2c1dc330370 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -46,11 +46,11 @@ var ( // New returns a Resource combined from the builtin detectors and user-provided detectors. func New(ctx context.Context, opts ...Option) (*Resource, error) { opts = append([]Option{WithBuiltinDetectors()}, opts...) - return NewEmptyResouce(ctx, opts...) + return NewEmptyResource(ctx, opts...) } -// NewEmptyResouce returns a Resource combined from the user-provided detectors. -func NewEmptyResouce(ctx context.Context, opts ...Option) (*Resource, error) { +// NewEmptyResource returns a Resource combined from the user-provided detectors. +func NewEmptyResource(ctx context.Context, opts ...Option) (*Resource, error) { cfg := config{} for _, opt := range opts { opt.Apply(&cfg) diff --git a/sdk/resource/resource_test.go b/sdk/resource/resource_test.go index 4846860e80a..fa33769ce68 100644 --- a/sdk/resource/resource_test.go +++ b/sdk/resource/resource_test.go @@ -357,7 +357,7 @@ func TestNewRawResource(t *testing.T) { defer func() { require.NoError(t, store.Restore()) }() ctx := context.Background() - res, err := resource.NewEmptyResouce(ctx, tt.options...) + res, err := resource.NewEmptyResource(ctx, tt.options...) require.NoError(t, err) require.EqualValues(t, tt.resourceValues, toMap(res)) From 0c209dad013171d1fb4d50c394820ef9d8e089fb Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Tue, 27 Apr 2021 02:17:16 +0000 Subject: [PATCH 12/15] Replaced NewEmptyResource() with New() --- exporters/metric/prometheus/example_test.go | 2 +- sdk/resource/builtin_test.go | 2 +- sdk/resource/os_test.go | 1 - sdk/resource/process_test.go | 11 ----------- sdk/resource/resource.go | 8 +------- sdk/resource/resource_test.go | 7 ++++--- 6 files changed, 7 insertions(+), 24 deletions(-) diff --git a/exporters/metric/prometheus/example_test.go b/exporters/metric/prometheus/example_test.go index cc738ab928e..e34de6a2c1e 100644 --- a/exporters/metric/prometheus/example_test.go +++ b/exporters/metric/prometheus/example_test.go @@ -38,7 +38,7 @@ import ( func ExampleNewExportPipeline() { // Create a resource, with builtin attributes plus R=V. - res, err := resource.NewEmptyResource( + res, err := resource.New( context.Background(), resource.WithAttributes(attribute.String("R", "V")), ) diff --git a/sdk/resource/builtin_test.go b/sdk/resource/builtin_test.go index 69f74ccc900..223bd0df800 100644 --- a/sdk/resource/builtin_test.go +++ b/sdk/resource/builtin_test.go @@ -59,7 +59,7 @@ func TestStringDetectorErrors(t *testing.T) { } for _, test := range tests { - res, err := resource.NewEmptyResource( + res, err := resource.New( context.Background(), resource.WithAttributes(attribute.String("A", "B")), resource.WithDetectors(test.s), diff --git a/sdk/resource/os_test.go b/sdk/resource/os_test.go index 7de00b4f6d6..909defdb28c 100644 --- a/sdk/resource/os_test.go +++ b/sdk/resource/os_test.go @@ -38,7 +38,6 @@ func TestWithOSType(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithOSType(), ) diff --git a/sdk/resource/process_test.go b/sdk/resource/process_test.go index d4692293d3e..947378ddc1d 100644 --- a/sdk/resource/process_test.go +++ b/sdk/resource/process_test.go @@ -148,7 +148,6 @@ func testWithProcessPID(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessPID(), ) @@ -162,7 +161,6 @@ func testWithProcessExecutableName(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessExecutableName(), ) @@ -176,7 +174,6 @@ func testWithProcessExecutablePath(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessExecutablePath(), ) @@ -190,7 +187,6 @@ func testWithProcessCommandArgs(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessCommandArgs(), ) @@ -204,7 +200,6 @@ func testWithProcessOwner(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessOwner(), ) @@ -218,7 +213,6 @@ func testWithProcessRuntimeName(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessRuntimeName(), ) @@ -232,7 +226,6 @@ func testWithProcessRuntimeVersion(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessRuntimeVersion(), ) @@ -246,7 +239,6 @@ func testWithProcessRuntimeDescription(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessRuntimeDescription(), ) @@ -260,7 +252,6 @@ func testWithProcess(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcess(), ) @@ -281,7 +272,6 @@ func testWithProcessExecutablePathError(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessExecutablePath(), ) @@ -293,7 +283,6 @@ func testWithProcessOwnerError(t *testing.T) { ctx := context.Background() res, err := resource.New(ctx, - resource.WithoutBuiltin(), resource.WithProcessOwner(), ) diff --git a/sdk/resource/resource.go b/sdk/resource/resource.go index 2c1dc330370..c64baaa6e9f 100644 --- a/sdk/resource/resource.go +++ b/sdk/resource/resource.go @@ -43,14 +43,8 @@ var ( }(Detect(context.Background(), defaultServiceNameDetector{}, fromEnv{}, telemetrySDK{})) ) -// New returns a Resource combined from the builtin detectors and user-provided detectors. +// New returns a Resource combined from the user-provided detectors. func New(ctx context.Context, opts ...Option) (*Resource, error) { - opts = append([]Option{WithBuiltinDetectors()}, opts...) - return NewEmptyResource(ctx, opts...) -} - -// NewEmptyResource returns a Resource combined from the user-provided detectors. -func NewEmptyResource(ctx context.Context, opts ...Option) (*Resource, error) { cfg := config{} for _, opt := range opts { opt.Apply(&cfg) diff --git a/sdk/resource/resource_test.go b/sdk/resource/resource_test.go index fa33769ce68..c7e77880fbc 100644 --- a/sdk/resource/resource_test.go +++ b/sdk/resource/resource_test.go @@ -357,7 +357,7 @@ func TestNewRawResource(t *testing.T) { defer func() { require.NoError(t, store.Restore()) }() ctx := context.Background() - res, err := resource.NewEmptyResource(ctx, tt.options...) + res, err := resource.New(ctx, tt.options...) require.NoError(t, err) require.EqualValues(t, tt.resourceValues, toMap(res)) @@ -365,7 +365,7 @@ func TestNewRawResource(t *testing.T) { } } -func TestNew(t *testing.T) { +func TestNew_WithBuiltinDetectors(t *testing.T) { tc := []struct { name string envars string @@ -413,7 +413,8 @@ func TestNew(t *testing.T) { defer func() { require.NoError(t, store.Restore()) }() ctx := context.Background() - res, err := resource.New(ctx, tt.options...) + options := append([]resource.Option{resource.WithBuiltinDetectors()}, tt.options...) + res, err := resource.New(ctx, options...) require.NoError(t, err) require.EqualValues(t, tt.resourceValues, toMap(res)) From 136d86bfba78db1357b4e6274fefb72ffb2c39c4 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Tue, 27 Apr 2021 02:23:35 +0000 Subject: [PATCH 13/15] Fix test function name --- sdk/resource/resource_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/resource/resource_test.go b/sdk/resource/resource_test.go index c7e77880fbc..ab9fb82e381 100644 --- a/sdk/resource/resource_test.go +++ b/sdk/resource/resource_test.go @@ -266,7 +266,7 @@ func TestMarshalJSON(t *testing.T) { string(data)) } -func TestNewRawResource(t *testing.T) { +func TestNew(t *testing.T) { tc := []struct { name string envars string From e1b9b1af8edec5ef4b2f47202c6afb261d77a31b Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Tue, 27 Apr 2021 02:25:41 +0000 Subject: [PATCH 14/15] Comment fixups. --- CHANGELOG.md | 2 +- sdk/resource/config.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 20f09f379ae..c1d10c468bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Make `NewSplitDriver` from `go.opentelemetry.io/otel/exporters/otlp` take variadic arguments instead of a `SplitConfig` item. `NewSplitDriver` now automically implements an internal `noopDriver` for `SplitConfig` fields that are not initialized. (#1798) -- `resource.New()` now creates a Resource without builtin detectors. Previous behavior is now achieved by using `WithBuiltinDetectors` Option (#1810) +- `resource.New()` now creates a Resource without builtin detectors. Previous behavior is now achieved by using `WithBuiltinDetectors` Option. (#1810) ### Deprecated diff --git a/sdk/resource/config.go b/sdk/resource/config.go index eb657cd41eb..d83a3ece263 100644 --- a/sdk/resource/config.go +++ b/sdk/resource/config.go @@ -69,24 +69,24 @@ func (o detectorsOption) Apply(cfg *config) { cfg.detectors = append(cfg.detectors, o.detectors...) } -// WithBuiltinDetectors adds the built detectors to the configured resoruce. +// WithBuiltinDetectors adds the built detectors to the configured resource. func WithBuiltinDetectors() Option { return WithDetectors(telemetrySDK{}, host{}, fromEnv{}) } -// WithFromEnv adds attributes from environment variables to the configured resoruce. +// WithFromEnv adds attributes from environment variables to the configured resource. func WithFromEnv() Option { return WithDetectors(fromEnv{}) } -// WithHost adds attributes from the host to the configured resoruce. +// WithHost adds attributes from the host to the configured resource. func WithHost() Option { return WithDetectors(host{}) } -// WithTelemetrySDK adds TelemetrySDK version info to the configured resoruce. +// WithTelemetrySDK adds TelemetrySDK version info to the configured resource. func WithTelemetrySDK() Option { return WithDetectors(telemetrySDK{}) } From 8e33b84d8d76f4fcd1881ea11c03cb65a39b3da9 Mon Sep 17 00:00:00 2001 From: Aaron Clawson Date: Wed, 28 Apr 2021 12:15:07 -0500 Subject: [PATCH 15/15] Apply suggestions from code review Co-authored-by: Tyler Yahn --- CHANGELOG.md | 3 ++- sdk/resource/resource_test.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1d10c468bd..fd15776c2b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,6 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Added - ### Changed - Make `NewSplitDriver` from `go.opentelemetry.io/otel/exporters/otlp` take variadic arguments instead of a `SplitConfig` item. @@ -20,8 +19,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ### Deprecated ### Removed + - Remove `resource.WithoutBuiltin()`. Use `resource.New()`. (#1810) - Unexported types `resource.FromEnv`, `resource.Host`, and `resource.TelemetrySDK`, Use the corresponding `With*()` to use individually. (#1810) + ### Fixed ### Security diff --git a/sdk/resource/resource_test.go b/sdk/resource/resource_test.go index ab9fb82e381..c426e56f0da 100644 --- a/sdk/resource/resource_test.go +++ b/sdk/resource/resource_test.go @@ -365,7 +365,7 @@ func TestNew(t *testing.T) { } } -func TestNew_WithBuiltinDetectors(t *testing.T) { +func TestNewWithBuiltinDetectors(t *testing.T) { tc := []struct { name string envars string