Skip to content

Commit

Permalink
otlp receiver: Fix HTTP server config source and legacy port mutation (
Browse files Browse the repository at this point in the history
  • Loading branch information
rmfitzpatrick committed Jan 5, 2022
1 parent 60e0cf6 commit f13a011
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

- Fix merge config map provider to close the watchers (#4570)
- Fix expand map provider to call close on the base provider (#4571)
- `otlp` receiver: Fix legacy port cfg value override and HTTP server starting bug (#4631)

## v0.41.0 Beta

Expand Down
6 changes: 3 additions & 3 deletions receiver/otlpreceiver/otlp.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func (r *otlpReceiver) startGRPCServer(cfg *configgrpc.GRPCServerSettings, host
func (r *otlpReceiver) startHTTPServer(cfg *confighttp.HTTPServerSettings, host component.Host) error {
r.settings.Logger.Info("Starting HTTP server on endpoint " + cfg.Endpoint)
var hln net.Listener
hln, err := r.cfg.HTTP.ToListener()
hln, err := cfg.ToListener()
if err != nil {
return err
}
Expand Down Expand Up @@ -147,10 +147,10 @@ func (r *otlpReceiver) startProtocolServers(host component.Host) error {
r.settings.Logger.Info("Setting up a second HTTP listener on legacy endpoint " + legacyHTTPEndpoint)

// Copy the config.
cfgLegacyHTTP := r.cfg.HTTP
cfgLegacyHTTP := *(r.cfg.HTTP)
// And use the legacy endpoint.
cfgLegacyHTTP.Endpoint = legacyHTTPEndpoint
err = r.startHTTPServer(cfgLegacyHTTP, host)
err = r.startHTTPServer(&cfgLegacyHTTP, host)
if err != nil {
return err
}
Expand Down
39 changes: 39 additions & 0 deletions receiver/otlpreceiver/otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ import (
"io/ioutil"
"net"
"net/http"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/zap"
"go.uber.org/zap/zaptest/observer"
spb "google.golang.org/genproto/googleapis/rpc/status"
"google.golang.org/grpc"
"google.golang.org/grpc/codes"
Expand All @@ -46,6 +49,7 @@ import (
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/consumertest"
"go.opentelemetry.io/collector/internal/internalconsumertest"
"go.opentelemetry.io/collector/internal/sharedcomponent"
"go.opentelemetry.io/collector/internal/testdata"
"go.opentelemetry.io/collector/internal/testutil"
"go.opentelemetry.io/collector/model/otlp"
Expand Down Expand Up @@ -629,6 +633,41 @@ func TestHTTPInvalidTLSCredentials(t *testing.T) {
`failed to load TLS config: for auth via TLS, either both certificate and key must be supplied, or neither`)
}

func TestHTTPUseLegacyPortWhenUsingDefaultEndpoint(t *testing.T) {
r := newHTTPReceiver(t, defaultHTTPEndpoint, consumertest.NewNop(), consumertest.NewNop())
require.NotNil(t, r)

logCore, logs := observer.New(zap.InfoLevel)
logger := zap.New(logCore)

metric := r.(*sharedcomponent.SharedComponent).Unwrap().(*otlpReceiver)
metric.settings.Logger = logger

t.Cleanup(func() { require.NoError(t, r.Shutdown(context.Background())) })

require.NoError(t, r.Start(context.Background(), componenttest.NewNopHost()))

require.True(t, func() bool {
for _, l := range logs.All() {
if strings.Contains(l.Message, "Setting up a second HTTP listener on legacy endpoint 0.0.0.0:55681") {
return true
}
}
return false
}())

require.False(t, func() bool {
for _, l := range logs.All() {
if strings.Contains(l.Message, "Legacy HTTP endpoint 0.0.0.0:55681 is configured, please use 0.0.0.0:4318 instead.") {
return true
}
}
return false
}())

require.Equal(t, defaultHTTPEndpoint, metric.cfg.HTTP.Endpoint)
}

func newGRPCReceiver(t *testing.T, name string, endpoint string, tc consumer.Traces, mc consumer.Metrics) component.Component {
factory := NewFactory()
cfg := factory.CreateDefaultConfig().(*Config)
Expand Down

0 comments on commit f13a011

Please sign in to comment.