Jaeger: es index-prefix argument should be validated

[jkandasa]

Requirement - what kind of business use case are you trying to solve?

Negative case: When adding invalid elasticsearch index-prefix collector/query service should not start. Should through validation error.

Problem - what in Jaeger blocks you from solving the requirement?

index-prefix accepting space and collector starts successfully. When collector receives trace/span throws es api error.

jaeger operator cr file:

  storage:
    type: elasticsearch
    esIndexCleaner:
      enabled: false
    dependencies:
      enabled: false
    options:
      es:
        index-prefix: "custom_prefix jk"
    elasticsearch:
      image: quay.io/openshift/origin-logging-elasticsearch5:latest
      nodeCount: 3

Collector error:

{"level":"error","ts":1563463447.2410033,"caller":"spanstore/writer.go:206","msg":"Failed to create index","trace_id":"562b81e6ce20591e","span_id":"562b81e6ce20591e","error":"elastic: Error 400 (Bad Request): Invalid index name [custom_prefix jk-jaeger-service-2019-07-18], must not contain the following characters [ , \", *, \\, <, |, ,, >, /, ?] [type=invalid_index_name_exception]","stacktrace":"github.com/jaegertracing/jaeger/plugin/storage/es/spanstore.(*SpanWriter).logError\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/es/spanstore/writer.go:206\ngithub.com/jaegertracing/jaeger/plugin/storage/es/spanstore.(*SpanWriter).createIndex\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/es/spanstore/writer.go:176\ngithub.com/jaegertracing/jaeger/plugin/storage/es/spanstore.(*SpanWriter).WriteSpan\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/es/spanstore/writer.go:135\ngithub.com/jaegertracing/jaeger/cmd/collector/app.(*spanProcessor).saveSpan\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:113\ngithub.com/jaegertracing/jaeger/cmd/collector/app.ChainedProcessSpan.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/model_consumer.go:34\ngithub.com/jaegertracing/jaeger/cmd/collector/app.(*spanProcessor).processItemFromQueue\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:139\ngithub.com/jaegertracing/jaeger/cmd/collector/app.NewSpanProcessor.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:68\ngithub.com/jaegertracing/jaeger/pkg/queue.(*BoundedQueue).StartConsumers.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/pkg/queue/bounded_queue.go:65"}
{"level":"error","ts":1563463447.2410674,"caller":"app/span_processor.go:114","msg":"Failed to save span","error":"Failed to create index: elastic: Error 400 (Bad Request): Invalid index name [custom_prefix jk-jaeger-service-2019-07-18], must not contain the following characters [ , \", *, \\, <, |, ,, >, /, ?] [type=invalid_index_name_exception]","errorVerbose":"elastic: Error 400 (Bad Request): Invalid index name [custom_prefix jk-jaeger-service-2019-07-18], must not contain the following characters [ , \", *, \\, <, |, ,, >, /, ?] [type=invalid_index_name_exception]\nFailed to create index\ngithub.com/jaegertracing/jaeger/plugin/storage/es/spanstore.(*SpanWriter).logError\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/es/spanstore/writer.go:207\ngithub.com/jaegertracing/jaeger/plugin/storage/es/spanstore.(*SpanWriter).createIndex\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/es/spanstore/writer.go:176\ngithub.com/jaegertracing/jaeger/plugin/storage/es/spanstore.(*SpanWriter).WriteSpan\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/plugin/storage/es/spanstore/writer.go:135\ngithub.com/jaegertracing/jaeger/cmd/collector/app.(*spanProcessor).saveSpan\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:113\ngithub.com/jaegertracing/jaeger/cmd/collector/app.ChainedProcessSpan.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/model_consumer.go:34\ngithub.com/jaegertracing/jaeger/cmd/collector/app.(*spanProcessor).processItemFromQueue\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:139\ngithub.com/jaegertracing/jaeger/cmd/collector/app.NewSpanProcessor.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:68\ngithub.com/jaegertracing/jaeger/pkg/queue.(*BoundedQueue).StartConsumers.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/pkg/queue/bounded_queue.go:65\nruntime.goexit\n\t/home/travis/.gimme/versions/go1.12.1.linux.amd64/src/runtime/asm_amd64.s:1337","stacktrace":"github.com/jaegertracing/jaeger/cmd/collector/app.(*spanProcessor).saveSpan\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:114\ngithub.com/jaegertracing/jaeger/cmd/collector/app.ChainedProcessSpan.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/model_consumer.go:34\ngithub.com/jaegertracing/jaeger/cmd/collector/app.(*spanProcessor).processItemFromQueue\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:139\ngithub.com/jaegertracing/jaeger/cmd/collector/app.NewSpanProcessor.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/collector/app/span_processor.go:68\ngithub.com/jaegertracing/jaeger/pkg/queue.(*BoundedQueue).StartConsumers.func1\n\t/home/travis/gopath/src/github.com/jaegertracing/jaeger/pkg/queue/bounded_queue.go:65"}

Proposal - what do you suggest to solve the problem or improve the existing situation?

run validation on input arguments

Any open questions to address

[yurishkuro]

Are you willing to put up a PR?

[bhavin192]

I would like to work on this. Can anyone help me to get started?

[yurishkuro]

@bhavin192 go ahead. Try adding invalid index-prefix to a unit test similar to this and make it fail.

https://github.com/jaegertracing/jaeger/blob/f6fc248e4c18a7c93967dcb9f4f2c8cea26ac8e2/plugin/storage/es/options_test.go#L43-L61

[pavolloffay]

Note that than we should do also validation in python scripts and spark-dependencies project.

It is more code to maintain with no real value. In the past we have decided to add a seprator into index prefix ., then we had to change it to - (because it was deprecated in ES7) we could just simply pass the prefix what user specify prefix- (mid the separator)

I wouldn't do any validation, it's more code, potentially in more components written in different languages, on top of it the index name validation is not super easy https://stackoverflow.com/questions/41585392/what-are-the-rules-for-index-names-in-elastic-search

[yurishkuro]

Fair enough, especially if the rules are not trivial. Since ES already fails with a fairly self-descriptive error in the logs, I agree that it's not a critical thing to validate.

[viveksinghggits]

hi @jkandasa
can you please share how can we know the fields/attributes that are supported for .storage.options.es. For example index-prefix add something at the start of the index that will be created by jaeger. What I want to do is to give an index so that jaeger will not not create the index and use an already created index.

[pavolloffay]

Jaeger with default configuration will always use the following indices:

• jaeger-[span|service|dependencies]-yyyy-mm-dd

only index prefix can be configured which is appended in front of the index name