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

Jump to bottom

Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_SAMPLING_ENDPOINT #282

Closed
Tracked by #124
jpkrohling opened this issue Apr 5, 2018 · 8 comments · Fixed by #511
Closed
Tracked by #124

Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_SAMPLING_ENDPOINT #282

jpkrohling opened this issue Apr 5, 2018 · 8 comments · Fixed by #511
Labels
enhancement good first issue hacktoberfest

Comments

@jpkrohling
Copy link
Contributor

jpkrohling commented Apr 5, 2018 •
edited by yurishkuro

As per #275 (comment) . This change needs to be coordinated with a change to the Java client.

Update 2020-02-07

Per jaegertracing/jaeger#1849, we want to go with the name JAEGER_SAMPLING_ENDPOINT.
@masroorhasan
Copy link

masroorhasan commented Aug 18, 2018 •
edited

Suggestion: JAEGER_CONFIG_MGR_HOST_PORT for consistent naming with all configuration environment variables. Looks like a simple change, mind if I take this one? @jpkrohling

@jpkrohling jpkrohling changed the title Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to CONFIG_MGR_HOST_PORT Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_CONFIG_MGR_HOST_PORT Aug 20, 2018
@jpkrohling
Copy link
Contributor Author

Be my guest! Just don't remove support for the old one right now. Rather, add the new env var JAEGER_CONFIG_MGR_HOST_PORT that takes precedence over JAEGER_SAMPLER_MANAGER_HOST_PORT and log a message at INFO level whenever this old env var is set, directing to the usage of the new name.

This was referenced Aug 20, 2018
Closed
Open
@masroorhasan masroorhasan mentioned this issue Aug 20, 2018
Closed
masroorhasan added a commit to masroorhasan/jaeger-client-go that referenced this issue Aug 20, 2018
@masroorhasan
Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_CONFIG_MANAGER_HOST…
550b381 
…_PORT

Fixes: jaegertracing#282
Signed-off-by: Masroor Hasan <masroor.hasan.n@gmail.com>
@masroorhasan
Copy link

masroorhasan commented Aug 20, 2018 •
edited

@jpkrohling Thanks! Put up a PR based on your feedback. Also renamed the env var to JAEGER_CONFIG_MANAGER_HOST_PORT to be more verbose.

lw346 added a commit to lw346/jaeger-client-java that referenced this issue Sep 15, 2018
@lw346
Introduce JAEGER_CONFIG_MANAGER_ENDPOINT property
e217aa6 
Introduce a new property, JAEGER_CONFIG_MANAGER_ENDPOINT, that should be
set to the URL of the Jaeger Config Manager sampling endpoint. If not
set, fall back to the now-deprecated JAEGER_SAMPLER_MANAGER_HOST_PORT as
the host and port of the URL.

If JAEGER_AGENT_HOST has been set as a property, further fall back to
that value as the host of the URL (using the default port of 5778).

This brings the SamplerConfiguration in-line with the Go client.

See also:
 * jaegertracing#521
 * jaegertracing#547
 * jaegertracing/jaeger-client-go#326
 * jaegertracing/jaeger-client-go#282

Signed-off-by: Luke Wood <luke@lwood.me.uk>
lw346 added a commit to lw346/jaeger-client-java that referenced this issue Sep 15, 2018
@lw346
Introduce JAEGER_CONFIG_MANAGER_ENDPOINT property
b70b73d 
Introduce a new property, JAEGER_CONFIG_MANAGER_ENDPOINT, that should be
set to the URL of the Jaeger Config Manager sampling endpoint. If not
set, fall back to the now-deprecated JAEGER_SAMPLER_MANAGER_HOST_PORT as
the host and port of the URL.

If JAEGER_AGENT_HOST has been set as a property, further fall back to
that value as the host of the URL (using the default port of 5778).

This brings the SamplerConfiguration in-line with the Go client.

See also:
 * jaegertracing#521
 * jaegertracing#547
 * jaegertracing/jaeger-client-go#326
 * jaegertracing/jaeger-client-go#282

Signed-off-by: Luke Wood <luke@lwood.me.uk>
lw346 added a commit to lw346/jaeger-client-java that referenced this issue Sep 15, 2018
@lw346
Introduce JAEGER_CONFIG_MANAGER_ENDPOINT property
8386985 
Introduce a new property, JAEGER_CONFIG_MANAGER_ENDPOINT, that should be
set to the URL of the Jaeger Config Manager sampling endpoint. If not
set, fall back to the now-deprecated JAEGER_SAMPLER_MANAGER_HOST_PORT as
the host and port of the URL.

If JAEGER_AGENT_HOST has been set as a property, further fall back to
that value as the host of the URL (using the default port of 5778).

This brings the SamplerConfiguration in-line with the Go client.

See also:
 * jaegertracing#521
 * jaegertracing#547
 * jaegertracing/jaeger-client-go#326
 * jaegertracing/jaeger-client-go#282

Signed-off-by: Luke Wood <luke@lwood.me.uk>
@lw346 lw346 mentioned this issue Sep 15, 2018
Closed
lw346 added a commit to lw346/jaeger-client-java that referenced this issue Sep 15, 2018
@lw346
Introduce JAEGER_CONFIG_MANAGER_ENDPOINT property
f90840e 
Introduce a new property, JAEGER_CONFIG_MANAGER_ENDPOINT, that should be
set to the URL of the Jaeger Config Manager sampling endpoint. If not
set, fall back to the now-deprecated JAEGER_SAMPLER_MANAGER_HOST_PORT as
the host and port of the URL.

If JAEGER_AGENT_HOST has been set as a property, further fall back to
that value as the host of the URL (using the default port of 5778).

This brings the SamplerConfiguration in-line with the Go client.

See also:
 * jaegertracing#521
 * jaegertracing#547
 * jaegertracing/jaeger-client-go#326
 * jaegertracing/jaeger-client-go#282

Signed-off-by: Luke Wood <luke@lwood.me.uk>
lw346 added a commit to lw346/jaeger-client-java that referenced this issue Sep 15, 2018
@lw346
Introduce JAEGER_CONFIG_MANAGER_ENDPOINT property
759ff48 
Introduce a new property, JAEGER_CONFIG_MANAGER_ENDPOINT, that should be
set to the URL of the Jaeger Config Manager sampling endpoint. If not
set, fall back to the now-deprecated JAEGER_SAMPLER_MANAGER_HOST_PORT as
the host and port of the URL.

If JAEGER_AGENT_HOST has been set as a property, further fall back to
that value as the host of the URL (using the default port of 5778).

This brings the SamplerConfiguration in-line with the Go client.

See also:
 * jaegertracing#521
 * jaegertracing#547
 * jaegertracing/jaeger-client-go#326
 * jaegertracing/jaeger-client-go#282

Signed-off-by: Luke Wood <luke@lwood.me.uk>
@VineethReddy02 VineethReddy02 mentioned this issue Oct 1, 2019
Closed
@VineethReddy02
Copy link
Contributor

@yurishkuro @jpkrohling I see this issue already has a referenced PR #325
Should I rename JAEGAR_SAMPLER_MANAGER_HOST_PORT to JAEGER_CONFIG_ENDPOINT

As yuri stated:
According to #326, the variable is actually a URL, not a host:port string. Similar to JAEGER_ENDPOINT, we can call this JAEGER_CONFIG_ENDPOINT.

@jpkrohling jpkrohling changed the title Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_CONFIG_MGR_HOST_PORT Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_CONFIG_ENDPOINT Oct 2, 2019
@jpkrohling
Copy link
Contributor Author

Yes, I believe it does make sense to use JAEGER_CONFIG_ENDPOINT instead. Just make sure that the change is backwards compatible. But before working on that, let's wait a couple of days and see if @masroorhasan still wants to work on this.

@masroorhasan, you are very close with that PR. Would you like to continue working on that?

@yurishkuro
Copy link
Member

The Java and Go issues are cross-referencing each other saying "do like it's done in {other ticket}".

We should have a language-agnostic issue in the main repo, agree on the name and the need for the change, and then follow through in different languages.

This was referenced Oct 8, 2019
Merged
Closed
@yurishkuro
Copy link
Member

Let's resolve this first: jaegertracing/jaeger#1849

@eundoosong eundoosong mentioned this issue May 8, 2020
Merged
@yurishkuro
Copy link
Member

Solved by #511.

@yurishkuro yurishkuro changed the title Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_CONFIG_ENDPOINT Rename JAEGER_SAMPLER_MANAGER_HOST_PORT to JAEGER_SAMPLING_ENDPOINT May 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement good first issue hacktoberfest
Projects
None yet
Milestone
No milestone
4 participants
@jpkrohling @masroorhasan @yurishkuro @VineethReddy02