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.

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 Elasticsearch version flag #1753

Merged
merged 3 commits into from
Aug 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pkg/es/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ type Client interface {
Search(indices ...string) SearchService
MultiSearch() MultiSearchService
io.Closer
GetVersion() int
GetVersion() uint
}

// IndicesExistsService is an abstraction for elastic.IndicesExistsService
Expand Down
30 changes: 20 additions & 10 deletions pkg/es/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ type Configuration struct {
TLS TLSConfig
UseReadWriteAliases bool
CreateIndexTemplates bool
Version uint
}

// TLSConfig describes the configuration properties to connect tls enabled ElasticSearch cluster
Expand All @@ -89,6 +90,7 @@ type ClientBuilder interface {
GetTokenFilePath() string
IsEnabled() bool
IsCreateIndexTemplates() bool
GetVersion() uint
}

// NewClient creates a new ElasticSearch client
Expand Down Expand Up @@ -157,18 +159,21 @@ func (c *Configuration) NewClient(logger *zap.Logger, metricsFactory metrics.Fac
return nil, err
}

// Determine ElasticSearch Version
pingResult, _, err := rawClient.Ping(c.Servers[0]).Do(context.Background())
if err != nil {
return nil, err
}
esVersion, err := strconv.Atoi(string(pingResult.Version.Number[0]))
if err != nil {
return nil, err
if c.Version == 0 {
// Determine ElasticSearch Version
pingResult, _, err := rawClient.Ping(c.Servers[0]).Do(context.Background())
if err != nil {
return nil, err
}
esVersion, err := strconv.Atoi(string(pingResult.Version.Number[0]))
if err != nil {
return nil, err
}
logger.Info("Elasticsearch detected", zap.Int("version", esVersion))
c.Version = uint(esVersion)
}
logger.Info("Elasticsearch detected", zap.Int("version", esVersion))

return eswrapper.WrapESClient(rawClient, service, esVersion), nil
return eswrapper.WrapESClient(rawClient, service, c.Version), nil
}

// ApplyDefaults copies settings from source unless its own value is non-zero.
Expand Down Expand Up @@ -243,6 +248,11 @@ func (c *Configuration) GetAllTagsAsFields() bool {
return c.AllTagsAsFields
}

// GetVersion returns Elasticsearch version
func (c *Configuration) GetVersion() uint {
return c.Version
}

// GetTagDotReplacement returns character is used to replace dots in tag keys, when
// the tag is stored as object field.
func (c *Configuration) GetTagDotReplacement() string {
Expand Down
15 changes: 9 additions & 6 deletions pkg/es/mocks/Client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions pkg/es/mocks/IndexService.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions pkg/es/mocks/IndicesCreateService.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions pkg/es/mocks/IndicesExistsService.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions pkg/es/mocks/MultiSearchService.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions pkg/es/mocks/SearchService.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 8 additions & 4 deletions pkg/es/mocks/TemplateCreateService.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions pkg/es/wrapper/wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ import (
type ClientWrapper struct {
client *elastic.Client
bulkService *elastic.BulkProcessor
esVersion int
esVersion uint
}

// GetVersion returns the ElasticSearch Version
func (c ClientWrapper) GetVersion() int {
func (c ClientWrapper) GetVersion() uint {
return c.esVersion
}

// WrapESClient creates a ESClient out of *elastic.Client.
func WrapESClient(client *elastic.Client, s *elastic.BulkProcessor, esVersion int) ClientWrapper {
func WrapESClient(client *elastic.Client, s *elastic.BulkProcessor, esVersion uint) ClientWrapper {
return ClientWrapper{client: client, bulkService: s, esVersion: esVersion}
}

Expand Down Expand Up @@ -152,11 +152,11 @@ func (c TemplateCreateServiceWrapper) Do(ctx context.Context) (*elastic.IndicesP
type IndexServiceWrapper struct {
bulkIndexReq *elastic.BulkIndexRequest
bulkService *elastic.BulkProcessor
esVersion int
esVersion uint
}

// WrapESIndexService creates an ESIndexService out of *elastic.ESIndexService.
func WrapESIndexService(indexService *elastic.BulkIndexRequest, bulkService *elastic.BulkProcessor, esVersion int) IndexServiceWrapper {
func WrapESIndexService(indexService *elastic.BulkIndexRequest, bulkService *elastic.BulkProcessor, esVersion uint) IndexServiceWrapper {
return IndexServiceWrapper{bulkIndexReq: indexService, bulkService: bulkService, esVersion: esVersion}
}

Expand Down
2 changes: 1 addition & 1 deletion plugin/storage/es/dependencystore/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const dependenciesMapping7 = `{
"mappings":{}
}`

func getMapping(version int) string {
func getMapping(version uint) string {
if version == 7 {
return dependenciesMapping7
}
Expand Down
2 changes: 1 addition & 1 deletion plugin/storage/es/dependencystore/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func TestWriteDependencies(t *testing.T) {
createIndexError error
writeError error
expectedError string
esVersion int
esVersion uint
}{
{
createIndexError: errors.New("index not created"),
Expand Down
2 changes: 1 addition & 1 deletion plugin/storage/es/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ func createSpanWriter(
}

// GetMappings returns span and service mappings
func GetMappings(shards, replicas int64, esVersion int) (string, string) {
func GetMappings(shards, replicas int64, esVersion uint) (string, string) {
if esVersion == 7 {
return fixMapping(loadMapping("/jaeger-span-7.json"), shards, replicas),
fixMapping(loadMapping("/jaeger-service-7.json"), shards, replicas)
Expand Down
2 changes: 1 addition & 1 deletion plugin/storage/es/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (m *mockClientBuilder) NewClient(logger *zap.Logger, metricsFactory metrics
tService.On("Body", mock.Anything).Return(tService)
tService.On("Do", context.Background()).Return(nil, m.createTemplateError)
c.On("CreateTemplate", mock.Anything).Return(tService)
c.On("GetVersion").Return(6)
c.On("GetVersion").Return(uint(6))
return c, nil
}
return nil, m.err
Expand Down
7 changes: 7 additions & 0 deletions plugin/storage/es/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ const (
suffixReadAlias = ".use-aliases"
suffixCreateIndexTemplate = ".create-index-templates"
suffixEnabled = ".enabled"
suffixVersion = ".version"
)

// TODO this should be moved next to config.Configuration struct (maybe ./flags package)
Expand Down Expand Up @@ -96,6 +97,7 @@ func NewOptions(primaryNamespace string, otherNamespaces ...string) *Options {
TagDotReplacement: "@",
Enabled: true,
CreateIndexTemplates: true,
Version: 0,
},
servers: "http://127.0.0.1:9200",
namespace: primaryNamespace,
Expand Down Expand Up @@ -221,6 +223,10 @@ func addFlags(flagSet *flag.FlagSet, nsConfig *namespaceConfig) {
nsConfig.namespace+suffixCreateIndexTemplate,
nsConfig.CreateIndexTemplates,
"Create index templates at application startup. Set to false when templates are installed manually.")
flagSet.Uint(
nsConfig.namespace+suffixVersion,
0,
"The major Elasticsearch version. If not specified, the value will be auto-detected from Elasticsearch.")
if nsConfig.namespace == archiveNamespace {
flagSet.Bool(
nsConfig.namespace+suffixEnabled,
Expand Down Expand Up @@ -264,6 +270,7 @@ func initFromViper(cfg *namespaceConfig, v *viper.Viper) {
cfg.UseReadWriteAliases = v.GetBool(cfg.namespace + suffixReadAlias)
cfg.Enabled = v.GetBool(cfg.namespace + suffixEnabled)
cfg.CreateIndexTemplates = v.GetBool(cfg.namespace + suffixCreateIndexTemplate)
cfg.Version = uint(v.GetInt(cfg.namespace + suffixVersion))
// TODO: Need to figure out a better way for do this.
cfg.AllowTokenFromContext = v.GetBool(spanstore.StoragePropagationKey)
}
Expand Down
4 changes: 2 additions & 2 deletions plugin/storage/integration/elasticsearch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type ESStorageIntegration struct {
logger *zap.Logger
}

func (s *ESStorageIntegration) getVersion() (int, error) {
func (s *ESStorageIntegration) getVersion() (uint, error) {
pingResult, _, err := s.client.Ping(queryURL).Do(context.Background())
if err != nil {
return 0, err
Expand All @@ -65,7 +65,7 @@ func (s *ESStorageIntegration) getVersion() (int, error) {
if err != nil {
return 0, err
}
return esVersion, nil
return uint(esVersion), nil
}

func (s *ESStorageIntegration) initializeES(allTagsAsFields, archive bool) error {
Expand Down
28 changes: 0 additions & 28 deletions plugin/storage/integration/token_propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,8 @@ import (
"context"
"encoding/json"
"net/http"
"os"
"os/exec"
"strings"
"testing"
"time"

"github.com/olivere/elastic"
"github.com/stretchr/testify/assert"
Expand All @@ -40,16 +37,6 @@ type esTokenPropagationTestHandler struct {
}

func (h *esTokenPropagationTestHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// Return the elasticsearch version
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this removed because it was a temporary workaround when the version wasn't available?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes

if r.URL.Path == "/" {
ret := new(elastic.PingResult)
ret.Version.Number = "7.3.0"
json_ret, _ := json.Marshal(ret)
w.Header().Add("Content-Type", "application/json; charset=UTF-8")
w.Write(json_ret)
return
}

authValue := r.Header.Get("Authorization")
if authValue != "" {
headerValue := strings.Split(authValue, " ")
Expand Down Expand Up @@ -102,19 +89,6 @@ func TestBearTokenPropagation(t *testing.T) {
defer srv.Shutdown(context.Background())

go createElasticSearchMock(srv, t)
// Wait for http server to start
time.Sleep(100 * time.Millisecond)

// Path relative to plugin/storage/integration/token_propagation_test.go
cmd := exec.Command("../../../cmd/query/query-linux", "--es.server-urls=http://127.0.0.1:9200", "--es.tls=false", "--query.bearer-token-propagation=true")
cmd.Env = []string{"SPAN_STORAGE_TYPE=elasticsearch"}
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
err := cmd.Start()
assert.NoError(t, err)

// Wait for query service to start
time.Sleep(100 * time.Millisecond)

// Test cases.
for _, testCase := range testCases {
Expand All @@ -130,6 +104,4 @@ func TestBearTokenPropagation(t *testing.T) {
assert.Equal(t, resp.StatusCode, http.StatusOK)
}
}

cmd.Process.Kill()
}
4 changes: 4 additions & 0 deletions scripts/travis/es-integration-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,8 @@ echo "Executing token propatagion test"
# Mock UI, needed only for build query service.
make build-crossdock-ui-placeholder
GOOS=linux make build-query

SPAN_STORAGE_TYPE=elasticsearch ./cmd/query/query-linux --es.server-urls=http://127.0.0.1:9200 --es.tls=false --es.version=7 --query.bearer-token-propagation=true &
PID=$(echo $!)
make token-propagation-integration-test
kill -9 ${PID}