Init Elasticsearch exporter #2324
Conversation
This is the first step in adding the Elasticsearch exporter. Initially we will only support the Logs exporter interface, and potentially will add metrics in the future as well. This change only provides some boilerplate initializing the exporter. But the exporter is not yet usable (or part of) any opentelemetry collector distribution. The elasticsearch exporter is based on the official [go-elasticsearch](https://github.com/elastic/go-elasticsearch) client. We will use the BulkIndexer provided by the client for event publishing. The client and BulkIndexer provide some support for retrying already. The Elasticsearch Bulk API can report errors at the HTTP level, but uses selective ACKs for individual events. This allows us to retry only failed events and/or reject events that can not be indexed (e.g. due to an mapping error). The 429 error code might even inidcate that we should backoff a little before retrying. **Link to tracking Issue:** open-telemetry#1800 **Testing:** Only configuration loading and validation tests have been added so far. The exporter currently panics when trying to publish events. More unit and integration tests will be added in the future. **Documentation:** All settings that will be available initially are documented in the README.md file.
|
|
Codecov Report
@@ Coverage Diff @@
## main #2324 +/- ##
==========================================
- Coverage 90.63% 90.61% -0.02%
==========================================
Files 400 400
Lines 19895 19955 +60
==========================================
+ Hits 18031 18082 +51
- Misses 1411 1418 +7
- Partials 453 455 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
| @@ -77,6 +77,6 @@ nodes will automatically be used for load balancing. | |||
| ```yaml | |||
| exporters: | |||
| elasticsearch: | |||
| urls: | |||
| endpoints: | |||
| - "https://localhost:9200" | |||
There was a problem hiding this comment.
How does a deployment typically look like? Is it common to have TLS for one server, but not for another? Do they have different sets of certs, or would a client only need one CA that works for all clients? I'm asking because endpoints are typically only host:port, without the protocol part, using TLS by default.
There was a problem hiding this comment.
Is it common to have TLS for one server, but not for another?
No, when using TLS one would want to have TLS for the complete cluster.
Do they have different sets of certs, or would a client only need one CA that works for all clients?
Ideally one CA cert.
I'm asking because endpoints are typically only host:port, without the protocol part, using TLS by default.
The exporter also accepts localhost:9200, but I'm not sure if TLS will be automatically enabled or disabled by default. I assume the client library will expand this to http://localhost:9200 if tls is not explicitely configured.
I took otlphttpexporter as example, which also uses a full URL in its example:
exporters:
otlphttp:
endpoint: https://example.com:55681/v1/traces
same for zipkin.
There was a problem hiding this comment.
Interesting, the one from OTLP should have been named URL then, instead of Endpoint. Looks good to me then!
| // Workers configures the number of workers publishing bulk requests. | ||
| Workers int `mapstructure:"workers"` |
There was a problem hiding this comment.
Consider "num_workers" to be consistent with NumConsumers in our sending_queue helper.
| // Max configures how often an HTTP request is retried before it is assumed to be failed. | ||
| Max int `mapstructure:"max"` |
There was a problem hiding this comment.
MaxRequests? If I were to add this to https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/exporterhelper/queued_retry.go#L58 that would probably be a more consistent name.
There was a problem hiding this comment.
+1
Updated field name and configuration name to max_requests.
| @@ -10,7 +10,7 @@ This exporter supports sending OpenTelemetry logs to [Elasticsearch](https://www | |||
| [ID](https://www.elastic.co/guide/en/cloud/current/ec-cloud-id.html) of the | |||
| Elastic Cloud Cluster to publish events to. The `cloudid` can be used instead | |||
| of `endpoints`. | |||
| - `workers` (optional): Number of workers publishing bulk requests concurrently. | |||
| - `nuym_workers` (optional): Number of workers publishing bulk requests concurrently. | |||
There was a problem hiding this comment.
| - `nuym_workers` (optional): Number of workers publishing bulk requests concurrently. | |
| - `num_workers` (optional): Number of workers publishing bulk requests concurrently. |
* fixing flaky deb package job * using waitgroup instead of sleep * moving wait in the same line as docker cp Co-authored-by: Azfaar Qureshi <[email protected]>
* Init Elasticsearch exporter This is the first step in adding the Elasticsearch exporter. Initially we will only support the Logs exporter interface, and potentially will add metrics in the future as well. This change only provides some boilerplate initializing the exporter. But the exporter is not yet usable (or part of) any opentelemetry collector distribution. The elasticsearch exporter is based on the official [go-elasticsearch](https://github.com/elastic/go-elasticsearch) client. We will use the BulkIndexer provided by the client for event publishing. The client and BulkIndexer provide some support for retrying already. The Elasticsearch Bulk API can report errors at the HTTP level, but uses selective ACKs for individual events. This allows us to retry only failed events and/or reject events that can not be indexed (e.g. due to an mapping error). The 429 error code might even inidcate that we should backoff a little before retrying. **Link to tracking Issue:** #1800 **Testing:** Only configuration loading and validation tests have been added so far. The exporter currently panics when trying to publish events. More unit and integration tests will be added in the future. **Documentation:** All settings that will be available initially are documented in the README.md file. * Rename urls setting to endpoints * fix lint * Add factory and exporter initialization tests * Lint checks * Error lint * Fix import order * fix typo * Do not "shadow" err in a deferred func * review * fix typo in exporter_test.go * use const for environmant variable name in tests * fix format after gorename * typo
**Description:** Remove exporter/elasticsearch config fields & mentions in the README. Nothing has ever used this config. They were added in #2324, but were not used. If one _were_ to add fields to all documents, then I think that should be done with a processor. **Link to tracking Issue:** N/A **Testing:** N/A **Documentation:** N/A
This is the first step in adding the Elasticsearch exporter. Initially we will only support the Logs exporter interface, and potentially will add metrics in the future as well.
This change only provides some boilerplate initializing the exporter. But the exporter is not yet usable (or part of) any opentelemetry collector distribution.
The elasticsearch exporter is based on the official go-elasticsearch client. We will use the BulkIndexer provided by the client for event publishing. The client and BulkIndexer provide some support for retrying already. The Elasticsearch Bulk API can report errors at the HTTP level, but uses selective ACKs for individual events. This allows us to retry only failed events and/or reject events that can not be indexed (e.g. due to an mapping error). The 429 error code might even inidcate that we should backoff a little before retrying.
Link to tracking Issue: #1800
Testing: Only configuration loading and validation tests have been added so far. The exporter currently panics when trying to publish events. More unit and integration tests will be added in the future.
Documentation: All settings that will be available initially are documented in the README.md file.