Skip to content
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.

Sign up for GitHub

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

Init Elasticsearch exporter #2324

Merged
merged 14 commits into from
Feb 18, 2021
Merged

Init Elasticsearch exporter #2324

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
66f7074
Init Elasticsearch exporter
Feb 10, 2021
04f1394
Rename urls setting to endpoints
Feb 12, 2021
98258d5
fix lint
Feb 15, 2021
cd186b1
Add factory and exporter initialization tests
Feb 15, 2021
9044bc2
Lint checks
Feb 15, 2021
bf95ba3
Error lint
Feb 16, 2021
5ce6181
Fix import order
Feb 16, 2021
5337d63
fix typo
Feb 16, 2021
63575c0
Do not "shadow" err in a deferred func
Feb 16, 2021
758b2c2
review
Feb 17, 2021
13243f6
fix typo in exporter_test.go
Feb 17, 2021
b4d147b
use const for environmant variable name in tests
Feb 17, 2021
617048e
fix format after gorename
Feb 17, 2021
9255024
typo
Feb 18, 2021
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
6 changes: 3 additions & 3 deletions exporter/elasticsearchexporter/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ This exporter supports sending OpenTelemetry logs to [Elasticsearch](https://www

## Configuration options

- `urls`: List of Elasticsearch URLS. If urls and cloudid is missing the
- `endpoints`: List of Elasticsearch URLs. If endpoints and cloudid is missing, the
ELASTICSEARCH_URL environment variable will be used.
- `cloudid` (optional):
[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 `urls`.
of `endpoints`.
- `workers` (optional): Number of workers publishing bulk requests concurrently.
- `index`: The
[index](https://www.elastic.co/guide/en/elasticsearch/reference/current/indices.html)
Expand Down Expand Up @@ -77,6 +77,6 @@ nodes will automatically be used for load balancing.
```yaml
exporters:
elasticsearch:
urls:
endpoints:
- "https://localhost:9200"
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

@bogdandrutu ^

Interesting, the one from OTLP should have been named URL then, instead of Endpoint. Looks good to me then!

```
19 changes: 12 additions & 7 deletions exporter/elasticsearchexporter/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package elasticsearchexporter
import (
"errors"
"fmt"
"os"
"strings"
"time"

Expand All @@ -28,11 +29,11 @@ import (
type Config struct {
configmodels.ExporterSettings `mapstructure:",squash"`

// URLs holds the Elasticsearch URLs the exporter should send events to.
// Endpoints holds the Elasticsearch URLs the exporter should send events to.
//
// This setting is required if CloudID is not set and if the
// ELASTICSEARCH_URL environment variable is not set.
URLs []string `mapstructure:"urls"`
Endpoints []string `mapstructure:"endpoints"`

// CloudID holds the cloud ID to identify the Elastic Cloud cluster to send events to.
// https://www.elastic.co/guide/en/cloud/current/ec-cloud-id.html
Expand Down Expand Up @@ -193,15 +194,19 @@ var mappingModes = func() map[string]MappingMode {
return table
}()

const defaultElasticsearchEnvName = "ELASTICSEARCH_URL"

// Validate validates the elasticsearch server configuration.
func (cfg *Config) Validate() error {
if len(cfg.URLs) == 0 && cfg.CloudID == "" {
return errors.New("Elasticsearch URL or CloudID must be specified")
if len(cfg.Endpoints) == 0 && cfg.CloudID == "" {
if os.Getenv(defaultElasticsearchEnvName) == "" {
return errors.New("Elasticsearch endpoints or cloudid must be specified")
}
}

for _, url := range cfg.URLs {
if url == "" {
return errors.New("Elasticsearch URL must not be empty")
for _, endpoint := range cfg.Endpoints {
if endpoint == "" {
return errors.New("endpoints must not include empty entries")
}
}

Expand Down
2 changes: 1 addition & 1 deletion exporter/elasticsearchexporter/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestLoadConfig(t *testing.T) {
r1 := cfg.Exporters["elasticsearch/customname"].(*Config)
assert.Equal(t, r1, &Config{
ExporterSettings: configmodels.ExporterSettings{TypeVal: configmodels.Type(typeStr), NameVal: "elasticsearch/customname"},
URLs: []string{"https://elastic.example.com:9200"},
Endpoints: []string{"https://elastic.example.com:9200"},
CloudID: "TRNMxjXlNJEt",
Index: "myindex",
Pipeline: "mypipeline",
Expand Down
2 changes: 1 addition & 1 deletion exporter/elasticsearchexporter/exporter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func newElasticsearchClient(logger *zap.Logger, config *Config) (*elasticsearch7
Transport: transport,

// configure connection setup
Addresses: config.URLs,
Addresses: config.Endpoints,
CloudID: config.CloudID,
Username: config.Authentication.User,
Password: config.Authentication.Password,
Expand Down
2 changes: 1 addition & 1 deletion exporter/elasticsearchexporter/testdata/config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ processors:
exporters:
elasticsearch:
elasticsearch/customname:
urls: [https://elastic.example.com:9200]
endpoints: [https://elastic.example.com:9200]
cloudid: TRNMxjXlNJEt
timeout: 2m
headers:
Expand Down