Skip to content

Commit

Permalink
Change awsutil to use host instead of empty extensions map (open-tele…
Browse files Browse the repository at this point in the history
…metry#12774)

In the RestClient did not change since it is created during component creation (not during start) and access to Host is not available.

Signed-off-by: Bogdan Drutu <[email protected]>
  • Loading branch information
bogdandrutu committed Jul 29, 2022
1 parent bdf8d83 commit 6f599c7
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 6 deletions.
9 changes: 6 additions & 3 deletions internal/aws/ecsutil/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"net/url"

"go.opentelemetry.io/collector/component"
cconfig "go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/confighttp"
"go.uber.org/zap"

Expand All @@ -34,10 +33,11 @@ type Client interface {
}

// NewClientProvider creates the default rest client provider
func NewClientProvider(baseURL url.URL, clientSettings confighttp.HTTPClientSettings, settings component.TelemetrySettings) ClientProvider {
func NewClientProvider(baseURL url.URL, clientSettings confighttp.HTTPClientSettings, host component.Host, settings component.TelemetrySettings) ClientProvider {
return &defaultClientProvider{
baseURL: baseURL,
clientSettings: clientSettings,
host: host,
settings: settings,
}
}
Expand All @@ -50,23 +50,26 @@ type ClientProvider interface {
type defaultClientProvider struct {
baseURL url.URL
clientSettings confighttp.HTTPClientSettings
host component.Host
settings component.TelemetrySettings
}

func (dcp *defaultClientProvider) BuildClient() (Client, error) {
return defaultClient(
dcp.baseURL,
dcp.clientSettings,
dcp.host,
dcp.settings,
)
}

func defaultClient(
baseURL url.URL,
clientSettings confighttp.HTTPClientSettings,
host component.Host,
settings component.TelemetrySettings,
) (*clientImpl, error) {
client, err := clientSettings.ToClient(map[cconfig.ComponentID]component.Extension{}, settings)
client, err := clientSettings.ToClientWithHost(host, settings)
if err != nil {
return nil, err
}
Expand Down
7 changes: 5 additions & 2 deletions internal/aws/ecsutil/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestClient(t *testing.T) {

func TestNewClientProvider(t *testing.T) {
baseURL, _ := url.Parse("http:https://localhost:8080")
provider := NewClientProvider(*baseURL, confighttp.HTTPClientSettings{}, componenttest.NewNopTelemetrySettings())
provider := NewClientProvider(*baseURL, confighttp.HTTPClientSettings{}, componenttest.NewNopHost(), componenttest.NewNopTelemetrySettings())
require.NotNil(t, provider)
_, ok := provider.(*defaultClientProvider)
require.True(t, ok)
Expand All @@ -59,7 +59,7 @@ func TestNewClientProvider(t *testing.T) {

func TestDefaultClient(t *testing.T) {
endpoint, _ := url.Parse("http:https://localhost:8080")
client, err := defaultClient(*endpoint, confighttp.HTTPClientSettings{}, componenttest.NewNopTelemetrySettings())
client, err := defaultClient(*endpoint, confighttp.HTTPClientSettings{}, componenttest.NewNopHost(), componenttest.NewNopTelemetrySettings())
require.NoError(t, err)
require.NotNil(t, client.httpClient.Transport)
require.Equal(t, "http:https://localhost:8080", client.baseURL.String())
Expand All @@ -69,6 +69,7 @@ func TestBuildReq(t *testing.T) {
endpoint, _ := url.Parse("http:https://localhost:8080")
p := &defaultClientProvider{
baseURL: *endpoint,
host: componenttest.NewNopHost(),
settings: componenttest.NewNopTelemetrySettings(),
}
cl, err := p.BuildClient()
Expand All @@ -84,6 +85,7 @@ func TestBuildBadReq(t *testing.T) {
endpoint, _ := url.Parse("http:https://localhost:8080")
p := &defaultClientProvider{
baseURL: *endpoint,
host: componenttest.NewNopHost(),
settings: componenttest.NewNopTelemetrySettings(),
}
cl, err := p.BuildClient()
Expand All @@ -96,6 +98,7 @@ func TestGetBad(t *testing.T) {
endpoint, _ := url.Parse("http:https://localhost:8080")
p := &defaultClientProvider{
baseURL: *endpoint,
host: componenttest.NewNopHost(),
settings: componenttest.NewNopTelemetrySettings(),
}
cl, err := p.BuildClient()
Expand Down
12 changes: 11 additions & 1 deletion internal/aws/ecsutil/rest_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@ import (
"net/url"

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config"
"go.opentelemetry.io/collector/config/confighttp"
)

func NewRestClient(baseEndpoint url.URL, clientSettings confighttp.HTTPClientSettings, settings component.TelemetrySettings) (RestClient, error) {
clientProvider := NewClientProvider(baseEndpoint, clientSettings, settings)
clientProvider := NewClientProvider(baseEndpoint, clientSettings, &nopHost{}, settings)

client, err := clientProvider.BuildClient()
if err != nil {
Expand All @@ -31,6 +32,15 @@ func NewRestClient(baseEndpoint url.URL, clientSettings confighttp.HTTPClientSet
return NewRestClientFromClient(client), nil
}

// TODO: Instead of using this, expose it as a argument to NewRestClient.
type nopHost struct {
component.Host
}

func (nh *nopHost) GetExtensions() map[config.ComponentID]component.Extension {
return map[config.ComponentID]component.Extension{}
}

// RestClient is swappable for testing.
type RestClient interface {
GetResponse(path string) ([]byte, error)
Expand Down

0 comments on commit 6f599c7

Please sign in to comment.