From 4502692056f36fdc636c674759af503267c8e9a7 Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Tue, 2 Jul 2024 12:27:55 +0100 Subject: [PATCH 1/7] [exporter/elasticsearch] Add telemetry settings --- exporter/elasticsearchexporter/README.md | 10 +++++ exporter/elasticsearchexporter/config.go | 9 +++++ .../elasticsearch_bulk.go | 37 ++++++++++++++----- 3 files changed, 47 insertions(+), 9 deletions(-) diff --git a/exporter/elasticsearchexporter/README.md b/exporter/elasticsearchexporter/README.md index 6c2db78b52da..15042dc43d8b 100644 --- a/exporter/elasticsearchexporter/README.md +++ b/exporter/elasticsearchexporter/README.md @@ -184,6 +184,16 @@ Settings related to node discovery are: Node discovery can be disabled by setting `discover.interval` to 0. +### Telemetry settings + +The Elasticsearch Exporter's own telemetry settings for testing and debugging purposes. + +⚠️ This is experimental and may change at any time. + +- `telemetry`: + - `log_request_body` (default=false): Logs Elasticsearch client request body. + - `log_response_body` (default=false): Logs Elasticsearch client response body. + ## Exporting metrics Metrics support is currently in development. diff --git a/exporter/elasticsearchexporter/config.go b/exporter/elasticsearchexporter/config.go index 1bcabb99a325..ec745c77050a 100644 --- a/exporter/elasticsearchexporter/config.go +++ b/exporter/elasticsearchexporter/config.go @@ -72,6 +72,15 @@ type Config struct { Flush FlushSettings `mapstructure:"flush"` Mapping MappingsSettings `mapstructure:"mapping"` LogstashFormat LogstashFormatSettings `mapstructure:"logstash_format"` + + // TelemetrySettings contains settings useful for testing/debugging purposes + // This is experimental and may change at any time. + TelemetrySettings `mapstructure:"telemetry"` +} + +type TelemetrySettings struct { + LogRequestBody bool `mapstructure:"log_request_body"` + LogResponseBody bool `mapstructure:"log_response_body"` } type LogstashFormatSettings struct { diff --git a/exporter/elasticsearchexporter/elasticsearch_bulk.go b/exporter/elasticsearchexporter/elasticsearch_bulk.go index c44a66f3db43..101c360d67fe 100644 --- a/exporter/elasticsearchexporter/elasticsearch_bulk.go +++ b/exporter/elasticsearchexporter/elasticsearch_bulk.go @@ -32,12 +32,16 @@ type esBulkIndexerItem = docappender.BulkIndexerItem // clientLogger implements the estransport.Logger interface // that is required by the Elasticsearch client for logging. -type clientLogger zap.Logger +type clientLogger struct { + *zap.Logger + logRequestBody bool + logResponseBody bool +} // LogRoundTrip should not modify the request or response, except for consuming and closing the body. // Implementations have to check for nil values in request and response. func (cl *clientLogger) LogRoundTrip(requ *http.Request, resp *http.Response, err error, _ time.Time, dur time.Duration) error { - zl := (*zap.Logger)(cl) + zl := cl.Logger switch { case err == nil && resp != nil: zl.Debug("Request roundtrip completed.", @@ -50,19 +54,28 @@ func (cl *clientLogger) LogRoundTrip(requ *http.Request, resp *http.Response, er zl.Error("Request failed.", zap.NamedError("reason", err)) } + if cl.logRequestBody && requ != nil { + if b, err := io.ReadAll(requ.Body); err == nil { + zl.Debug("Request body", zap.ByteString("body", b)) + } + } + if cl.logResponseBody && resp != nil { + if b, err := io.ReadAll(resp.Body); err == nil { + zl.Debug("Response body", zap.ByteString("body", b)) + } + } + return nil } // RequestBodyEnabled makes the client pass a copy of request body to the logger. -func (*clientLogger) RequestBodyEnabled() bool { - // TODO: introduce setting log the bodies for more detailed debug logs - return false +func (cl *clientLogger) RequestBodyEnabled() bool { + return cl.logRequestBody } // ResponseBodyEnabled makes the client pass a copy of response body to the logger. -func (*clientLogger) ResponseBodyEnabled() bool { - // TODO: introduce setting log the bodies for more detailed debug logs - return false +func (cl *clientLogger) ResponseBodyEnabled() bool { + return cl.logResponseBody } func newElasticsearchClient( @@ -97,6 +110,12 @@ func newElasticsearchClient( return nil, err } + esLogger := clientLogger{ + Logger: telemetry.Logger, + logRequestBody: config.LogRequestBody, + logResponseBody: config.LogResponseBody, + } + return elasticsearch7.NewClient(esConfigCurrent{ Transport: httpClient.Transport, @@ -122,7 +141,7 @@ func newElasticsearchClient( // configure internal metrics reporting and logging EnableMetrics: false, // TODO EnableDebugLogger: false, // TODO - Logger: (*clientLogger)(telemetry.Logger), + Logger: &esLogger, }) } From 7c6f0798f376a1f534b04d527b9b518e76d57a85 Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Tue, 2 Jul 2024 12:30:23 +0100 Subject: [PATCH 2/7] Explicitly state defaults --- exporter/elasticsearchexporter/factory.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/exporter/elasticsearchexporter/factory.go b/exporter/elasticsearchexporter/factory.go index 7826fb59a47e..3fb8b295e352 100644 --- a/exporter/elasticsearchexporter/factory.go +++ b/exporter/elasticsearchexporter/factory.go @@ -84,6 +84,10 @@ func createDefaultConfig() component.Config { PrefixSeparator: "-", DateFormat: "%Y.%m.%d", }, + TelemetrySettings: TelemetrySettings{ + LogRequestBody: false, + LogResponseBody: false, + }, } } From d508e212785ed919f1e368561f2d4912c6bea652 Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Tue, 2 Jul 2024 12:32:46 +0100 Subject: [PATCH 3/7] Add changelog --- ...sticsearchexporter_telemetry_settings.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .chloggen/elasticsearchexporter_telemetry_settings.yaml diff --git a/.chloggen/elasticsearchexporter_telemetry_settings.yaml b/.chloggen/elasticsearchexporter_telemetry_settings.yaml new file mode 100644 index 000000000000..f4ae41675eca --- /dev/null +++ b/.chloggen/elasticsearchexporter_telemetry_settings.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: enhancement + +# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver) +component: elasticsearchexporter + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Introduce experimental `telemetry.log_request_body` and `telemetry.log_response_body` config + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [33854] + +# (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: + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] From 21bf3476ba973d1e6323bce5739698f20091f29f Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Tue, 2 Jul 2024 14:15:47 +0100 Subject: [PATCH 4/7] Log in 1 log line; Change request fields to debug --- .../elasticsearch_bulk.go | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/exporter/elasticsearchexporter/elasticsearch_bulk.go b/exporter/elasticsearchexporter/elasticsearch_bulk.go index 101c360d67fe..194446285bfc 100644 --- a/exporter/elasticsearchexporter/elasticsearch_bulk.go +++ b/exporter/elasticsearchexporter/elasticsearch_bulk.go @@ -42,27 +42,36 @@ type clientLogger struct { // Implementations have to check for nil values in request and response. func (cl *clientLogger) LogRoundTrip(requ *http.Request, resp *http.Response, err error, _ time.Time, dur time.Duration) error { zl := cl.Logger + + var fields []zap.Field + if cl.logRequestBody && requ != nil && requ.Body != nil { + if b, err := io.ReadAll(requ.Body); err == nil { + fields = append(fields, zap.ByteString("request_body", b)) + } + } + if cl.logResponseBody && resp != nil && resp.Body != nil { + if b, err := io.ReadAll(resp.Body); err == nil { + fields = append(fields, zap.ByteString("response_body", b)) + } + } + switch { case err == nil && resp != nil: - zl.Debug("Request roundtrip completed.", + fields = append( + fields, zap.String("path", sanitize.String(requ.URL.Path)), zap.String("method", requ.Method), zap.Duration("duration", dur), - zap.String("status", resp.Status)) + zap.String("status", resp.Status), + ) + zl.Debug("Request roundtrip completed.", fields...) case err != nil: - zl.Error("Request failed.", zap.NamedError("reason", err)) - } - - if cl.logRequestBody && requ != nil { - if b, err := io.ReadAll(requ.Body); err == nil { - zl.Debug("Request body", zap.ByteString("body", b)) - } - } - if cl.logResponseBody && resp != nil { - if b, err := io.ReadAll(resp.Body); err == nil { - zl.Debug("Response body", zap.ByteString("body", b)) - } + fields = append( + fields, + zap.NamedError("reason", err), + ) + zl.Debug("Request failed.", fields...) } return nil From 0ef3edf6de80a2a337bd96d72dd212209c111fe2 Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Tue, 2 Jul 2024 14:49:13 +0100 Subject: [PATCH 5/7] Mention log at DEBUG and security warning --- exporter/elasticsearchexporter/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporter/elasticsearchexporter/README.md b/exporter/elasticsearchexporter/README.md index 15042dc43d8b..5a93074e1c26 100644 --- a/exporter/elasticsearchexporter/README.md +++ b/exporter/elasticsearchexporter/README.md @@ -191,8 +191,8 @@ The Elasticsearch Exporter's own telemetry settings for testing and debugging pu ⚠️ This is experimental and may change at any time. - `telemetry`: - - `log_request_body` (default=false): Logs Elasticsearch client request body. - - `log_response_body` (default=false): Logs Elasticsearch client response body. + - `log_request_body` (default=false): Logs Elasticsearch client request body as a field in a log line at DEBUG level. WARNING: Enabling this config may expose sensitive data. + - `log_response_body` (default=false): Logs Elasticsearch client response body as a field in a log line at DEBUG level. WARNING: Enabling this config may expose sensitive data. ## Exporting metrics From 84a15978afe28bc9206b37d12ac45de34181226a Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Tue, 2 Jul 2024 14:51:51 +0100 Subject: [PATCH 6/7] Mention service::telemetry::logs::level --- exporter/elasticsearchexporter/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/exporter/elasticsearchexporter/README.md b/exporter/elasticsearchexporter/README.md index 5a93074e1c26..1c0597e76797 100644 --- a/exporter/elasticsearchexporter/README.md +++ b/exporter/elasticsearchexporter/README.md @@ -191,8 +191,8 @@ The Elasticsearch Exporter's own telemetry settings for testing and debugging pu ⚠️ This is experimental and may change at any time. - `telemetry`: - - `log_request_body` (default=false): Logs Elasticsearch client request body as a field in a log line at DEBUG level. WARNING: Enabling this config may expose sensitive data. - - `log_response_body` (default=false): Logs Elasticsearch client response body as a field in a log line at DEBUG level. WARNING: Enabling this config may expose sensitive data. + - `log_request_body` (default=false): Logs Elasticsearch client request body as a field in a log line at DEBUG level. It requires `service::telemetry::logs::level` to be set to `debug`. WARNING: Enabling this config may expose sensitive data. + - `log_response_body` (default=false): Logs Elasticsearch client response body as a field in a log line at DEBUG level. It requires `service::telemetry::logs::level` to be set to `debug`. WARNING: Enabling this config may expose sensitive data. ## Exporting metrics From ed26707d07e4d736a145fa5b1049595c732bdf76 Mon Sep 17 00:00:00 2001 From: Carson Ip Date: Tue, 2 Jul 2024 15:40:39 +0100 Subject: [PATCH 7/7] Make linter happy --- exporter/elasticsearchexporter/elasticsearch_bulk.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/exporter/elasticsearchexporter/elasticsearch_bulk.go b/exporter/elasticsearchexporter/elasticsearch_bulk.go index 194446285bfc..5beb768bc582 100644 --- a/exporter/elasticsearchexporter/elasticsearch_bulk.go +++ b/exporter/elasticsearchexporter/elasticsearch_bulk.go @@ -40,7 +40,7 @@ type clientLogger struct { // LogRoundTrip should not modify the request or response, except for consuming and closing the body. // Implementations have to check for nil values in request and response. -func (cl *clientLogger) LogRoundTrip(requ *http.Request, resp *http.Response, err error, _ time.Time, dur time.Duration) error { +func (cl *clientLogger) LogRoundTrip(requ *http.Request, resp *http.Response, clientErr error, _ time.Time, dur time.Duration) error { zl := cl.Logger var fields []zap.Field @@ -56,7 +56,7 @@ func (cl *clientLogger) LogRoundTrip(requ *http.Request, resp *http.Response, er } switch { - case err == nil && resp != nil: + case clientErr == nil && resp != nil: fields = append( fields, zap.String("path", sanitize.String(requ.URL.Path)), @@ -66,10 +66,10 @@ func (cl *clientLogger) LogRoundTrip(requ *http.Request, resp *http.Response, er ) zl.Debug("Request roundtrip completed.", fields...) - case err != nil: + case clientErr != nil: fields = append( fields, - zap.NamedError("reason", err), + zap.NamedError("reason", clientErr), ) zl.Debug("Request failed.", fields...) }