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

Use port 4318 for otlp*http client default #2625

Merged
merged 7 commits into from Feb 23, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -25,6 +25,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Fixed

- Remove the OTLP trace exporter limit of SpanEvents when exporting. (#2616)
- Use port `4318` instead of `4317` for default for the `otlpmetrichttp` and `otlptracehttp` client. (#2614, #2625)

## [1.4.1] - 2022-02-16

Expand Down
40 changes: 35 additions & 5 deletions exporters/otlp/otlpmetric/internal/otlpconfig/options.go
Expand Up @@ -17,6 +17,8 @@ package otlpconfig // import "go.opentelemetry.io/otel/exporters/otlp/otlpmetric
import (
"crypto/tls"
"fmt"
"path"
"strings"
"time"

"google.golang.org/grpc"
Expand Down Expand Up @@ -72,24 +74,52 @@ type (
}
)

func NewDefaultConfig() Config {
c := Config{
// NewHTTPConfig returns a new Config with all settings applied from opts and
// any unset setting using the default HTTP config values.
func NewHTTPConfig(opts ...HTTPOption) Config {
cfg := Config{
Metrics: SignalConfig{
Endpoint: fmt.Sprintf("%s:%d", DefaultCollectorHost, DefaultCollectorPort),
Endpoint: fmt.Sprintf("%s:%d", DefaultCollectorHost, DefaultCollectorHTTPPort),
URLPath: DefaultMetricsPath,
Compression: NoCompression,
Timeout: DefaultTimeout,
},
RetryConfig: retry.DefaultConfig,
}
cfg = ApplyHTTPEnvConfigs(cfg)
for _, opt := range opts {
cfg = opt.ApplyHTTPOption(cfg)
}

return c
for pathPtr, defaultPath := range map[*string]string{
&cfg.Metrics.URLPath: DefaultMetricsPath,
} {
tmp := strings.TrimSpace(*pathPtr)
if tmp == "" {
tmp = defaultPath
} else {
tmp = path.Clean(tmp)
if !path.IsAbs(tmp) {
tmp = fmt.Sprintf("/%s", tmp)
}
}
*pathPtr = tmp
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
}
return cfg
}

// NewGRPCConfig returns a new Config with all settings applied from opts and
// any unset setting using the default gRPC config values.
func NewGRPCConfig(opts ...GRPCOption) Config {
cfg := NewDefaultConfig()
cfg := Config{
Metrics: SignalConfig{
Endpoint: fmt.Sprintf("%s:%d", DefaultCollectorHost, DefaultCollectorGRPCPort),
URLPath: DefaultMetricsPath,
Compression: NoCompression,
Timeout: DefaultTimeout,
},
RetryConfig: retry.DefaultConfig,
}
cfg = ApplyGRPCEnvConfigs(cfg)
for _, opt := range opts {
cfg = opt.ApplyGRPCOption(cfg)
Expand Down
20 changes: 14 additions & 6 deletions exporters/otlp/otlpmetric/internal/otlpconfig/options_test.go
Expand Up @@ -76,7 +76,11 @@ func TestConfigs(t *testing.T) {
{
name: "Test default configs",
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "localhost:4317", c.Metrics.Endpoint)
if grpcOption {
assert.Equal(t, "localhost:4317", c.Metrics.Endpoint)
} else {
assert.Equal(t, "localhost:4318", c.Metrics.Endpoint)
}
assert.Equal(t, otlpconfig.NoCompression, c.Metrics.Compression)
assert.Equal(t, map[string]string(nil), c.Metrics.Headers)
assert.Equal(t, 10*time.Second, c.Metrics.Timeout)
Expand Down Expand Up @@ -386,11 +390,7 @@ func TestConfigs(t *testing.T) {
t.Cleanup(func() { otlpconfig.DefaultEnvOptionsReader = origEOR })

// Tests Generic options as HTTP Options
cfg := otlpconfig.NewDefaultConfig()
cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg)
for _, opt := range tt.opts {
cfg = opt.ApplyHTTPOption(cfg)
}
cfg := otlpconfig.NewHTTPConfig(asHTTPOptions(tt.opts)...)
tt.asserts(t, &cfg, false)

// Tests Generic options as gRPC Options
Expand All @@ -400,6 +400,14 @@ func TestConfigs(t *testing.T) {
}
}

func asHTTPOptions(opts []otlpconfig.GenericOption) []otlpconfig.HTTPOption {
converted := make([]otlpconfig.HTTPOption, len(opts))
for i, o := range opts {
converted[i] = otlpconfig.NewHTTPOption(o.ApplyHTTPOption)
}
return converted
}

func asGRPCOptions(opts []otlpconfig.GenericOption) []otlpconfig.GRPCOption {
converted := make([]otlpconfig.GRPCOption, len(opts))
for i, o := range opts {
Expand Down
Expand Up @@ -17,9 +17,10 @@ package otlpconfig // import "go.opentelemetry.io/otel/exporters/otlp/otlpmetric
import "time"

const (
// DefaultCollectorPort is the port the Exporter will attempt connect to
// if no collector port is provided.
DefaultCollectorPort uint16 = 4317
// DefaultCollectorGRPCPort is the default gRPC port of the collector.
DefaultCollectorGRPCPort uint16 = 4317
// DefaultCollectorHTTPPort is the default HTTP port of the collector.
DefaultCollectorHTTPPort uint16 = 4318
// DefaultCollectorHost is the host address the Exporter will attempt
// connect to if no collector address is provided.
DefaultCollectorHost string = "localhost"
Expand Down
23 changes: 1 addition & 22 deletions exporters/otlp/otlpmetric/otlpmetrichttp/client.go
Expand Up @@ -24,9 +24,7 @@ import (
"net"
"net/http"
"net/url"
"path"
"strconv"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -77,26 +75,7 @@ type client struct {

// NewClient creates a new HTTP metric client.
func NewClient(opts ...Option) otlpmetric.Client {
cfg := otlpconfig.NewDefaultConfig()
cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg)
for _, opt := range opts {
cfg = opt.applyHTTPOption(cfg)
}

for pathPtr, defaultPath := range map[*string]string{
&cfg.Metrics.URLPath: otlpconfig.DefaultMetricsPath,
} {
tmp := strings.TrimSpace(*pathPtr)
if tmp == "" {
tmp = defaultPath
} else {
tmp = path.Clean(tmp)
if !path.IsAbs(tmp) {
tmp = fmt.Sprintf("/%s", tmp)
}
}
*pathPtr = tmp
}
cfg := otlpconfig.NewHTTPConfig(asHTTPOptions(opts)...)

httpClient := &http.Client{
Transport: ourTransport,
Expand Down
17 changes: 12 additions & 5 deletions exporters/otlp/otlpmetric/otlpmetrichttp/options.go
Expand Up @@ -40,6 +40,14 @@ type Option interface {
applyHTTPOption(otlpconfig.Config) otlpconfig.Config
}

func asHTTPOptions(opts []Option) []otlpconfig.HTTPOption {
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
converted := make([]otlpconfig.HTTPOption, len(opts))
for i, o := range opts {
converted[i] = otlpconfig.NewHTTPOption(o.applyHTTPOption)
}
return converted
}

// RetryConfig defines configuration for retrying batches in case of export
// failure using an exponential backoff.
type RetryConfig retry.Config
Expand All @@ -52,11 +60,10 @@ func (w wrappedOption) applyHTTPOption(cfg otlpconfig.Config) otlpconfig.Config
return w.ApplyHTTPOption(cfg)
}

// WithEndpoint allows one to set the address of the collector
// endpoint that the driver will use to send metrics. If
// unset, it will instead try to use
// the default endpoint (localhost:4317). Note that the endpoint
// must not contain any URL path.
// WithEndpoint allows one to set the address of the collector endpoint that
// the driver will use to send metrics. If unset, it will instead try to use
// the default endpoint (localhost:4318). Note that the endpoint must not
// contain any URL path.
func WithEndpoint(endpoint string) Option {
return wrappedOption{otlpconfig.WithEndpoint(endpoint)}
}
Expand Down
16 changes: 9 additions & 7 deletions exporters/otlp/otlptrace/README.md
Expand Up @@ -38,12 +38,14 @@ override the default configuration. For more information about how each of
these environment variables is interpreted, see [the OpenTelemetry
specification](https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/protocol/exporter.md).

| Environment variable | Option | Default value |
| ------------------------------------------------------------------------ |------------------------------ | ----------------------------------- |
| `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` | `WithEndpoint` `WithInsecure` | `https://localhost:4317` |
| `OTEL_EXPORTER_OTLP_CERTIFICATE` `OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE` | `WithTLSClientConfig` | |
| `OTEL_EXPORTER_OTLP_HEADERS` `OTEL_EXPORTER_OTLP_TRACES_HEADERS` | `WithHeaders` | |
| `OTEL_EXPORTER_OTLP_COMPRESSION` `OTEL_EXPORTER_OTLP_TRACES_COMPRESSION` | `WithCompression` | |
| `OTEL_EXPORTER_OTLP_TIMEOUT` `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` | `WithTimeout` | `10s` |
| Environment variable | Option | Default value |
| ------------------------------------------------------------------------ |------------------------------ | -------------------------------------------------------- |
| `OTEL_EXPORTER_OTLP_ENDPOINT` `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` | `WithEndpoint` `WithInsecure` | `https://localhost:4317` or `https://localhost:4318`[^1] |
| `OTEL_EXPORTER_OTLP_CERTIFICATE` `OTEL_EXPORTER_OTLP_TRACES_CERTIFICATE` | `WithTLSClientConfig` | |
| `OTEL_EXPORTER_OTLP_HEADERS` `OTEL_EXPORTER_OTLP_TRACES_HEADERS` | `WithHeaders` | |
| `OTEL_EXPORTER_OTLP_COMPRESSION` `OTEL_EXPORTER_OTLP_TRACES_COMPRESSION` | `WithCompression` | |
| `OTEL_EXPORTER_OTLP_TIMEOUT` `OTEL_EXPORTER_OTLP_TRACES_TIMEOUT` | `WithTimeout` | `10s` |

[^1]: The gRPC client defaults to `https://localhost:4317` and the HTTP client `https://localhost:4318`.

Configuration using options have precedence over the environment variables.
40 changes: 35 additions & 5 deletions exporters/otlp/otlptrace/internal/otlpconfig/options.go
Expand Up @@ -17,6 +17,8 @@ package otlpconfig // import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/
import (
"crypto/tls"
"fmt"
"path"
"strings"
"time"

"google.golang.org/grpc"
Expand Down Expand Up @@ -65,24 +67,52 @@ type (
}
)

func NewDefaultConfig() Config {
c := Config{
// NewHTTPConfig returns a new Config with all settings applied from opts and
// any unset setting using the default HTTP config values.
func NewHTTPConfig(opts ...HTTPOption) Config {
cfg := Config{
Traces: SignalConfig{
Endpoint: fmt.Sprintf("%s:%d", DefaultCollectorHost, DefaultCollectorPort),
Endpoint: fmt.Sprintf("%s:%d", DefaultCollectorHost, DefaultCollectorHTTPPort),
URLPath: DefaultTracesPath,
Compression: NoCompression,
Timeout: DefaultTimeout,
},
RetryConfig: retry.DefaultConfig,
}
cfg = ApplyHTTPEnvConfigs(cfg)
for _, opt := range opts {
cfg = opt.ApplyHTTPOption(cfg)
}

return c
for pathPtr, defaultPath := range map[*string]string{
&cfg.Traces.URLPath: DefaultTracesPath,
} {
tmp := strings.TrimSpace(*pathPtr)
if tmp == "" {
tmp = defaultPath
} else {
tmp = path.Clean(tmp)
if !path.IsAbs(tmp) {
tmp = fmt.Sprintf("/%s", tmp)
}
}
*pathPtr = tmp
}
MrAlias marked this conversation as resolved.
Show resolved Hide resolved
return cfg
}

// NewGRPCConfig returns a new Config with all settings applied from opts and
// any unset setting using the default gRPC config values.
func NewGRPCConfig(opts ...GRPCOption) Config {
cfg := NewDefaultConfig()
cfg := Config{
Traces: SignalConfig{
Endpoint: fmt.Sprintf("%s:%d", DefaultCollectorHost, DefaultCollectorGRPCPort),
URLPath: DefaultTracesPath,
Compression: NoCompression,
Timeout: DefaultTimeout,
},
RetryConfig: retry.DefaultConfig,
}
cfg = ApplyGRPCEnvConfigs(cfg)
for _, opt := range opts {
cfg = opt.ApplyGRPCOption(cfg)
Expand Down
20 changes: 14 additions & 6 deletions exporters/otlp/otlptrace/internal/otlpconfig/options_test.go
Expand Up @@ -76,7 +76,11 @@ func TestConfigs(t *testing.T) {
{
name: "Test default configs",
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "localhost:4317", c.Traces.Endpoint)
if grpcOption {
assert.Equal(t, "localhost:4317", c.Traces.Endpoint)
} else {
assert.Equal(t, "localhost:4318", c.Traces.Endpoint)
}
assert.Equal(t, otlpconfig.NoCompression, c.Traces.Compression)
assert.Equal(t, map[string]string(nil), c.Traces.Headers)
assert.Equal(t, 10*time.Second, c.Traces.Timeout)
Expand Down Expand Up @@ -384,11 +388,7 @@ func TestConfigs(t *testing.T) {
t.Cleanup(func() { otlpconfig.DefaultEnvOptionsReader = origEOR })

// Tests Generic options as HTTP Options
cfg := otlpconfig.NewDefaultConfig()
cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg)
for _, opt := range tt.opts {
cfg = opt.ApplyHTTPOption(cfg)
}
cfg := otlpconfig.NewHTTPConfig(asHTTPOptions(tt.opts)...)
tt.asserts(t, &cfg, false)

// Tests Generic options as gRPC Options
Expand All @@ -398,6 +398,14 @@ func TestConfigs(t *testing.T) {
}
}

func asHTTPOptions(opts []otlpconfig.GenericOption) []otlpconfig.HTTPOption {
converted := make([]otlpconfig.HTTPOption, len(opts))
for i, o := range opts {
converted[i] = otlpconfig.NewHTTPOption(o.ApplyHTTPOption)
}
return converted
}

func asGRPCOptions(opts []otlpconfig.GenericOption) []otlpconfig.GRPCOption {
converted := make([]otlpconfig.GRPCOption, len(opts))
for i, o := range opts {
Expand Down
7 changes: 4 additions & 3 deletions exporters/otlp/otlptrace/internal/otlpconfig/optiontypes.go
Expand Up @@ -15,9 +15,10 @@
package otlpconfig // import "go.opentelemetry.io/otel/exporters/otlp/otlptrace/internal/otlpconfig"

const (
// DefaultCollectorPort is the port the Exporter will attempt connect to
// if no collector port is provided.
DefaultCollectorPort uint16 = 4317
// DefaultCollectorGRPCPort is the default gRPC port of the collector.
DefaultCollectorGRPCPort uint16 = 4317
// DefaultCollectorHTTPPort is the default HTTP port of the collector.
DefaultCollectorHTTPPort uint16 = 4318
// DefaultCollectorHost is the host address the Exporter will attempt
// connect to if no collector address is provided.
DefaultCollectorHost string = "localhost"
Expand Down
23 changes: 1 addition & 22 deletions exporters/otlp/otlptrace/otlptracehttp/client.go
Expand Up @@ -24,9 +24,7 @@ import (
"net"
"net/http"
"net/url"
"path"
"strconv"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -79,26 +77,7 @@ var _ otlptrace.Client = (*client)(nil)

// NewClient creates a new HTTP trace client.
func NewClient(opts ...Option) otlptrace.Client {
cfg := otlpconfig.NewDefaultConfig()
cfg = otlpconfig.ApplyHTTPEnvConfigs(cfg)
for _, opt := range opts {
cfg = opt.applyHTTPOption(cfg)
}

for pathPtr, defaultPath := range map[*string]string{
&cfg.Traces.URLPath: otlpconfig.DefaultTracesPath,
} {
tmp := strings.TrimSpace(*pathPtr)
if tmp == "" {
tmp = defaultPath
} else {
tmp = path.Clean(tmp)
if !path.IsAbs(tmp) {
tmp = fmt.Sprintf("/%s", tmp)
}
}
*pathPtr = tmp
}
cfg := otlpconfig.NewHTTPConfig(asHTTPOptions(opts)...)

httpClient := &http.Client{
Transport: ourTransport,
Expand Down