Review: feat: Added host.name to OTel traces

Reviewed the change end-to-end and verified the two load-bearing OTel-SDK behaviors (detector merge precedence and schema-URL handling) against the go.opentelemetry.io/otel/sdk v1.38.0 source. The full protocol/tracing suite passes, and go vet / gofmt are clean.

Verdict: clean, well-scoped, well-documented. The mechanism is correct. One robustness edge case is worth considering plus a few trivial notes — none blocking.

What's done well

• The precedence comment is accurate and earns its place. Detector merge-ordering in resource.New is genuinely non-obvious. Confirmed: detect folds each detector via Merge(res, r) and Merge(a, b) lets b win on key collisions, so later detectors win — WithFromEnv() being last is what gives operators the override. TestNewResource_HostNameOverridableViaEnv exercises this.
• No schema-URL conflict — checked, because it's the classic footgun here. Only WithHost() contributes a non-empty schema URL (semconv.SchemaURL); WithAttributes and WithFromEnv produce schemaless resources, and merging empty-with-non-empty never triggers ErrSchemaURLConflict. Verified empirically (newResource returns no error in the tests).
• The hermeticity fix is correct and important. Adding OTEL_RESOURCE_ATTRIBUTES to clearTracingEnv stops the override test from leaking host.name=eu-west-1-consumer into sibling tests. Easy to forget.
• The newResource extraction is the right move — it makes resource construction unit-testable without standing up the full exporter/SDK pipeline.

Potential issue (worth considering, non-blocking)

Adding WithHost() introduces a new, environmental trigger for a fatal startup path.

resource.WithHost() calls os.Hostname(). On error, the SDK's stringDetector.Detect returns a plain error (not ErrPartialResource); detect then joins it and continues with the error still set, so resource.New returns a usable partial resource plus a non-nil error. That error flows up through newResource → tracing.New, and all three call sites abort startup:

// rpcconsumer.go:684, rpcprovider.go:900, rpcsmartrouter.go:1589
traceManager, err := tracing.New(ctx, traceCfg)
if err != nil {
    return err   // returned from cobra RunE → the lavap binary fails to start
}

The point isn't probability — os.Hostname() essentially never fails on Linux (realistically only ENAMETOOLONG, a >64-byte hostname, which normal pod/host names won't hit). The point is asymmetry: before this PR, newResource could only fail on operator-malformed OTEL_RESOURCE_ATTRIBUTES (a config error). This adds the first environmental, non-config way for it to fail — and the consequence is a node that won't boot, with a failed to create OTel resource message that gives no hint the culprit is a hostname quirk. Since the PR makes this unconditional, the blast radius is every binary, all to protect an observability label that should never be able to take a node down.

Suggested fix (preferred): log-and-continue. This also matches OTel's guidance to treat resource.New errors as non-fatal. You already get a usable partial resource back, so in New():

res, err := newResource(ctx, cfg)
if err != nil {
    // resource.New returns a usable partial resource even on error (e.g. if
    // os.Hostname() fails). Warn and proceed rather than aborting node startup
    // over a missing observability attribute.
    utils.LavaFormatWarning("OTel resource detection incomplete; continuing", err)
}

(res is always non-nil here — resource.New allocates it before detection and mutates in place.) Keeps WithHost(), no new imports.

Narrower alternative: detect the hostname yourself with os.Hostname() and add it via WithAttributes only on success, dropping WithHost() — a failed lookup is then silently skipped and newResource keeps its old config-error-only failure profile. Touches only the new path, but the log-and-continue version is more robust since it also covers any future detector.

Minor / informational

• Schema-URL side-effect (benign): before this PR the resource's schema URL was empty; after, it becomes the SDK's bundled semconv version (from WithHost()). Harmless — the attribute keys are stable across semconv versions — but backends keying on schema URL will now see one. Worth a note, not a change.
• semconv version mix (cosmetic): the code pins semconv/v1.21.0 for ServiceName while WithHost() uses the SDK's newer internal semconv for the host attribute. Not a bug, just slightly inconsistent.
• Test DRY: both new tests repeat the same for … range res.Attributes() scan; a small getAttr(res, "host.name") (string, bool) helper would tighten them. TestNewResource_HostNameOverridableViaEnv also asserts only the value, not presence — a dropped attr would give a slightly less clear "" != "eu-west-1-consumer" failure.
• Coverage gap (optional): no test asserts that service.name and host.name coexist, nor that OTEL_SERVICE_NAME override still works now that a second detector is in the chain. Low value if service-name override is covered elsewhere.

Summary

I'd suggest the log-and-continue tweak before merge, but it's a judgment call — the change is otherwise solid.