diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f204165297..cb5b50dc4ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Changed the project minimum supported Go version from 1.15 to 1.16. (#2412) - The `"go.opentelemetry.io/otel/exporter/otel/otlpmetric/otlpmetricgrpc".Client` now uses the underlying gRPC `ClientConn` to handle name resolution, TCP connection establishment (with retries and backoff) and TLS handshakes, and handling errors on established connections by re-resolving the name and reconnecting. (#2425) - The `"go.opentelemetry.io/otel/exporter/otel/otlpmetric/otlpmetricgrpc".RetrySettings` type is renamed to `RetryConfig`. (#2425) +- The `go.opentelemetry.io/otel/exporter/otel/*` gRPC exporters now default to using the host's root CA set if none are provided by the user and `WithInsecure` is not specified. (#1584, #2432) ### Deprecated diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go b/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go index 17d0e71bc97..1d1663415cf 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/envconfig.go @@ -28,24 +28,21 @@ import ( "go.opentelemetry.io/otel" ) -var httpSchemeRegexp = regexp.MustCompile(`(?i)^http://|https://`) +var ( + httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`) -func ApplyGRPCEnvConfigs(cfg *Config) { - e := EnvOptionsReader{ + DefaultEnvOptionsReader = EnvOptionsReader{ GetEnv: os.Getenv, ReadFile: ioutil.ReadFile, } +) - e.ApplyGRPCEnvConfigs(cfg) +func ApplyGRPCEnvConfigs(cfg *Config) { + DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) } func ApplyHTTPEnvConfigs(cfg *Config) { - e := EnvOptionsReader{ - GetEnv: os.Getenv, - ReadFile: ioutil.ReadFile, - } - - e.ApplyHTTPEnvConfigs(cfg) + DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg) } type EnvOptionsReader struct { diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options.go index 067c72fd4ab..afae071f7c8 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/options.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/options.go @@ -97,10 +97,16 @@ func NewGRPCConfig(opts ...GRPCOption) Config { if cfg.ServiceConfig != "" { cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig)) } + // Priroritize GRPCCredentials over Insecure (passing both is an error). if cfg.Metrics.GRPCCredentials != nil { cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Metrics.GRPCCredentials)) } else if cfg.Metrics.Insecure { cfg.DialOptions = append(cfg.DialOptions, grpc.WithInsecure()) + } else { + // Default to using the host's root CA. + creds := credentials.NewTLS(nil) + cfg.Metrics.GRPCCredentials = creds + cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds)) } if cfg.Metrics.Compression == GzipCompression { cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name))) diff --git a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go index 2d47e7c285f..cd6694cce43 100644 --- a/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go @@ -189,6 +189,16 @@ func TestConfigs(t *testing.T) { }, // Certificate tests + { + name: "Test Default Certificate", + asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { + if grpcOption { + assert.NotNil(t, c.Metrics.GRPCCredentials) + } else { + assert.Nil(t, c.Metrics.TLSCfg) + } + }, + }, { name: "Test With Certificate", opts: []otlpconfig.GenericOption{ @@ -380,27 +390,32 @@ func TestConfigs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - - e := otlpconfig.EnvOptionsReader{ + origEOR := otlpconfig.DefaultEnvOptionsReader + otlpconfig.DefaultEnvOptionsReader = otlpconfig.EnvOptionsReader{ GetEnv: tt.env.getEnv, ReadFile: tt.fileReader.readFile, } + t.Cleanup(func() { otlpconfig.DefaultEnvOptionsReader = origEOR }) // Tests Generic options as HTTP Options cfg := otlpconfig.NewDefaultConfig() - e.ApplyHTTPEnvConfigs(&cfg) + otlpconfig.ApplyHTTPEnvConfigs(&cfg) for _, opt := range tt.opts { opt.ApplyHTTPOption(&cfg) } tt.asserts(t, &cfg, false) // Tests Generic options as gRPC Options - cfg = otlpconfig.NewDefaultConfig() - e.ApplyGRPCEnvConfigs(&cfg) - for _, opt := range tt.opts { - opt.ApplyGRPCOption(&cfg) - } + cfg = otlpconfig.NewGRPCConfig(asGRPCOptions(tt.opts)...) tt.asserts(t, &cfg, true) }) } } + +func asGRPCOptions(opts []otlpconfig.GenericOption) []otlpconfig.GRPCOption { + converted := make([]otlpconfig.GRPCOption, len(opts)) + for i, o := range opts { + converted[i] = otlpconfig.NewGRPCOption(o.ApplyGRPCOption) + } + return converted +} diff --git a/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go b/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go index dbd8f47998b..bc979541c90 100644 --- a/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go +++ b/exporters/otlp/otlpmetric/otlpmetricgrpc/client_test.go @@ -280,18 +280,6 @@ func TestNewExporter_WithTimeout(t *testing.T) { } } -func TestStartErrorInvalidSecurityConfiguration(t *testing.T) { - mc := runMockCollector(t) - defer func() { - _ = mc.stop() - }() - - client := otlpmetricgrpc.NewClient(otlpmetricgrpc.WithEndpoint(mc.endpoint)) - err := client.Start(context.Background()) - // https://github.com/grpc/grpc-go/blob/a671967dfbaab779d37fd7e597d9248f13806087/clientconn.go#L82 - assert.EqualError(t, err, "grpc: no transport security set (use grpc.WithInsecure() explicitly or set credentials)") -} - func TestStartErrorInvalidAddress(t *testing.T) { client := otlpmetricgrpc.NewClient( otlpmetricgrpc.WithInsecure(), diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go index 352a88206da..570d31ba8fa 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/envconfig.go @@ -28,24 +28,21 @@ import ( "go.opentelemetry.io/otel" ) -var httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`) +var ( + httpSchemeRegexp = regexp.MustCompile(`(?i)^(http://|https://)`) -func ApplyGRPCEnvConfigs(cfg *Config) { - e := EnvOptionsReader{ + DefaultEnvOptionsReader = EnvOptionsReader{ GetEnv: os.Getenv, ReadFile: ioutil.ReadFile, } +) - e.ApplyGRPCEnvConfigs(cfg) +func ApplyGRPCEnvConfigs(cfg *Config) { + DefaultEnvOptionsReader.ApplyGRPCEnvConfigs(cfg) } func ApplyHTTPEnvConfigs(cfg *Config) { - e := EnvOptionsReader{ - GetEnv: os.Getenv, - ReadFile: ioutil.ReadFile, - } - - e.ApplyHTTPEnvConfigs(cfg) + DefaultEnvOptionsReader.ApplyHTTPEnvConfigs(cfg) } type EnvOptionsReader struct { diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options.go b/exporters/otlp/otlptrace/internal/otlpconfig/options.go index 6cfb9fd2e67..47c0344c62c 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/options.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/options.go @@ -90,10 +90,16 @@ func NewGRPCConfig(opts ...GRPCOption) Config { if cfg.ServiceConfig != "" { cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultServiceConfig(cfg.ServiceConfig)) } + // Priroritize GRPCCredentials over Insecure (passing both is an error). if cfg.Traces.GRPCCredentials != nil { cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(cfg.Traces.GRPCCredentials)) } else if cfg.Traces.Insecure { cfg.DialOptions = append(cfg.DialOptions, grpc.WithInsecure()) + } else { + // Default to using the host's root CA. + creds := credentials.NewTLS(nil) + cfg.Traces.GRPCCredentials = creds + cfg.DialOptions = append(cfg.DialOptions, grpc.WithTransportCredentials(creds)) } if cfg.Traces.Compression == GzipCompression { cfg.DialOptions = append(cfg.DialOptions, grpc.WithDefaultCallOptions(grpc.UseCompressor(gzip.Name))) diff --git a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go index ad25efdac04..59bd09eab8c 100644 --- a/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go +++ b/exporters/otlp/otlptrace/internal/otlpconfig/options_test.go @@ -189,6 +189,16 @@ func TestConfigs(t *testing.T) { }, // Certificate tests + { + name: "Test Default Certificate", + asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) { + if grpcOption { + assert.NotNil(t, c.Traces.GRPCCredentials) + } else { + assert.Nil(t, c.Traces.TLSCfg) + } + }, + }, { name: "Test With Certificate", opts: []otlpconfig.GenericOption{ @@ -378,27 +388,32 @@ func TestConfigs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - - e := otlpconfig.EnvOptionsReader{ + origEOR := otlpconfig.DefaultEnvOptionsReader + otlpconfig.DefaultEnvOptionsReader = otlpconfig.EnvOptionsReader{ GetEnv: tt.env.getEnv, ReadFile: tt.fileReader.readFile, } + t.Cleanup(func() { otlpconfig.DefaultEnvOptionsReader = origEOR }) // Tests Generic options as HTTP Options cfg := otlpconfig.NewDefaultConfig() - e.ApplyHTTPEnvConfigs(&cfg) + otlpconfig.ApplyHTTPEnvConfigs(&cfg) for _, opt := range tt.opts { opt.ApplyHTTPOption(&cfg) } tt.asserts(t, &cfg, false) // Tests Generic options as gRPC Options - cfg = otlpconfig.NewDefaultConfig() - e.ApplyGRPCEnvConfigs(&cfg) - for _, opt := range tt.opts { - opt.ApplyGRPCOption(&cfg) - } + cfg = otlpconfig.NewGRPCConfig(asGRPCOptions(tt.opts)...) tt.asserts(t, &cfg, true) }) } } + +func asGRPCOptions(opts []otlpconfig.GenericOption) []otlpconfig.GRPCOption { + converted := make([]otlpconfig.GRPCOption, len(opts)) + for i, o := range opts { + converted[i] = otlpconfig.NewGRPCOption(o.ApplyGRPCOption) + } + return converted +} diff --git a/exporters/otlp/otlptrace/otlptracegrpc/client_test.go b/exporters/otlp/otlptrace/otlptracegrpc/client_test.go index a290255f7eb..9a02ca227b6 100644 --- a/exporters/otlp/otlptrace/otlptracegrpc/client_test.go +++ b/exporters/otlp/otlptrace/otlptracegrpc/client_test.go @@ -238,16 +238,6 @@ func TestExportSpansTimeoutHonored(t *testing.T) { require.Equal(t, codes.DeadlineExceeded, status.Convert(err).Code()) } -func TestStartErrorInvalidSecurityConfiguration(t *testing.T) { - mc := runMockCollector(t) - t.Cleanup(func() { require.NoError(t, mc.stop()) }) - - client := otlptracegrpc.NewClient(otlptracegrpc.WithEndpoint(mc.endpoint)) - err := client.Start(context.Background()) - // https://github.com/grpc/grpc-go/blob/a671967dfbaab779d37fd7e597d9248f13806087/clientconn.go#L82 - assert.EqualError(t, err, "grpc: no transport security set (use grpc.WithInsecure() explicitly or set credentials)") -} - func TestNew_withMultipleAttributeTypes(t *testing.T) { mc := runMockCollector(t)