Include pod UID and container name as optional trace labels

[sfleen]

It's useful to include these fields in the trace span attributes to help correlate traffic within a cluster.

This propagates these labels through the trace initialization, similar to how we handle the service name (and other labels that last for the entire pod lifetime).

See linkerd/linkerd2#13501 for the corresponding control plane change.

[cratelyn]

you've been clippy'ed 📎 🙀

Error: error: this function has too many arguments (8/7)
  --> linkerd/app/src/trace_collector/otel_collector.rs:21:1
   |
21 | / pub(super) fn create_collector<S>(
22 | |     addr: ControlAddr,
23 | |     hostname: Option<String>,
24 | |     pod_uid: Option<String>,
...  |
29 | |     legacy_metrics: metrics::Registry,
30 | | ) -> EnabledCollector
   | |_____________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
   = note: `-D clippy::too-many-arguments` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(clippy::too_many_arguments)]`

[cratelyn]

besides the note about clippy lints above, this looks like a good PR to me. this should be a nice improvement to our traces!

[codecov]

Codecov Report

Attention: Patch coverage is 5.00000% with 19 lines in your changes missing coverage. Please review.

Project coverage is 66.74%. Comparing base (96124bc) to head (bedbace).
Report is 715 commits behind head on main.

Files with missing lines	Patch %	Lines
linkerd/app/src/trace_collector/otel_collector.rs	0.00%	7 Missing ⚠️
linkerd/app/src/trace_collector.rs	0.00%	6 Missing ⚠️
linkerd/app/src/env.rs	20.00%	4 Missing ⚠️
linkerd/app/src/env/trace.rs	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3483      +/-   ##
==========================================
- Coverage   67.68%   66.74%   -0.94%     
==========================================
  Files         332      388      +56     
  Lines       15158    18171    +3013     
==========================================
+ Hits        10259    12128    +1869     
- Misses       4899     6043    +1144     

Files with missing lines	Coverage Δ
linkerd/app/src/env/trace.rs	46.66% <0.00%> (ø)
linkerd/app/src/env.rs	52.73% <20.00%> (-6.67%)	⬇️
linkerd/app/src/trace_collector.rs	26.47% <0.00%> (ø)
linkerd/app/src/trace_collector/otel_collector.rs	0.00% <0.00%> (ø)

... and 176 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 387197e...bedbace. Read the comment docs.

[olix0r]

We've generally been pretty adamant (and successful) about not leaking kubernetes concepts (pods, containers, uids, etc) into the proxy's codebase.

You may notice that we load a bunch of attributes from a file at starttime:

https://github.com/linkerd/linkerd2-proxy/pull/3483/files#diff-0ee25f0088036de1851c9f218f1abb2fd0c4abecefe846112c34c6533b2267ccR840-R845

is it possible to encode these attributes into this file so the proxy doesn't need special casing for them?

[sfleen]

AFAICT, the downward API doesn't let you encode more than one field ref in a single file, nor does it give you particularly good control over the keys for the values (unless you're using labels or annotations, and I'd rather not dump all of those for a pod into each trace).

That being said, I think we can still encode these through environment variables as a list of arbitrary extra fields to add. Thoughts on linkerd/linkerd2#13544?

[olix0r]

Ah, I see your point: the file as it is only include pod labels. I agree that the next best solution is as you suggest--make the proxy generic about the specific keys.

[sfleen]

Cool, I've updated this PR to reflect those changes to the attributes, PTAL