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

Strip whitespace from value for servers in configmap #1300

Closed
joshushua opened this issue Jan 25, 2019 · 12 comments
Closed

Strip whitespace from value for servers in configmap #1300

joshushua opened this issue Jan 25, 2019 · 12 comments
Labels

Comments

@joshushua
Copy link

joshushua commented Jan 25, 2019

Requirement

Listing the IP addresses of each node of a Cassandra cluster.

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

In the configmap.yml file, the key-value pair of servers: XX.XX.XXX.XX for one IP entry works fine. But when providing more than one entry, like servers: XX.XX.XXX.XX, XX.XXX.XX.XXX, XX.XXX.XXX.XX, the Collector's gocql reveals an error for the second IP: dns error: lookup XX.XXX.XX.XXX: no such host and reveals the same error for the third IP.

When I remove the whitespace between the comma and the next IP, the thing works fine and the Collector successfully connects to the Cassandra cluster.

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

Strip white spaces from the value, so that humans can still use whitespaces when writing and reading these values.

Any open questions to address

Will you please include this type of Cassandra example in your documentation? Something beyond the all-in-one example which is too simple for production scenarios. I couldn't find anything about clusters and raw IP addresses for the cluster.

@jpkrohling
Copy link
Contributor

For those looking to send their first contribution to the project, this is probably the right place to apply the fix:

https://github.com/jaegertracing/jaeger/blob/master/plugin/storage/cassandra/options.go

Make sure to add a new test to https://github.com/jaegertracing/jaeger/blob/master/plugin/storage/cassandra/options_test.go

@jpkrohling
Copy link
Contributor

And don't forget to read the CONTRIBUTING.md!

@karlpokus
Copy link
Contributor

I'll have a PR ready shortly.

@black-adder
Copy link
Contributor

reopening to address the documentation ask

@Abhi-khandelwal
Copy link

Hi @black-adder, I'm new to this community, can I work on this issue ?

@yurishkuro
Copy link
Member

yurishkuro commented Aug 8, 2019

@Abhi-khandelwal by all means, but note that at this point it's a documentation task, not coding.

@Abhi-khandelwal
Copy link

Ohk @yurishkuro , I'll try to fix some other beginner coding issues. :)

@aloeinc
Copy link

aloeinc commented Nov 1, 2019

@jpkrohling I'm coming from Outreachy, am interested in data tracing research, and would enjoy solving this documentation issue. Would you recommend any other tasks I could complete in order to exemplify my skills?

@yurishkuro yurishkuro removed the bug label Nov 3, 2019
@jpkrohling
Copy link
Contributor

@aloeankh sorry, I just saw your message. The application phase for Outreachy is over (or almost), but if you'd still like to contribute, the right place for this would probably be in the Cassandra-specific documentation, here:

https://www.jaegertracing.io/docs/1.14/deployment/#cassandra
(source: https://github.com/jaegertracing/documentation/blob/master/content/docs/next-release/deployment.md#cassandra )

@Git-Jiro
Copy link
Contributor

Git-Jiro commented Oct 8, 2021

Hi @jpkrohling,
I would like to do that docs update.

One question though:
The link https://github.com/jaegertracing/documentation/blob/master/content/docs/next-release/deployment.md#cassandra points to the doc source. The examples start a docker container and provide some env vars to the container.
Is the assumption correct that the CASSANDRA_SERVERS env variable somehow ends up as the parameter --cas.servers for the jaeger binary?

If yes, then I kinda now what needs to be added to the docs.

@jpkrohling
Copy link
Contributor

Every parameter can be specified both as CLI parameter and as env var (except the span storage type one). We list only env vars in the docs page, though.

@Git-Jiro
Copy link
Contributor

@jpkrohling anything else that needs to be documented before this issue can be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants