Auto-inject the IP tag for Operator-injected Agents

[TBBle]

The Operator provides a few default tags for Jaeger Agents when they are deployed as auto-injected side-cars.

Since a Pod is limited to a single host, it'd be useful to inject the 'ip' tag the same way. This would make life much simpler for non-Jaeger clients (such as the OpenCensus exporter) and saves having to include code to discover the hostIP in every k8s-hosted client, since the default IP for the networking stack is almost certainly for the container's network namespace, not the actual host IP.

This would help alleviate jaegertracing/jaeger#1459 by ensuring that at-least such Pods do not get false results from the Clock Skew Adjuster because they are seen as different hosts, even though they are different pods on the same hardware.

This probably isn't trivial hard, I believe this requires that the Jaeger Agent container have an env-var populated from its status.hostIP, and then that can be passed through via the command-line.

[jpkrohling]

It is probably trivial, as we inject the sidecar ourselves. As you hinted, we can use the downward API, like we do with the pod name, to get the host IP:

jaeger-operator/pkg/inject/sidecar.go

Lines 161 to 168 in b4a35c4

	Env: []corev1.EnvVar{{
	Name: envVarPodName,
	ValueFrom: &corev1.EnvVarSource{
	FieldRef: &corev1.ObjectFieldSelector{
	FieldPath: "metadata.name",
	},
	},
	}},

And then, just need to add a new agent tag, like this:

jaeger-operator/pkg/inject/sidecar.go

Line 139 in b4a35c4

"pod.name", fmt.Sprintf("${%s:}", envVarPodName),

Would you like to give it a try?

[TBBle]

I can't commit to having time to knock it out, but if no one else gets around to it, I'll try looking at it when I add unit tests to #864.

[jpkrohling]

Sounds good! Just make sure to send it as a separate PR.

[TBBle]

For anyone coming to this issue later, in the end the injected tag was not ip but host.ip, as the Jaeger clients internally add an ip tag already, and so the span would end up with two ip tags with different values.

So we have a nice and shiny new tag to use for tying together different pods on the same node, we don't get the magic "Clock Skew Adjuster does the right thing" fix I was hoping for.

[jpkrohling]

we don't get the magic "Clock Skew Adjuster does the right thing" fix I was hoping for.

Yeah, sorry about that, but I don't think there's a sane way of doing that right now. Perhaps you could propose a feature in the main repository for it? Something like a host-correlation tag? Or perhaps to take into consideration other ip tags, such as host.ip and possibly node.ip (in case they ever mean something different?).

[TBBle]

Yup, I just added a note to the issue about the Clock Skew Adjustor for exactly that. ^_^

I probably won't have time soon to propose a PR for such a change, but I can hope someone else might like the idea enough to try it, depending on the maintainer reaction to the suggestion.