Skip to content

Commit

Permalink
[confighttp] Change Headers field type to have opaque values (open-te…
Browse files Browse the repository at this point in the history
…lemetry#6637)

* [confighttp] Deprecate Headers field in favor of OpaqueHeaders

* Set Headers to non-nil to avoid contrib tests failure

* Do a breaking change in one step
  • Loading branch information
mx-psi committed Dec 16, 2022
1 parent 557c077 commit 892d07d
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 16 deletions.
16 changes: 16 additions & 0 deletions .chloggen/mx-psi_configopaque-http.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
component: config/confighttp

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Change confighttp.HTTPClientSettings.Headers type to map[string]configopaque.String

# One or more tracking issues or pull requests related to the change
issues: [5653]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
8 changes: 5 additions & 3 deletions config/confighttp/confighttp.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configauth"
"go.opentelemetry.io/collector/config/configcompression"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/config/internal"
"go.opentelemetry.io/collector/extension/auth"
Expand All @@ -55,7 +56,8 @@ type HTTPClientSettings struct {

// Additional headers attached to each HTTP request sent by the client.
// Existing header values are overwritten if collision happens.
Headers map[string]string `mapstructure:"headers"`
// Header values are opaque since they may be sensitive.
Headers map[string]configopaque.String `mapstructure:"headers"`

// Custom Round Tripper to allow for individual components to intercept HTTP requests
CustomRoundTripper func(next http.RoundTripper) (http.RoundTripper, error)
Expand Down Expand Up @@ -188,13 +190,13 @@ func (hcs *HTTPClientSettings) ToClient(host component.Host, settings component.
// Custom RoundTripper that adds headers.
type headerRoundTripper struct {
transport http.RoundTripper
headers map[string]string
headers map[string]configopaque.String
}

// RoundTrip is a custom RoundTripper that adds headers to the request.
func (interceptor *headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
for k, v := range interceptor.headers {
req.Header.Set(k, v)
req.Header.Set(k, string(v))
}
// Send the request to next transport.
return interceptor.transport.RoundTrip(req)
Expand Down
13 changes: 6 additions & 7 deletions config/confighttp/confighttp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configauth"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/extension/auth"
"go.opentelemetry.io/collector/extension/auth/authtest"
Expand Down Expand Up @@ -824,11 +825,11 @@ func ExampleHTTPServerSettings() {
func TestHttpHeaders(t *testing.T) {
tests := []struct {
name string
headers map[string]string
headers map[string]configopaque.String
}{
{
"with_headers",
map[string]string{
name: "with_headers",
headers: map[string]configopaque.String{
"header1": "value1",
},
},
Expand All @@ -837,7 +838,7 @@ func TestHttpHeaders(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
for k, v := range tt.headers {
assert.Equal(t, r.Header.Get(k), v)
assert.Equal(t, r.Header.Get(k), string(v))
}
w.WriteHeader(200)
}))
Expand All @@ -849,9 +850,7 @@ func TestHttpHeaders(t *testing.T) {
ReadBufferSize: 0,
WriteBufferSize: 0,
Timeout: 0,
Headers: map[string]string{
"header1": "value1",
},
Headers: tt.headers,
}
client, _ := setting.ToClient(componenttest.NewNopHost(), componenttest.NewNopTelemetrySettings())
req, err := http.NewRequest("GET", setting.Endpoint, nil)
Expand Down
3 changes: 2 additions & 1 deletion exporter/otlphttpexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/confmap"
"go.opentelemetry.io/collector/confmap/confmaptest"
Expand Down Expand Up @@ -59,7 +60,7 @@ func TestUnmarshalConfig(t *testing.T) {
QueueSize: 10,
},
HTTPClientSettings: confighttp.HTTPClientSettings{
Headers: map[string]string{
Headers: map[string]configopaque.String{
"can you have a . here?": "F0000000-0000-0000-0000-000000000000",
"header1": "234",
"another": "somevalue",
Expand Down
3 changes: 2 additions & 1 deletion exporter/otlphttpexporter/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/config/configcompression"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/exporter"
"go.opentelemetry.io/collector/exporter/exporterhelper"
Expand Down Expand Up @@ -51,7 +52,7 @@ func createDefaultConfig() component.Config {
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: "",
Timeout: 30 * time.Second,
Headers: map[string]string{},
Headers: map[string]configopaque.String{},
// Default to gzip compression
Compression: configcompression.Gzip,
// We almost read 0 bytes, so no need to tune ReadBufferSize.
Expand Down
3 changes: 2 additions & 1 deletion exporter/otlphttpexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/configcompression"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/exporter/exportertest"
"go.opentelemetry.io/collector/internal/testutil"
Expand Down Expand Up @@ -93,7 +94,7 @@ func TestCreateTracesExporter(t *testing.T) {
config: Config{
HTTPClientSettings: confighttp.HTTPClientSettings{
Endpoint: endpoint,
Headers: map[string]string{
Headers: map[string]configopaque.String{
"hdr1": "val1",
"hdr2": "val2",
},
Expand Down
7 changes: 4 additions & 3 deletions exporter/otlphttpexporter/otlp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/component/componenttest"
"go.opentelemetry.io/collector/config/confighttp"
"go.opentelemetry.io/collector/config/configopaque"
"go.opentelemetry.io/collector/consumer"
"go.opentelemetry.io/collector/consumer/consumererror"
"go.opentelemetry.io/collector/consumer/consumertest"
Expand Down Expand Up @@ -525,7 +526,7 @@ func TestUserAgent(t *testing.T) {

tests := []struct {
name string
headers map[string]string
headers map[string]configopaque.String
expectedUA string
}{
{
Expand All @@ -534,12 +535,12 @@ func TestUserAgent(t *testing.T) {
},
{
name: "custom_user_agent",
headers: map[string]string{"User-Agent": "My Custom Agent"},
headers: map[string]configopaque.String{"User-Agent": "My Custom Agent"},
expectedUA: "My Custom Agent",
},
{
name: "custom_user_agent_lowercase",
headers: map[string]string{"user-agent": "My Custom Agent"},
headers: map[string]configopaque.String{"user-agent": "My Custom Agent"},
expectedUA: "My Custom Agent",
},
}
Expand Down

0 comments on commit 892d07d

Please sign in to comment.