Install Jaeger by default in all Sourcegraph deployment options

[beyang]

Top-line goal: Boost use of Jaeger at customers to close to 100%.
Sub-goals:

• Update installation instructions to include Jaeger for all 3 deployment methods:
  • k8s Sourcegraph (Jaeger without the Jaeger Operator deploy-sourcegraph#559)
  • Single Docker image
  • 🏃‍♂️ Docker Compose
• Turn Jaeger on/off using config watch (no restart required)
• Site config field that enables selective sampling triggered by URL parameter: jaeger.sampling: {selective,comprehensive} (necessary to address https://github.com/sourcegraph/sourcegraph/issues/5363)
• Deprecate useJaeger site config option
• Jaeger collection should be toggleable without needing to restart sourcegraph-frontend service
• Add docs for changing Jaeger sampling strategy (https://www.jaegertracing.io/docs/1.17/sampling/)
• Migration docs for existing customers (need to cover installation instructions and possible site config migration)
• Make it easier to report slow queries by linking to Jaeger info. E.g., slow searches should prompt to report the issue with a link to the Jaeger span.

Follow-up tasks

• From @keegan: I think using @uwedeportivo's work around admin proxying to include Jaeger would be very useful here. Then updating x-trace header to link via the admin proxy. Moved to: https://github.com/sourcegraph/sourcegraph/issues/10048

[keegancsmith]

Are we at all concerned about increased memory/cpu use?

Add docs for changing Jaeger sampling strategy (jaegertracing.io/docs/1.17/sampling)

What should the default be? Should we be conservative with resources, or conservative w.r.t. debuggability.

Turn Jaeger on/off using config watch (no restart required)

have you tested this with race enabled? I know when I lasted looked at the opentracing library it had no sync around setting and getting of the global tracer.

Make it easier to report slow queries by linking to Jaeger info. E.g., slow searches should prompt to report the issue with a link to the Jaeger span.

I think using @uwedeportivo's work around admin proxying to include Jaeger would be very useful here. Then updating x-trace header to link via the admin proxy.

[keegancsmith]

An idea for the default trace policy: "admin only". If the request is from an admin collect the trace.

[beyang]

Are we at all concerned about increased memory/cpu use?
What should the default be? Should we be conservative with resources, or conservative w.r.t. debuggability.

By default, the resource consumption overhead should be on par with Prometheus/Grafana, because we'll only trace selectively (i.e., if ?trace=1 is in the parent request URL). I also like the admin-only idea, so am thinking of offering 4 levels of tracing: none, selective, admin-only, comprehensive. selective means "only trace if ?trace=1 is in the URL".

have you tested this with race enabled? I know when I lasted looked at the opentracing library it had no sync around setting and getting of the global tracer.

Nope. I think I added a mutex protecting the conf.Watch, but will double check.

I think using @uwedeportivo's work around admin proxying to include Jaeger would be very useful here. Then updating x-trace header to link via the admin proxy.

Yep, this would be a great follow-up item. Added to issue description.

[emidoots]

Cassandra (the jaeger DB backend) uses a TON of memory, be sure to check on that

On Mon, Mar 30, 2020 at 10:29 AM Beyang Liu ***@***.***> wrote: Are we at all concerned about increased memory/cpu use? What should the default be? Should we be conservative with resources, or conservative w.r.t. debuggability. By default, the resource consumption overhead should be on par with Prometheus/Grafana, because we'll only trace selectively (i.e., if ?trace=1 is in the parent request URL). I also like the admin-only idea, so am thinking of offering 4 levels of tracing: none, selective, admin-only, comprehensive. selective means "only trace if ?trace=1 is in the URL". have you tested this with race enabled? I know when I lasted looked at the opentracing library it had no sync around setting and getting of the global tracer. Nope. I *think* I added a mutex protecting the conf.Watch, but will double check. I think using @uwedeportivo <https://github.com/uwedeportivo>'s work around admin proxying to include Jaeger would be very useful here. Then updating x-trace header to link via the admin proxy. Yep, this would be a great follow-up item. Added to issue description. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/sourcegraph/sourcegraph/issues/9300#issuecomment-606135626>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYGWOCU6P5JYCHDGJZ365TRKDJHXANCNFSM4LTFPPOA> .

-- Follow me on twitter @slimsag <https://twitter.com/slimsag>.

[keegancsmith]

Cassandra (the jaeger DB backend) uses a TON of memory, be sure to check on
that

I think the default environment will be using jaeger-all-in-one, which uses an in-memory collector (even worse memory, but I suppose operationally much simpler). I think having a default conservative collection policy will obviate that concern (eg opt-in tracing + admin as suggested above).

[emidoots]

Yeah that sounds good, Cassandra would be worse because it allocates the entire heap up-front (e.g. 8GB of memory) by default

On Mon, Mar 30, 2020 at 1:09 PM Keegan Carruthers-Smith < ***@***.***> wrote: Cassandra (the jaeger DB backend) uses a TON of memory, be sure to check on that I think the default environment will be using jaeger-all-in-one, which uses an in-memory collector (even worse memory, but I suppose operationally much simpler). I think having a default conservative collection policy will obviate that concern (eg opt-in tracing + admin as suggested above). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/sourcegraph/sourcegraph/issues/9300#issuecomment-606218895>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYGWOBIGHCJ6YZ62FXIUXTRKD36BANCNFSM4LTFPPOA> .

-- Follow me on twitter @slimsag <https://twitter.com/slimsag>.

[beyang]

Operationally, the Elasticsearch backend is easier to deploy than the Cassandra one (which requires an extra initialization step to create the CSQL schema, thereby complicating installation of Jaeger and therefore installation of Sourcegraph, too). So I'm going to go with that one (sourcegraph/deploy-sourcegraph#559). Alternatively, we could have people install the all-in-one (i.e., in-memory) Jaeger by default, but in our own clusters, that one seems to OOM a lot. Unless there are objections, I will go with the Elasticsearch backend in all three Sourcegraph deployment models (single Docker, Docker Compose, and Kubernetes).

[emidoots]

SGTM, I’ll be curious to hear what the average memory usage of that is

On Mon, Mar 30, 2020 at 9:28 PM Beyang Liu ***@***.***> wrote: Operationally, the Elasticsearch backend is easier to deploy than the Cassandra one (which requires an extra initialization step to create the CSQL schema, thereby complicating installation of Jaeger and also installation of Sourcegraph). So I'm going to go with that one. Alternatively, we could have people install the all-in-one (i.e., in-memory) Jaeger by default, but in our own clusters, that one seems to OOM a lot. Unless there are objections, I will go with the Elasticsearch backend in all three Sourcegraph deployment models (single Docker, Docker Compose, and Kubernetes). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/sourcegraph/sourcegraph/issues/9300#issuecomment-606392070>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAYGWOCQC7RTNPUB4VHQHZLRKFWP3ANCNFSM4LTFPPOA> .

-- Follow me on twitter @slimsag <https://twitter.com/slimsag>.

[keegancsmith]

Isn't elasticsearch also JVM based? This is also a non-insignificant cost w.r.t. image size and memory.

Can't we have a default collection policy that is very conservative (ie opt-in per request), then the in memory collector won't oom. If someone wants to adjust the policy they need to deploy a proper jaeger installation.

I'm concerned that the resource use of sourcegraph will start having a non-insignificant portion allocated to observability. Especially since it will be observing a bunch of stuff no one will ever look at.