-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a header round tripper option to httpcommon #27509
Changes from 11 commits
5e6e358
6d6d5a6
8f61c1c
04762fd
e5c7902
27c93e4
46ec023
d6e9027
14e7358
ab53cb4
5bd4973
5590c5c
4212819
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -531,7 +531,7 @@ func (b *Beat) Setup(settings Settings, bt beat.Creator, setup SetupSettings) er | |
if outCfg.Name() != "elasticsearch" { | ||
return fmt.Errorf("Index management requested but the Elasticsearch output is not configured/enabled") | ||
} | ||
esClient, err := eslegclient.NewConnectedClient(outCfg.Config()) | ||
esClient, err := eslegclient.NewConnectedClient(outCfg.Config(), b.Info.Name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this can be set to hostname in some cases, is that ok? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's not really what we want, is there a way we can get the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this one will be correct most of the time but i've seen some initialization with hostname. would be good to get some input from somebody who knows this a bit better There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The beatname is under |
||
if err != nil { | ||
return err | ||
} | ||
|
@@ -808,7 +808,7 @@ func (b *Beat) loadDashboards(ctx context.Context, force bool) error { | |
// initKibanaConfig will attach the username and password into kibana config as a part of the initialization. | ||
kibanaConfig := InitKibanaConfig(b.Config) | ||
|
||
client, err := kibana.NewKibanaClient(kibanaConfig) | ||
client, err := kibana.NewKibanaClient(kibanaConfig, b.Info.Name) | ||
if err != nil { | ||
return fmt.Errorf("error connecting to Kibana: %v", err) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ import ( | |
"github.com/elastic/beats/v7/libbeat/common/transport/httpcommon" | ||
"github.com/elastic/beats/v7/libbeat/common/transport/kerberos" | ||
"github.com/elastic/beats/v7/libbeat/common/transport/tlscommon" | ||
"github.com/elastic/beats/v7/libbeat/common/useragent" | ||
"github.com/elastic/beats/v7/libbeat/logp" | ||
"github.com/elastic/beats/v7/libbeat/testing" | ||
) | ||
|
@@ -57,7 +58,8 @@ type Connection struct { | |
|
||
// ConnectionSettings are the settings needed for a Connection | ||
type ConnectionSettings struct { | ||
URL string | ||
URL string | ||
Beatname string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this client useful for things other than a Beat? Having a parameter that is the beat name sort of implies that this is only usable by a Beat. An alternative would be to accept the user agent string as a parameter or call it "product name". Even if it were called "product name" the code would still assume it's a beat since it creates the user-agent string based on the beat version info. (I'm not saying you need to change it.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need to expose this one? this is also not serialized so it hiding it should not matter, wdyt? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @andrewkroh, I think that |
||
|
||
Username string | ||
Password string | ||
|
@@ -110,6 +112,11 @@ func NewConnection(s ConnectionSettings) (*Connection, error) { | |
} | ||
} | ||
|
||
if s.Beatname == "" { | ||
s.Beatname == "Libbeat" | ||
} | ||
userAgent := useragent.UserAgent(s.Beatname) | ||
|
||
httpClient, err := s.Transport.Client( | ||
httpcommon.WithLogger(logger), | ||
httpcommon.WithIOStats(s.Observer), | ||
|
@@ -119,6 +126,7 @@ func NewConnection(s ConnectionSettings) (*Connection, error) { | |
// eg, like in https://github.com/elastic/apm-server/blob/7.7/elasticsearch/client.go | ||
return apmelasticsearch.WrapRoundTripper(rt) | ||
}), | ||
httpcommon.WithHeaderRoundTripper(map[string]string{"User-Agent": userAgent}), | ||
) | ||
if err != nil { | ||
return nil, err | ||
|
@@ -160,7 +168,7 @@ func settingsWithDefaults(s ConnectionSettings) ConnectionSettings { | |
// configuration. It accepts the same configuration parameters as the Elasticsearch | ||
// output, except for the output specific configuration options. If multiple hosts | ||
// are defined in the configuration, a client is returned for each of them. | ||
func NewClients(cfg *common.Config) ([]Connection, error) { | ||
func NewClients(cfg *common.Config, beatname string) ([]Connection, error) { | ||
config := defaultConfig() | ||
if err := cfg.Unpack(&config); err != nil { | ||
return nil, err | ||
|
@@ -185,6 +193,7 @@ func NewClients(cfg *common.Config) ([]Connection, error) { | |
|
||
client, err := NewConnection(ConnectionSettings{ | ||
URL: esURL, | ||
Beatname: beatname, | ||
Kerberos: config.Kerberos, | ||
Username: config.Username, | ||
Password: config.Password, | ||
|
@@ -205,8 +214,8 @@ func NewClients(cfg *common.Config) ([]Connection, error) { | |
return clients, nil | ||
} | ||
|
||
func NewConnectedClient(cfg *common.Config) (*Connection, error) { | ||
clients, err := NewClients(cfg) | ||
func NewConnectedClient(cfg *common.Config, beatname string) (*Connection, error) { | ||
clients, err := NewClients(cfg, beatname) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing this excellent test!