Skip to content
This repository has been archived by the owner on Aug 30, 2022. It is now read-only.

Support Tracer tags and configuration via environment variables #181

Merged
merged 3 commits into from
Oct 8, 2019

Conversation

mdouaihy
Copy link
Contributor

@mdouaihy mdouaihy commented Oct 6, 2019

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy [email protected]

Which problem is this PR solving?

Short description of the changes

  • Add fromEnv method to the Tracer Config class. The method will overwrite the configuration if the corresponding environment variable is set.

Supported properties

  • Tracer
    • JAEGER_SERVICE_NAME
    • JAEGER_TAGS
    • JAEGER_DISABLED
  • Reporter
    • JAEGER_AGENT_HOST
    • JAEGER_AGENT_PORT
    • JAEGER_ENDPOINT
    • JAEGER_REPORTER_LOG_SPANS
    • JAEGER_REPORTER_FLUSH_INTERVAL
    • JAEGER_REPORTER_MAX_QUEUE_SIZE
  • Sampler
    • JAEGER_SAMPLER_TYPE
    • JAEGER_SAMPLER_PARAM
    • JAEGER_SAMPLER_MANAGER_HOST_PORT

@AppVeyorBot
Copy link

@yurishkuro yurishkuro changed the title Support configuring client from environment variables (fix #178) Support Tracer tags and configuration via environment variables Oct 6, 2019
@mdouaihy
Copy link
Contributor Author

mdouaihy commented Oct 8, 2019

Hi @yurishkuro, any comment on this PR?

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

sorry, I never submitted the review

src/jaegertracing/Tracer.h Outdated Show resolved Hide resolved
src/jaegertracing/ConfigTest.cpp Outdated Show resolved Hide resolved
src/jaegertracing/ConfigTest.cpp Show resolved Hide resolved
src/jaegertracing/utils/EnvVariable.cpp Show resolved Hide resolved
1. When testing the config, start with a defined config instead of an empty one
2. Make sure fromEnv is harmful in case nothing is defined
3. Use literals to avoid fat-finger problem in the code
4. Avoid adding yet additional methods to the Tracer

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <[email protected]>
@AppVeyorBot
Copy link

Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

lgtml, minor nits

src/jaegertracing/ConfigTest.cpp Outdated Show resolved Hide resolved
src/jaegertracing/ConfigTest.cpp Outdated Show resolved Hide resolved
src/jaegertracing/samplers/Config.cpp Outdated Show resolved Hide resolved
Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <[email protected]>
Copy link
Member

@yurishkuro yurishkuro left a comment

Choose a reason for hiding this comment

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

Thanks!

@AppVeyorBot
Copy link

@yurishkuro yurishkuro merged commit 5fc9c65 into jaegertracing:master Oct 8, 2019
@mdouaihy mdouaihy deleted the configfromenv branch October 8, 2019 22:47
@mdouaihy
Copy link
Contributor Author

mdouaihy commented Oct 8, 2019

thank you @yurishkuro !

Patrick0308 pushed a commit to Patrick0308/jaeger-client-cpp that referenced this pull request Jul 21, 2021
…ertracing#181)

* Support configuring client from environment variables (fix jaegertracing#178)

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <[email protected]>

* Apply Changes requested by @Yurishkhuro
1. When testing the config, start with a defined config instead of an empty one
2. Make sure fromEnv is harmful in case nothing is defined
3. Use literals to avoid fat-finger problem in the code
4. Avoid adding yet additional methods to the Tracer

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <[email protected]>

* Exclude sampler config.

Signed-off-by: FR-MUREX-COM\mchaikhadouaihy <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support configuring client from environment variables
3 participants