Configuration: Update tracing section to clarify GF_TRACING prefix applies to all Jaeger variables

[conorevans]

Signed-off-by: Conor Evans coevans@tcd.ie

What this PR does / why we need it: Since these variables do not feature in the [tracing.jaeger] section and they are explicitly called out as JAEGER_..., it was definitely not clear to me that the GF_<section> prefix applied here. I spent some time trying to debug, seeing if there was a problem with the agent, before I realized that I wasn't seeing anything tracing related in the logs on startup. I took a hunch on this and my hunch was confirmed:

t=2021-04-03T18:00:00+0000 lvl=info msg="Config overridden from Environment variable" logger=settings var="GF_TRACING_JAEGER_SAMPLER_TYPE=const"

[conorevans]

I suppose this and sampler_param do follow the convention already given that it is under [tracing.jaeger.sampler_type] but it's called out as JAEGER_SAMPLER_TYPE here (when it doesn't need to be, given that it follows the GF_<section>_<parameter> convention). So I think the comment could either be updated as here, or removed entirely, but as JAEGER_SAMPLER_TYPE I do think it's somewhat confusing.

[conorevans]

I have actually submitted an issue relating to these env vars as they do not enable tracing - address must be set, unlike it states here. #32667

[marefr]

This doesn't look right. You can actually set it either using GF_TRACING_JAEGER_ADDRESS or either using JAEGER_AGENT_HOST and JAEGER_AGENT_PORT, see https://www.jaegertracing.io/docs/1.16/client-features/.

[conorevans]

I used JAEGER_AGENT_HOST and JAEGER_AGENT_PORT initially and it did not initialize tracing for me. Let's look at tracing.go:

func (ts *TracingService) Init() error {
	ts.log = log.New("tracing")
	ts.parseSettings()

	if ts.enabled {
		return ts.initGlobalTracer()
	}

	return nil
}

ts.initGlobalTracer() is where the Jaeger Cfg is parsed:

func (ts *TracingService) initGlobalTracer() error {
	cfg, err := ts.initJaegerCfg()
	if err != nil {
		return err
	}
....

And it'll only go to ts.initGlobalTracer() if ts.enabled. So let's return to ts.ParseSettings() just before it:

func (ts *TracingService) parseSettings() {
	var section, err = ts.Cfg.Raw.GetSection("tracing.jaeger")
	if err != nil {
		return
	}

	ts.address = section.Key("address").MustString("")
	if ts.address != "" {
		ts.enabled = true
	}
...

We can see ts.enabled is explictly dependent on GF_TRACING_JAEGER_ADDRESS.

Maybe the Jaeger config would start up successfully using JAEGER_AGENT_HOST and JAEGER_AGENT_PORT (rather than GF_TRACING_JAEGER_AGENT_PORT, so the update to the text here isn't relevant, but the issue #32667 is relevant.

[marefr]

#32667 is definitely relevant for enable tracing and should be fixed. I thought that actually was fixed, but I was wrong. I'm pretty sure there was some other PR that changed things. Had #23887 in mind, but no it didn't touch upon this problem. #25768 is related, but didn't really do anything in regards to enable tracing. Maybe I started some work on this back in the day, but never ended up in a PR?

However, if you would provide a value for GF_TRACING_JAEGER_ADDRESS and values for JAEGER_AGENT_HOST and JAEGER_AGENT_PORT I think the latter ones would be used.

[conorevans]

However, if you would provide a value for GF_TRACING_JAEGER_ADDRESS and values for JAEGER_AGENT_HOST and JAEGER_AGENT_PORT I think the latter ones would be used.

Yeah definitely.

I think this PR arose from my troubles at #32667. I think this PR can be closed - after the above comment where I dug a bit further, I realized that these docs would be fine and accurate if #32667 did not exist.

[achatterjee-grafana]

Need developer review and approval.

[marefr]

Standard jaeger environment variables are still supported and should still be documented, but agree that explaining that GF_<section> convention also could be used is a good thing.

[marefr]

Based on information about standard jaeger environment variables above (documented at https://www.jaegertracing.io/docs/1.16/client-features/) this needs to explain that you can use GF_<section> convention if you prefer as well.

The reason for using standard jaeger environment variables is because some uses that for all their services, including Grafana, as convenience.

[marefr]

Information about JAEGER_TAGS is still needed, but you can explain that GF_TRACING_JAEGER_TAGS can be used as well.