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

Trace the dedicated query server #454

Merged
merged 1 commit into from
Oct 6, 2017

Conversation

rbtcollins
Copy link
Contributor

The standalone query server already has tracing enabled. This is a
trivial parity patch.

The standalone query server already has tracing enabled. This is a
trivial parity patch.

Signed-off-by: Robert Collins <[email protected]>
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e0b3828 on rbtcollins:master into 5030513 on jaegertracing:master.

@@ -65,6 +66,18 @@ func main() {

metricsFactory := xkit.Wrap("jaeger-query", expvar.NewFactory(10))

tracer, closer, err := jaegerClientConfig.Configuration{
Sampler: &jaegerClientConfig.SamplerConfig{
Type: "probabilistic",
Copy link
Member

Choose a reason for hiding this comment

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

use const instead of probabilistic, it's cheaper.

Copy link
Member

Choose a reason for hiding this comment

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

actually, I think this needs to be overridable vie cmd line switches

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be via jaegertracing/jaeger-client-go#206 no? I think we should have one set of variables for all the Jaeger bindings, so that its really easy to configure jaeger enabled components in one consistent fashion. That hasn't been done yet though.

I can switch this to const easily - I think its worth noting though that this is the code thats in the all-in-one entry point. Should I change that at the same time?

Given that this is a very simple and relatively low-use UI: scales O(developers) not O(users), I think a const of 1 is fine to start with.

@yurishkuro yurishkuro merged commit d17fad0 into jaegertracing:master Oct 6, 2017
ideepika pushed a commit to ideepika/jaeger that referenced this pull request Oct 22, 2017
The standalone query server already has tracing enabled. This is a
trivial parity patch.

Signed-off-by: Robert Collins <[email protected]>
isaachier pushed a commit to isaachier/jaeger that referenced this pull request Nov 1, 2017
The standalone query server already has tracing enabled. This is a
trivial parity patch.

Signed-off-by: Robert Collins <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants