Skip to content

Commit

Permalink
Update the review comment on PR 4160
Browse files Browse the repository at this point in the history
  • Loading branch information
parauliya committed Jul 4, 2023
1 parent b40d8f1 commit ede596c
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 15 deletions.
9 changes: 8 additions & 1 deletion exporters/otlp/otlpmetric/internal/oconf/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,16 @@ func NewGRPCOption(fn func(cfg Config) Config) GRPCOption {
return &grpcOption{fn: fn}
}

func WithGRPCEndpoint(endpoint string) GenericOption {
return newGenericOption(func(cfg Config) Config {
cfg.Metrics.Endpoint = endpoint
return cfg
})
}

// Generic Options

func WithEndpoint(endpoint string) GenericOption {
func WithHTTPEndpoint(endpoint string) GenericOption {
return newGenericOption(func(cfg Config) Config {
// Add scheme if not present
if !internal.HasScheme(endpoint) {
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlpmetric/otlpmetricgrpc/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func WithInsecure() Option {
//
// This option has no effect if WithGRPCConn is used.
func WithEndpoint(endpoint string) Option {
return wrappedOption{oconf.WithEndpoint(endpoint)}
return wrappedOption{oconf.WithGRPCEndpoint(endpoint)}
}

// WithReconnectionPeriod set the minimum amount of time between connection
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlpmetric/otlpmetrichttp/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (w wrappedOption) applyHTTPOption(cfg oconf.Config) oconf.Config {
// By default, if an environment variable is not set, and this option is not
// passed, "localhost:4318" will be used.
func WithEndpoint(endpoint string) Option {
return wrappedOption{oconf.WithEndpoint(endpoint)}
return wrappedOption{oconf.WithHTTPEndpoint(endpoint)}
}

// WithCompression sets the compression strategy the Exporter will use to
Expand Down
17 changes: 14 additions & 3 deletions exporters/otlp/otlptrace/internal/otlpconfig/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,20 @@ func NewGRPCOption(fn func(cfg Config) Config) GRPCOption {
return &grpcOption{fn: fn}
}

func NewGRPCOption(fn func(cfg Config) Config) GRPCOption {
return &grpcOption{fn: fn}
}

func WithGRPCEndpoint(endpoint string) GenericOption {
return newGenericOption(func(cfg Config) Config {
cfg.Metrics.Endpoint = endpoint
return cfg
})
}

// Generic Options

func WithEndpoint(endpoint string) GenericOption {
func WithHTTPEndpoint(endpoint string) GenericOption {
return newGenericOption(func(cfg Config) Config {
// Add scheme if not present
if !internal.HasScheme(endpoint) {
Expand All @@ -256,11 +267,11 @@ func WithEndpoint(endpoint string) GenericOption {
global.Error(err, "parse url", "input", endpoint)
return cfg
}
cfg.Traces.Endpoint = u.Host
cfg.Metrics.Endpoint = u.Host
// For OTLP/HTTP endpoint URLs without a per-signal
// configuration, the passed endpoint is used as a base URL
// and the signals are sent to these paths relative to that.
cfg.Traces.URLPath = path.Join(u.Path, DefaultTracesPath)
cfg.Metrics.URLPath = path.Join(u.Path, DefaultMetricsPath)
return cfg
})
}
Expand Down
45 changes: 38 additions & 7 deletions exporters/otlp/otlptrace/internal/otlpconfig/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,10 @@ func TestConfigs(t *testing.T) {
},
},

// Endpoint Tests
{
name: "Test With Endpoint",
opts: []otlpconfig.GenericOption{
otlpconfig.WithEndpoint("https://127.0.0.1:1234"),
otlpconfig.WithHTTPEndpoint("https://127.0.0.1:1234"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "127.0.0.1:1234", c.Traces.Endpoint)
Expand All @@ -101,7 +100,39 @@ func TestConfigs(t *testing.T) {
{
name: "Test With Endpoint without scheme",
opts: []otlpconfig.GenericOption{
otlpconfig.WithEndpoint("127.0.0.1:4318"),
otlpconfig.WithHTTPEndpoint("127.0.0.1:4318"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "127.0.0.1:4318", c.Traces.Endpoint)
assert.Equal(t, otlpconfig.DefaultTracesPath, c.Traces.URLPath)
},
},

// GRPC Endpoint Test
{
name: "Test With Endpoint",
opts: []otlpconfig.GenericOption{
otlpconfig.WithHTTPEndpoint("https://127.0.0.1:1234"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "127.0.0.1:1234", c.Traces.Endpoint)
},
},

// HTTP Endpoint Tests
{
name: "Test With Endpoint",
opts: []otlpconfig.GenericOption{
otlpconfig.WithHTTPEndpoint("https://127.0.0.1:1234"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "127.0.0.1:1234", c.Traces.Endpoint)
},
},
{
name: "Test With Endpoint without scheme",
opts: []otlpconfig.GenericOption{
otlpconfig.WithHTTPEndpoint("127.0.0.1:4318"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "127.0.0.1:4318", c.Traces.Endpoint)
Expand All @@ -111,7 +142,7 @@ func TestConfigs(t *testing.T) {
{
name: "Test With Endpoint with invalid URL",
opts: []otlpconfig.GenericOption{
otlpconfig.WithEndpoint(" 127.0.0.1:4318"),
otlpconfig.WithHTTPEndpoint(" 127.0.0.1:4318"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
if grpcOption {
Expand All @@ -126,7 +157,7 @@ func TestConfigs(t *testing.T) {
name: "Test With Endpoint with no scheme and 'insecure' config",
opts: []otlpconfig.GenericOption{
otlpconfig.WithInsecure(),
otlpconfig.WithEndpoint("127.0.0.1:4318"),
otlpconfig.WithHTTPEndpoint("127.0.0.1:4318"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "127.0.0.1:4318", c.Traces.Endpoint)
Expand All @@ -136,7 +167,7 @@ func TestConfigs(t *testing.T) {
{
name: "Test With Endpoint with upper case scheme",
opts: []otlpconfig.GenericOption{
otlpconfig.WithEndpoint("HTTPS://127.0.0.1:4318"),
otlpconfig.WithHTTPEndpoint("HTTPS://127.0.0.1:4318"),
},
asserts: func(t *testing.T, c *otlpconfig.Config, grpcOption bool) {
assert.Equal(t, "127.0.0.1:4318", c.Traces.Endpoint)
Expand Down Expand Up @@ -175,7 +206,7 @@ func TestConfigs(t *testing.T) {
{
name: "Test Mixed Environment and With Endpoint",
opts: []otlpconfig.GenericOption{
otlpconfig.WithEndpoint("https://127.0.0.1:1234"),
otlpconfig.WithHTTPEndpoint("https://127.0.0.1:1234"),
},
env: map[string]string{
"OTEL_EXPORTER_OTLP_ENDPOINT": "env_endpoint",
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlptrace/otlptracegrpc/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func WithInsecure() Option {
//
// This option has no effect if WithGRPCConn is used.
func WithEndpoint(endpoint string) Option {
return wrappedOption{otlpconfig.WithEndpoint(endpoint)}
return wrappedOption{otlpconfig.WithGRPCEndpoint(endpoint)}
}

// WithReconnectionPeriod set the minimum amount of time between connection
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/otlptrace/otlptracehttp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (w wrappedOption) applyHTTPOption(cfg otlpconfig.Config) otlpconfig.Config
// 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)}
return wrappedOption{otlpconfig.WithHTTPEndpoint(endpoint)}
}

// WithCompression tells the driver to compress the sent data.
Expand Down
10 changes: 10 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A=
Expand All @@ -9,9 +10,18 @@ github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
go.opentelemetry.io/otel v1.16.0/go.mod h1:vl0h9NUa1D5s1nv3A5vZOYWn8av4K8Ml6JDeHrT/bx4=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=

0 comments on commit ede596c

Please sign in to comment.