Skip to content
This repository has been archived by the owner on Aug 30, 2019. It is now read-only.

writer: make internal identifiers private #504

Merged
merged 4 commits into from
Oct 25, 2018
Merged

writer: make internal identifiers private #504

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8793e43
writer: absorb BasePayloadSender into QueuablePayloadSender
gbbr Oct 24, 2018
d6baf32
writer: rename QueuablePayloadSender to queuableSender
gbbr Oct 24, 2018
e9a47fa
writer: unexported Sender, Payload and Endpoint.
gbbr Oct 24, 2018
0f05969
writer: upper-case interface methods
gbbr Oct 25, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions writer/datadog_endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,25 @@ var userAgent = fmt.Sprintf(
userAgentPrefix, info.Version, info.GitCommit, userAgentSupportURL,
)

// DatadogEndpoint sends payloads to Datadog API.
type DatadogEndpoint struct {
APIKey string
Host string

// datadogEndpoint sends payloads to Datadog API.
type datadogEndpoint struct {
apiKey string
host string
client *http.Client
path string
}

// NewEndpoints returns the set of endpoints configured in the AgentConfig, appending the given path.
// The first endpoint is the main API endpoint, followed by any additional endpoints.
func NewEndpoints(conf *config.AgentConfig, path string) []Endpoint {
func newEndpoints(conf *config.AgentConfig, path string) []endpoint {
if !conf.Enabled {
log.Info("API interface is disabled, flushing to /dev/null instead")
return []Endpoint{&NullEndpoint{}}
return []endpoint{&nullEndpoint{}}
}
if e := conf.Endpoints; len(e) == 0 || e[0].Host == "" || e[0].APIKey == "" {
panic(errors.New("must have at least one endpoint with key"))
}
endpoints := make([]Endpoint, len(conf.Endpoints))
endpoints := make([]endpoint, len(conf.Endpoints))
ignoreProxy := true
client := newClient(conf, !ignoreProxy)
clientIgnoreProxy := newClient(conf, ignoreProxy)
Expand All @@ -54,36 +53,38 @@ func NewEndpoints(conf *config.AgentConfig, path string) []Endpoint {
if e.NoProxy {
c = clientIgnoreProxy
}
endpoints[i] = &DatadogEndpoint{
APIKey: e.APIKey,
Host: e.Host,
endpoints[i] = &datadogEndpoint{
apiKey: e.APIKey,
host: e.Host,
path: path,
client: c,
}
}
return endpoints
}

// BaseURL implements Endpoint.
func (e *DatadogEndpoint) BaseURL() string { return e.Host }
// baseURL implements Endpoint.
func (e *datadogEndpoint) baseURL() string { return e.host }

// Write will send the serialized traces payload to the Datadog traces endpoint.
func (e *DatadogEndpoint) Write(payload *Payload) error {
// write will send the serialized traces payload to the Datadog traces endpoint.
func (e *datadogEndpoint) write(payload *payload) error {
// Create the request to be sent to the API
url := e.Host + e.path
req, err := http.NewRequest("POST", url, bytes.NewBuffer(payload.Bytes))
url := e.host + e.path
req, err := http.NewRequest("POST", url, bytes.NewBuffer(payload.bytes))
if err != nil {
return err
}

req.Header.Set("DD-Api-Key", e.APIKey)
req.Header.Set("DD-Api-Key", e.apiKey)
req.Header.Set("User-Agent", userAgent)
SetExtraHeaders(req.Header, payload.Headers)
for key, value := range payload.headers {
req.Header.Set(key, value)
}

resp, err := e.client.Do(req)

if err != nil {
return &RetriableError{
return &retriableError{
err: err,
endpoint: e,
}
Expand All @@ -96,7 +97,7 @@ func (e *DatadogEndpoint) Write(payload *Payload) error {
err := fmt.Errorf("request to %s responded with %s", url, resp.Status)
if resp.StatusCode/100 == 5 {
// 5xx errors are retriable
return &RetriableError{
return &retriableError{
err: err,
endpoint: e,
}
Expand All @@ -110,8 +111,8 @@ func (e *DatadogEndpoint) Write(payload *Payload) error {
return nil
}

func (e *DatadogEndpoint) String() string {
return fmt.Sprintf("DataDogEndpoint(%q)", e.Host+e.path)
func (e *datadogEndpoint) String() string {
return fmt.Sprintf("DataDogEndpoint(%q)", e.host+e.path)
}

// timeout is the HTTP timeout for POST requests to the Datadog backend
Expand Down
34 changes: 17 additions & 17 deletions writer/datadog_endpoint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ func TestNewClient(t *testing.T) {

func TestNewEndpoints(t *testing.T) {
t.Run("disabled", func(t *testing.T) {
e := NewEndpoints(&config.AgentConfig{Enabled: false}, "")
_, ok := e[0].(*NullEndpoint)
e := newEndpoints(&config.AgentConfig{Enabled: false}, "")
_, ok := e[0].(*nullEndpoint)
assert.True(t, ok)
})

Expand All @@ -68,7 +68,7 @@ func TestNewEndpoints(t *testing.T) {
}
}
}()
NewEndpoints(tt.cfg, "")
newEndpoints(tt.cfg, "")
})
}
})
Expand All @@ -77,12 +77,12 @@ func TestNewEndpoints(t *testing.T) {
for name, tt := range map[string]struct {
cfg *config.AgentConfig
path string
exp []*DatadogEndpoint
exp []*datadogEndpoint
}{
"main": {
cfg: &config.AgentConfig{Enabled: true, Endpoints: []*config.Endpoint{{Host: "host1", APIKey: "key1"}}},
path: "/api/trace",
exp: []*DatadogEndpoint{{Host: "host1", APIKey: "key1", path: "/api/trace"}},
exp: []*datadogEndpoint{{host: "host1", apiKey: "key1", path: "/api/trace"}},
},
"additional": {
cfg: &config.AgentConfig{
Expand All @@ -95,21 +95,21 @@ func TestNewEndpoints(t *testing.T) {
},
},
path: "/api/trace",
exp: []*DatadogEndpoint{
{Host: "host1", APIKey: "key1", path: "/api/trace"},
{Host: "host2", APIKey: "key2", path: "/api/trace"},
{Host: "host3", APIKey: "key3", path: "/api/trace"},
{Host: "host4", APIKey: "key4", path: "/api/trace"},
exp: []*datadogEndpoint{
{host: "host1", apiKey: "key1", path: "/api/trace"},
{host: "host2", apiKey: "key2", path: "/api/trace"},
{host: "host3", apiKey: "key3", path: "/api/trace"},
{host: "host4", apiKey: "key4", path: "/api/trace"},
},
},
} {
t.Run(name, func(t *testing.T) {
assert := assert.New(t)
e := NewEndpoints(tt.cfg, tt.path)
e := newEndpoints(tt.cfg, tt.path)
for i, want := range tt.exp {
got := e[i].(*DatadogEndpoint)
assert.Equal(want.Host, got.Host)
assert.Equal(want.APIKey, got.APIKey)
got := e[i].(*datadogEndpoint)
assert.Equal(want.host, got.host)
assert.Equal(want.apiKey, got.apiKey)
assert.Equal(want.path, got.path)
}
})
Expand All @@ -122,7 +122,7 @@ func TestNewEndpoints(t *testing.T) {
if err != nil {
t.Fatal(err)
}
e := NewEndpoints(&config.AgentConfig{
e := newEndpoints(&config.AgentConfig{
Enabled: true,
ProxyURL: proxyURL,
Endpoints: []*config.Endpoint{
Expand All @@ -134,13 +134,13 @@ func TestNewEndpoints(t *testing.T) {

// proxy ok
for _, i := range []int{0, 1} {
tr := e[i].(*DatadogEndpoint).client.Transport.(*http.Transport)
tr := e[i].(*datadogEndpoint).client.Transport.(*http.Transport)
p, _ := tr.Proxy(nil)
assert.Equal("test_url", p.String())
}

// proxy skipped
tr := e[2].(*DatadogEndpoint).client.Transport.(*http.Transport)
tr := e[2].(*datadogEndpoint).client.Transport.(*http.Transport)
assert.Nil(tr.Proxy)
})
}
38 changes: 15 additions & 23 deletions writer/endpoint.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,49 +2,41 @@ package writer

import (
"fmt"
"net/http"

log "github.com/cihub/seelog"
)

const languageHeaderKey = "X-Datadog-Reported-Languages"

// Endpoint is an interface where we send the data from the Agent.
type Endpoint interface {
// endpoint is an interface where we send the data from the Agent.
type endpoint interface {
// Write writes the payload to the endpoint.
Write(payload *Payload) error
write(payload *payload) error
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a matter of consistency we should strive for this uppercase public interface methods everywhere, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't want to enforce that. I get the logic in the sender because of setEndpoint which is specifically there for tests, but I wouldn't want to make this a rule. If we capitalise these, it means that technically they can be accessed from outside the package too if this interface would be returned anywhere.


// BaseURL returns the base URL for this endpoint. e.g. For the URL "https://trace.agent.datadoghq.eu/api/v0.2/traces"
// baseURL returns the base URL for this endpoint. e.g. For the URL "https://trace.agent.datadoghq.eu/api/v0.2/traces"
// it returns "https://trace.agent.datadoghq.eu".
BaseURL() string
baseURL() string
}

// NullEndpoint is a void endpoint dropping data.
type NullEndpoint struct{}
// nullEndpoint is a void endpoint dropping data.
type nullEndpoint struct{}

// Write of NullEndpoint just drops the payload and log its size.
func (ne *NullEndpoint) Write(payload *Payload) error {
log.Debug("null endpoint: dropping payload, size: %d", len(payload.Bytes))
// Write of nullEndpoint just drops the payload and log its size.
func (ne *nullEndpoint) write(payload *payload) error {
log.Debug("null endpoint: dropping payload, size: %d", len(payload.bytes))
return nil
}

// BaseURL implements Endpoint.
func (ne *NullEndpoint) BaseURL() string { return "<NullEndpoint>" }
func (ne *nullEndpoint) baseURL() string { return "<nullEndpoint>" }

// SetExtraHeaders appends a header map to HTTP headers.
func SetExtraHeaders(h http.Header, extras map[string]string) {
for key, value := range extras {
h.Set(key, value)
}
}

// RetriableError is an endpoint error that signifies that the associated operation can be retried at a later point.
type RetriableError struct {
// retriableError is an endpoint error that signifies that the associated operation can be retried at a later point.
type retriableError struct {
err error
endpoint Endpoint
endpoint endpoint
}

// Error returns the error string.
func (re *RetriableError) Error() string {
func (re *retriableError) Error() string {
return fmt.Sprintf("%s: %v", re.endpoint, re.err)
}
Loading